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

Make menubar its own widget and add overflow #63954

Merged
merged 7 commits into from
Dec 3, 2018

Conversation

sbatten
Copy link
Member

@sbatten sbatten commented Nov 28, 2018

MenubarControl has been broken into 2 pieces:

  1. a widget that controls display and interaction and
  2. a control that handles the model and vs code settings changes

This made it very easy to add a the overflow logic as part of the widget

resolves #57756

@sbatten sbatten added this to the November 2018 milestone Nov 28, 2018
@sbatten sbatten self-assigned this Nov 28, 2018
@sbatten sbatten requested a review from bpasero November 28, 2018 23:26
@sbatten
Copy link
Member Author

sbatten commented Nov 28, 2018

Should probably not use More, but rather triple dot icon

@bpasero bpasero self-assigned this Nov 29, 2018
Copy link
Member

@bpasero bpasero left a comment

Choose a reason for hiding this comment

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

Given the amount of changes, I did not do a code review. I like the direction though, especially making the menubarControl.ts in workbench slimmer.

Playing around with it (on Linux): I agree we could use a "..." similar to this one:

image

Though I was actually not opposed to the "More" either.

PS: wow, windows seems to wrap actually (not my preference):

image

@bpasero
Copy link
Member

bpasero commented Nov 29, 2018

@sbatten there seems to be an issue with the direction of where the menu opens in this case:

image

Since space is typically limited to the right when "More" shows up I believe you need to show the menu then to the left to account for the narrow size, otherwise a user will probably have issues accessing all menu entries from the "More" menu.

@bpasero bpasero removed their assignment Nov 29, 2018
@sbatten
Copy link
Member Author

sbatten commented Dec 1, 2018

@bpasero thanks for the review, you can see quite a few changes since your approval.

  • Using ellipsis
  • Compact menus that cascade when needed. The drawing to the left didn't help in most cases, because a lot of the time you only have room for 1.5 menus when testing this
  • Mnemonics fixed when in overflow

@bpasero bpasero assigned bpasero and unassigned bpasero Dec 1, 2018
@bpasero
Copy link
Member

bpasero commented Dec 2, 2018

@sbatten looks good to me. if you want to merge it for Nov, it should get a good test plan item.

@sbatten sbatten merged commit aedd440 into microsoft:master Dec 3, 2018
@sbatten sbatten deleted the overflow branch December 3, 2018 14:40
bpasero added a commit that referenced this pull request Dec 4, 2018
sbatten added a commit to sbatten/vscode that referenced this pull request Dec 4, 2018
sbatten added a commit that referenced this pull request Dec 4, 2018
* Revert "Revert "Make menubar its own widget and add overflow (#63954)""

This reverts commit fa87e67.

* add checks for whencustom menubar is not present
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No menu overflow causes problems with narrow windows/long titles
2 participants