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

Compact menubar option #80900

Merged
merged 8 commits into from
Sep 25, 2019
Merged

Compact menubar option #80900

merged 8 commits into from
Sep 25, 2019

Conversation

sbatten
Copy link
Member

@sbatten sbatten commented Sep 14, 2019

Use window.menuBarVisibility setting to use the new compact option.

@sbatten sbatten self-assigned this Sep 14, 2019
@sbatten
Copy link
Member Author

sbatten commented Sep 14, 2019

@bpasero feel free to test out this branch and review at a high level. I've also queued a build that you can play with from the branch sbatten-build-compactMenus. some code cleanup to be done around theming and css

I know your feedback currently has been apprehensive to the location, but I think it is good to have everyone actually try it and see how we feel after some usage.

@bpasero
Copy link
Member

bpasero commented Sep 14, 2019

Adding @misolori (and I would also like to hear @stevencl feedback as someone that has been designing the activity bar from the beginning with us).

Current State:

image

image

Some unsorted feedback:

  • I feel that the location is not good because we already have an icon with a menu to the bottom and now we have one to the top that is indistinguishable from the view icons. If we keep it to the top I would suggest to maybe change the background color of it to make clear it is something unrelated to the view icons
    image
    Maybe we should even put the VSCode icon in there? You should check out these designs:
    image

From #17060

  • hovering with the mouse over the menu icon should give me the same feedback as on any other icon (cursor change, color change)
  • Should the compact option only be available for web or for all platforms?
  • There should not be a menubar control within the activity bar, rather I feel like we need 2 locations for menu contributions in the activity bar: global (settings) and global (menu). The existing gear icon is doing it properly imho by allowing to contribute items into
  • I feel like .monaco-workbench .activitybar > .content that need :not(.monaco-menu) is a hack to be avoided

@sbatten
Copy link
Member Author

sbatten commented Sep 23, 2019

Regarding the menu bar control, this reuse minimizes the amount of code churn to recreate the menubar without breaking things like mnemonics, which work on windows on full screen. Otherwise I will have to rewrite some of the menubar logic to make this work properly. It is a fair debt item but I think this first approach is a good way to test various designs.

Regarding the css rules, for now I can change the rules to have one for global and activities to look less hacky. If we end up keeping the menubar in the activity bar I can try to clean up the menu system to address this issue and above

@bpasero
Copy link
Member

bpasero commented Sep 24, 2019

I am fine leaving debt items for the debt week to move this forward. What about the UX issues?

@miguelsolorio
Copy link
Contributor

If we keep it to the top I would suggest to maybe change the background color of it to make clear it is something unrelated to the view icons

I do agree that we should probably add some type of visual cue that this is different than the rest.

Maybe we should even put the VSCode icon in there?

I did try this option out but it's not obvious that the logo is clickable/a menu:

image

I think this is enough to start to get feedback on the menu and we can continue to iterate on the styling (adding hover feedback, background color, etc.).

@bpasero
Copy link
Member

bpasero commented Sep 24, 2019

@misolori I really like the variant with the icon, can we try it out?

@sbatten
Copy link
Member Author

sbatten commented Sep 25, 2019

I've made minor changes and merged master. @misolori if you want to drop in an icon or have any idea what that differentiator should be, feel free to drop it in on this branch.

@sbatten sbatten marked this pull request as ready for review September 25, 2019 00:53
@sbatten sbatten merged commit d49dad0 into microsoft:master Sep 25, 2019
@sbatten sbatten deleted the compactMenu branch September 25, 2019 18:16
@miguelsolorio
Copy link
Contributor

Notes from today's UX sync:

  • We should look for ways to make the menu stand out so it’s obvious that it is different from the rest of the activity bar, we'll continue to iterate on this
  • Merge this into master so we can get more feedback from Insiders
  • Re-visit icon choice if feedback comes back negatively
  • Once enabled on desktop, this could cleanup the menu on Windows

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.

3 participants