Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented Toolbar support for sidepanels and changed sidepanel tabs. #4600

Merged
merged 3 commits into from
Mar 20, 2019

Conversation

jbicker
Copy link
Contributor

@jbicker jbicker commented Mar 18, 2019

Every sidepanel has an header now where the title and a toolbar
is shown (if tools are registered for that widget).
As an example the control buttons of search in workspace extension are
moved to the toolbar.

Additionally the tabs of sidebars don't show labels anymore but icons.

fIx #3864, fIx #716

@akosyakov
Copy link
Member

akosyakov commented Mar 19, 2019

@jbicker build is broken and some nice screenshots in the description will be cool :)

TODO:

@@ -174,6 +180,8 @@ export interface TabBarToolbarItem {
*/
readonly when?: string;

onDidChange?: (handler: () => void) => void;
Copy link
Member

@akosyakov akosyakov Mar 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use Event<void> instead

Copy link
Contributor

@svenefftinge svenefftinge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great just a few small improvements.

packages/core/src/browser/style/sidepanel.css Show resolved Hide resolved
packages/core/src/browser/shell/side-panel-toolbar.ts Outdated Show resolved Hide resolved
packages/core/src/browser/style/sidepanel.css Outdated Show resolved Hide resolved
@svenefftinge
Copy link
Contributor

Also please change the label for our navigator from 'Files' to 'Explorer'.

@akosyakov
Copy link
Member

akosyakov commented Mar 19, 2019

For https://marketplace.visualstudio.com/items?itemName=ms-vscode.references-view extension:
Screen Shot 2019-03-19 at 09 33 35

There is no icon, duplicate title.

Updated
I will take care about it.

@akosyakov
Copy link
Member

Menubar does not seem to aligned:
Screen Shot 2019-03-19 at 11 04 07

It also feels so small compare to side bars.

@vince-fugnitto
Copy link
Member

vince-fugnitto commented Mar 19, 2019

Menubar does not seem to aligned:
Screen Shot 2019-03-19 at 11 04 07

It also feels so small compare to side bars.

I was also wondering, do we want to have a black background over the active items, or simply have all icons dimmed except for the active one? This of course comes from VSCode.

image

@akosyakov
Copy link
Member

I was also wondering, do we want to have a black background over the active items, or simply have all icons dimmed except for the active one? This of course comes from VSCode.

The active item is highlighted with blue line. The current item has black background right now, in addition to having 1 as opacity. I would be for aligning with VS Code and removing black background, only use opacity.

@kittaakos
Copy link
Contributor

I got this in Gitpod:

Screen Shot 2019-03-19 at 16 46 40

@kittaakos
Copy link
Contributor

If I move from the side to the bottom or main panel, I do not have the icons anymore, there should be a fallback:

Screen Shot 2019-03-19 at 16 47 46

@kittaakos
Copy link
Contributor

kittaakos commented Mar 19, 2019

The SVG icons and the fallback ones have different color:
Screen Shot 2019-03-19 at 16 49 01

Not a bug, I was blind and the opened tab has a different color.

@akosyakov
Copy link
Member

In the light theme it is hard to tell which tab is current and title could be a bit more darker.
Screen Shot 2019-03-19 at 16 46 09

For comparison in VS Code:
Screen Shot 2019-03-19 at 16 47 57

@kittaakos
Copy link
Contributor

The toolbar icons have black color on the dark theme. It is hardly noticable:
Screen Shot 2019-03-19 at 16 50 01

@kittaakos
Copy link
Contributor

Other strange icons on the side bar:

Screen Shot 2019-03-19 at 16 51 55

@jbicker
Copy link
Contributor Author

jbicker commented Mar 19, 2019

The SVG icons and the fallback ones have different color:
Screen Shot 2019-03-19 at 16 49 01

The currently opened panel has a highlighted tab. That is why both are different.

jbicker and others added 3 commits March 20, 2019 09:32
Every sidepanel has an header now where the title and a toolbar
is shown (if tools are registered for that widget).
As an example the control buttons of search in workspace extension are
moved to the toolbar.

Additionally the tabs of sidebars  don't show labels anymore but icons.

Fixes #3864

Signed-off-by: Jan Bicker <jan.bicker@typefox.io>
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jbicker looks good now, please proceed with merging

@jbicker jbicker merged commit 6d5c14a into master Mar 20, 2019
@svenefftinge svenefftinge deleted the GH-3864 branch March 20, 2019 13:22
@benoitf
Copy link
Contributor

benoitf commented Mar 22, 2019

Hello, I've noticed this PR caused a regression on webview tabs
there is a white background color behind icon
image

while for other icons of tabs
image

there is an override with background: none

I've created #4664

@benoitf
Copy link
Contributor

benoitf commented Mar 22, 2019

I commented on #4654 but it should be better to comment it there:

I think it's misleading people to have extension icon of Theia being the same icon as VS Code extension as you might expect that you're able to use VSCode extensions while it's bring by plug-ins.

So I would be more in favor of having plug-in icon being the same than vscode extension as you're managing the same packaging unit.

@akosyakov
Copy link
Member

akosyakov commented Mar 23, 2019

@benoitf i don't care about extension-manager, we can just erase for now references to it from everywhere (examples, docs, images). plugin ext can us VS Code extension icon regardless of it. Also in docs we should include plugin-ext-vscode instead (see https://stackoverflow.com/questions/55254187/cant-find-hosted-mode-in-theia) Let's create an issue for it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants