-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Rework sidepanel behavior on tab overflow #12593
Conversation
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.
The change looks fairly good. I only have a single point of critique: When decreasing the size of the window so that two widgets are hidden, increasing the size afterwards only shows one again, while the other one keeps being hidden behind the kebab menu:
2023-06-13.19-16-11.mp4
I would expect the last two entries to reappear at the same time.
@@ -586,7 +595,7 @@ export class ScrollableTabBar extends TabBar<Widget> { | |||
protected scrollBar?: PerfectScrollbar; | |||
|
|||
private scrollBarFactory: () => PerfectScrollbar; | |||
private pendingReveal?: Promise<void>; | |||
protected pendingReveal?: Promise<void>; |
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.
Minor: I'm wondering why we have any private
fields at all here. Usually, we just have everything protected
, especially in the core
package.
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.
Good point. I changed the visibility of private fields in tab-bars.ts
class to protected.
Rework behavior of the sidepanels if there more tabs than they can fit. Introduce behavior similar to VS Code: Overflowing tabs will simply be hidden or 'merged' into one menu button at the end of the tabbar. Clicking the button allows revealing of a currently hidden view. Special handling is implemented to ensure that the currently selected view never gets hidden away. Fixes #12416
Thanks for the review @msujew. I have rebased and updated the PR. |
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.
Thanks, looks good to me 👍 Only one remark, see below.
What it does
Reworks the behavior of the sidepanels if there are more tabs than they can fit (overflow). Instead of the counter intuitive on-hover scroll bar we introduce behavior similar to VS Code:
Overflowing tabs will simply be hidden or 'merged' into one menu button at the end of the tabBar. Clicking the button allows revealing of a currently hidden view. Special handling is implemented to ensure that the currently selected view never gets hidden away. When a view that is currently hidden is revelead (i.e. by opening via command) it will be moved to the visible part of the tabbar.
As outline in #12416 this feature is currently intended to become the default behavior and should replace the scroll approach. If there are any objects we could also make in an opt-in feature (toggleable via settings).
Fixes #12416
Contributed on behalf of STMicroelectronics
How to test
Feature can be tested by populating the sidebars with a lot of views and/or resizing the window. Following cases should be tested:
And probably a lot of other corner cases that I didn't think about 😉
Review checklist
Reminder for reviewers