-
Notifications
You must be signed in to change notification settings - Fork 30.6k
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
Add settings for fixed-width tabs #181729
Conversation
This is meant at least partially to address microsoft#40290 and is a continuation of the unfinished work from microsoft#40750.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a nice chance of implementing exactly what Google Chrome does for tabs:
- tabs have a fixed width always and scrollbars never show up
- when available space is not enough, all tabs shrink evenly
- when you close a tab, sizes remain until mouse moves out [1]
[1] I think this can easily be achieved with a change such as the one for Atom: https://github.com/atom/tabs/pull/300/files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a nice chance of implementing exactly what Google Chrome does for tabs:
- tabs have a fixed width always and scrollbars never show up
- when available space is not enough, all tabs shrink evenly
- when you close a tab, sizes remain until mouse moves out [1]
[1] I think this can easily be achieved with a change such as the one for Atom: https://github.com/atom/tabs/pull/300/files
I think just having a new option to address the linked issue is not sufficient, it would be nice if we can even make this new option useful for people that prefer the browser tab model.
Thanks for the pointer @bpasero , I'll see what I can do. |
Tabs shrink uniformly (down to a limit) but stay fixed-width when the mouse is over the tab bar.
@bpasero done, please let me know what you think. 8-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WOW 👏
Just testing this for 1 minute and it seems exactly what we want.
But give me more time to review closely and see for corner cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please also do some testing of how this new sizing option behaves with:
workbench.editor.pinnedTabSizing
and pinned tabs in general (pin a tab from its right click menu)workbench.editor.tabCloseButton
onleft
oroff
workbench.editor.wrapTabs
being enabled
src/vs/workbench/browser/parts/editor/media/tabstitlecontrol.css
Outdated
Show resolved
Hide resolved
Thanks for the thorough review @bpasero, I'll address everything, possibly over the weekend. |
To achieve this, it's best to remove the transition delay.
@bpasero report from testing:
I have one outstanding question on your review, on guidance about disposable listeners; I'll put it in a comment there. |
Also, we need to fix tab fading. Here is what is expected when the label is too large (see how Needs the fix for: #182481 |
Co-authored-by: Benjamin Pasero <benjamin.pasero@gmail.com>
I don't understand yet. The way I see it is like this:
In CSS, this means the transition must be there when the mouse is outside of the tab stripe. I personally like the transition when the setting changes. Maybe I'm just used to this kind of smooth setting change from somewhere - maybe it's a Mac thing. |
Tbh we use transitions very rarely throughout the UI. Mostly to draw attention, for example when showing notifications. We do not use it as a way to convey that something has changed in the UI. I would even argue that for this feature we should maybe not do the transition. |
On advice of @bpasero, removed transition because the editor doesn't really use transition that much.
Yeah, all that complexity is why I suggested simple fixed-width tabs in the first place… 8-) I have no idea on how confusing it would be; if someone tries it and doesn't like it, they can go back to the previous behaviour and open issues; if that happens, feel free to tag me in case I can address them. |
I'll be happy to chat, I'm also on the vscode discord server, but I don't know my way around there much as I've never really used it… |
@jacekkopecky I took the liberty to push more changes, specifically the fixed tabs width now only applies when editors are closed: I will add more feedback in a review. |
src/vs/workbench/browser/parts/editor/media/tabstitlecontrol.css
Outdated
Show resolved
Hide resolved
@bpasero I'll be busy for a few days, so I may not get to working more on this until Friday, sorry. I'll have a look at your changes and try to record the problem above to better illustrate it. |
@jacekkopecky I was about to approve this but found another case where it can be hard to close multiple tabs with the mouse: some editors show more actions than other editors, so the available space for tabs can grow and shrink while closing. This currently causes the close button to move and thus prevents closing if you keep the mouse over the same spot: I think in addition to having tabs be fixed when mouse is over tabs, we also need to prevent actions from drawing and only appear when the mouse moves out? The code for updating editor toolbar actions is shared between tabs and no-tabs: vscode/src/vs/workbench/browser/parts/editor/titleControl.ts Lines 230 to 235 in 222a960
|
@bpasero in the recording I don't see you closing a tab, but in my testing, I have found this problem (I don't know if it's the one you mean): When we click the close button, I have pushed code in cac6854 that fixes that problem; I'm not sure if I've done it right, or if we already have a way to register something that should run before the close action. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏 , this was a really fun one. You are very fast in responding, that is a pleasure. If you want to pick up any other issue where we ask for help, be my guest 🚀
Thank you @bpasero for being patient with me; I'm happy to get this through, mainly for myself. 8-) |
@bpasero well this is embarrassing, but as you closed #40290 , I looked at it again and saw I missed one corner case – when the tab being closed is the last tab – in which case the tabs might get back to their normal size, as they do in Chrome and Safari. Here is a fix for your consideration: jacekkopecky@5e85bb6 For illustration, here's a before and after: (my cursor is huge and the recording software misplaces it, but it was right over the close icons) |
@jacekkopecky good catch! want to open a new PR? |
This PR adds a new setting value for
workbench.editor.tabSizing
:fixed
makes all tabs (except pinned when those are compact or shrunk) the same width; and, like theshrink
setting, the tabs can shrink when there are too many.The PR also adds a setting for the maximum (or default) width:
workbench.editor.tabSizingMaxWidth
, at least 50px to give enough width for the close button as well as at least the icon or part of the title.This is meant to address #40290 and is a continuation of the unfinished work from #40750.
This is what it looks like:
Limitation: the width are fixed solid while the mouse is over the tab bar, so creating a new tab with the keyboard while the mouse is over tabs behaves a wee bit funny.
Let me know what you think.