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

Align tabbar styling to vscode #10908

Merged
merged 6 commits into from
Apr 11, 2022
Merged

Align tabbar styling to vscode #10908

merged 6 commits into from
Apr 11, 2022

Conversation

msujew
Copy link
Member

@msujew msujew commented Mar 22, 2022

What it does

Closes #10906

How to test

  1. Open editors/widgets in the main area
  2. While hovering over them, changing tabs, etc. they should behave as VSCode's main area tabs
  3. The focus for bottom/side panel tabs now behaves a bit differently, there's no distinction between active and non-active tabs anymore.

Review checklist

Reminder for reviewers

@msujew msujew added the ui/ux issues related to user interface / user experience label Mar 22, 2022
@colin-grant-work colin-grant-work self-requested a review March 23, 2022 17:00
Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

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

Looks good to me, and closely mirrors the behavior of VSCode 👍. Would be nice to get others' opinions before merging, as I'm not a particularly sensitive observer of design aesthetics. @vince-fugnitto, @JonasHelming

@msujew msujew mentioned this pull request Mar 28, 2022
1 task
@colin-grant-work
Copy link
Contributor

colin-grant-work commented Mar 29, 2022

One thing I've noticed just now is that it looks like we have two levels of tabbar prominence:

image

the active tabs on all tabbars have the same level of prominence, and the inactive tabs are all the same.

But VSCode has four:

image

The inactive tabs on the active tabbar are different from the inactive tabs on inactive tabbars, and the active tab on the active tabbar is different from the active tab on the inactive tabbar.

@msujew
Copy link
Member Author

msujew commented Apr 5, 2022

@colin-grant-work Thanks for the hint, I added support for "active/inactive" tabbars and adjusted the tabbar font color as needed 👍

@msujew msujew requested a review from vince-fugnitto April 7, 2022 22:39
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

@msujew I noticed that the flicker I fixed as part of #10822 happens again (I even rebased the changes).

The fix I had included was to add a transparent border for inactive tabs at all times, so that when a tab is in fact activated it would not cause the flicker compared to inactive ones.

@msujew msujew force-pushed the msujew/vscode-tab-bar branch from 2a47f28 to fe22518 Compare April 8, 2022 13:54
@msujew
Copy link
Member Author

msujew commented Apr 8, 2022

@vince-fugnitto I've decided to replace the border-top style with a box-shadow which accomplishes the same thing without influencing the layout. It seems like if the border-top-color property isn't set correctly (such as for the default dark/light themes), the border isn't rendered at all, thereby changing the layout of the tab.

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

The changes look good to me 👍
I confirmed that the styling of tabs in the main-area and bottom area work well, there is no flicker and if a theme sets the tab.activeBorderTop it is applied.


@msujew perhaps out of the scope of the pull-request but should we use the same box-shadow trick to properly apply tabs.activeBorder?

Border on the bottom of an active tab. Tabs are the containers for editors in the editor area. Multiple tabs can be opened in one editor group. There can be multiple editor groups.

I used vs-theme-0.0.1.zip to test:

"colors": {
    "tab.activeBorderTop": "#00ff00",
    "tab.activeBorder": "#ff0000"
}

vscode:

master-active-border

pull-request:

pr-active-border

@msujew msujew force-pushed the msujew/vscode-tab-bar branch from fe22518 to 12dd01a Compare April 8, 2022 15:09
@msujew
Copy link
Member Author

msujew commented Apr 8, 2022

@vince-fugnitto Yep, was perfectly doable using the same box-shadow approach :)

grafik

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

The changes are a really nice improvement 👍

Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

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

I reaffirm my approval with the latest changes: All four levels of prominence are respected for tabs.

@msujew msujew force-pushed the msujew/vscode-tab-bar branch from 45f7a3b to aeb6c00 Compare April 11, 2022 15:29
@msujew msujew merged commit a9c970d into master Apr 11, 2022
@msujew msujew deleted the msujew/vscode-tab-bar branch April 11, 2022 15:44
@github-actions github-actions bot added this to the 1.25.0 milestone Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ui/ux issues related to user interface / user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update the tab bars to VSCode's new style
3 participants