-
Notifications
You must be signed in to change notification settings - Fork 37.7k
Adding context menu on the sticky scroll line (Fixes #162330) #162425
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
Conversation
Tirednfedup
left a comment
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.
Is there any suggestions?
jrieken
left a comment
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 looks really good, left a tiny nit about the name of the menu (consistency matters)
I would also suggest to add the 'Toggle Breadcrumbs' command into the command. I believe sticky scroll and breadcrumbs are closely related and configured them in a single place sounds like a good idea. You can find the command here: ToggleBreadcrumb
Doing this would also act as a clue about where I might be able to go to toggle Sticky Scroll back on again. But only if right-click on breadcrumbs also gave this kind of menu. Without that I have to realize that the View menu is the place to go, or Command Palette. Also, I wonder if the "Toggle prefix is necessary? And should other editor-view-related toggles appear here as well? For example Minimap (currently on a context menu of an editor's vertical scrollbar, as well as one from the minimap itself when visible). |
I would agree that having the menu appear also on the breadcrumbs bar would be more intuitive. |
That's a good point. It seems that we don't have a good story for this, we often use "Toggle XYZ" because of places that don't render the check mark, e.g the command palette. We should maybe expand on the existing toggled title so that the default label is "XYZ" (which gets rendered with the check mark_ and then have on/off labels like "Show XYZ" and "Hide XYZ" for places that don't renderer the check mark. This is an interesting discussion but I am in favour of leaving this out of this PR. |
|
There is #162672 for the toogle-title disuccsion |
Fixes #162330.
Adding the possibility to toggle the sticky scroll line by right clicking on it. I had to modify the ClickLinkGesture in order to introduce a new parameter 'hasRightClick' which will differentiate between a normal click and a right click. Here is an example of how it currently works: