Update custom menu styles #149187

daviddossett · 2022-05-10T20:53:41Z

This PR fixes #148587 by updating the following menu styles:

Background color on default dark and base dark theme for better separation
Foreground color on base dark theme for better contrast
100% width separators with updated colors
Slight increase in item height
Border radius on context menu
Removes 1px border from items

To do:

Fix focus border jank on hc themes (@sbatten I'm attempting to use an existing rule that addressed this only for HC themes, but using :host-context doesn't seem to have any effect here.

Before

After

miguelsolorio

Overall, the new styling is looking really slick 👌 just need to fine-tune the color tokens and ensure it doesn't break existing themes

extensions/theme-defaults/themes/dark_vs.json

src/vs/platform/theme/common/colorRegistry.ts

sbatten · 2022-05-12T00:21:56Z

There is a problem with the corners

You can see the rounded corner, then a sharp corner, then the shadows

sbatten · 2022-05-12T00:26:37Z

Are you intentionally making the keyboard shortcuts use disabled styling when not focused?

sbatten · 2022-05-12T00:27:53Z

High contrast separator margins are not consistent with other themes

sbatten

Can you clarify the todo above?

We should match the menu rounded corners in the menubar buttons as well. This is notepad in win11

How did we choose the corner radius? On windows 11, I find the native UI a lot more round, which maybe we could match?

daviddossett · 2022-05-12T15:35:58Z

Can you clarify the todo above?

This rule prevented the behavior in the gif below. A side effect of that rule was a that it rendered an unwanted thin border around all menu items on any theme. I'm trying to scope it to only hc themes like this to prevent the focus outline janking the layout around, but it doesn't seem to work. Could use your input here.

How did we choose the corner radius? On windows 11, I find the native UI a lot more round, which maybe we could match?

It's roughly similar to what we use for our icon buttons. I'll take a closer look at Win 11 for inspiration. Will also look at the menu buttons themselves.

Are you intentionally making the keyboard shortcuts use disabled styling when not focused?

They are using descriptionForeground to be less prominent on purpose. Looks like not every theme maintains great contrast though, so I'll see what I can do. Will fix the hc separator padding as well.

daviddossett · 2022-05-12T18:56:28Z

Many thanks to @sbatten for helping our with the high contrast layout shift fix 🙏

I also fixed the following:

Border radius applies to all themes and matches icon button @ 5px
Box shadow respects border-radius
Separators maintain correct width and margin/padding
Shortcut uses opacity value to appear less prominent unless parent irem is hovered/focused

CleanShot.2022-05-12.at.11.54.47.mp4

src/vs/base/browser/ui/actionbar/actionbar.css

src/vs/platform/contextview/browser/contextMenuHandler.css

fix opacity for separators in menus

* Initial updates * Add border radius * Address PR feedback * Fix typo * Update shadow blur * Update LR padding and use description foreground for shortcuts * Typo * Fix separator padding/margin * fix jumpy items in hc themes * Fix shadow and border radius * Use opacity for keybinding for better color blend * Update min width and container padding T/B * Revert actionbar margin and remove unnecessary menu css file * Ensure menus respect 0 horizontal margin rule * set bg/fg color on menu container * better fix for jumpy menu items * use outline instead of border * clean up dead css in style.css fix opacity for separators in menus * bring back vertical action bar margins * Remove old CSS import Co-authored-by: SteVen Batten <sbatten@microsoft.com>

daviddossett added 2 commits May 10, 2022 13:33

Initial updates

3f4553c

Add border radius

7a39bf4

daviddossett added this to the May 2022 milestone May 10, 2022

daviddossett requested review from sbatten and miguelsolorio May 10, 2022 20:53

daviddossett self-assigned this May 10, 2022

miguelsolorio suggested changes May 11, 2022

View reviewed changes

extensions/theme-defaults/themes/dark_vs.json Outdated Show resolved Hide resolved

src/vs/platform/theme/common/colorRegistry.ts Outdated Show resolved Hide resolved

daviddossett added 5 commits May 11, 2022 09:41

Address PR feedback

4356b5b

Fix typo

c4140e7

Update shadow blur

2ed9c4d

Update LR padding and use description foreground for shortcuts

2046856

Typo

02e3b31

miguelsolorio previously approved these changes May 11, 2022

View reviewed changes

sbatten requested changes May 12, 2022

View reviewed changes

Fix separator padding/margin

1161226

daviddossett dismissed miguelsolorio’s stale review via 1161226 May 12, 2022 17:59

sbatten self-assigned this May 12, 2022

sbatten and others added 2 commits May 12, 2022 11:41

fix jumpy items in hc themes

ff6d082

Fix shadow and border radius

dc1bc27

daviddossett added 2 commits May 12, 2022 12:09

Use opacity for keybinding for better color blend

9481c2c

Update min width and container padding T/B

739fde1

jrieken previously approved these changes May 13, 2022

View reviewed changes

sbatten reviewed May 16, 2022

View reviewed changes

src/vs/base/browser/ui/actionbar/actionbar.css Outdated Show resolved Hide resolved

sbatten reviewed May 16, 2022

View reviewed changes

src/vs/platform/contextview/browser/contextMenuHandler.css Outdated Show resolved Hide resolved

Revert actionbar margin and remove unnecessary menu css file

82f1c59

daviddossett dismissed jrieken’s stale review via 82f1c59 May 16, 2022 19:01

daviddossett and others added 6 commits May 16, 2022 12:12

Ensure menus respect 0 horizontal margin rule

7e8791b

set bg/fg color on menu container

266834d

better fix for jumpy menu items

32cbf28

use outline instead of border

17f7560

clean up dead css in style.css

46c4500

fix opacity for separators in menus

bring back vertical action bar margins

04ce765

sbatten previously approved these changes May 17, 2022

View reviewed changes

Remove old CSS import

aa4b46e

daviddossett dismissed sbatten’s stale review via aa4b46e May 17, 2022 01:37

sbatten self-requested a review May 17, 2022 01:38

sbatten approved these changes May 17, 2022

View reviewed changes

daviddossett merged commit 5f3e9c1 into main May 17, 2022

daviddossett deleted the ddossett/custom-menu-styles branch May 17, 2022 01:40

This was referenced Jun 13, 2022

Can you clarify the todo above? #151965

Closed

Feature request: add config option for the new menu styles #151968

Closed

github-actions bot locked and limited conversation to collaborators Jul 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update custom menu styles #149187

Update custom menu styles #149187

daviddossett commented May 10, 2022 •

edited by sbatten

Loading

miguelsolorio left a comment

sbatten commented May 12, 2022

sbatten commented May 12, 2022

sbatten commented May 12, 2022

sbatten left a comment

daviddossett commented May 12, 2022 •

edited

Loading

daviddossett commented May 12, 2022 •

edited

Loading

Update custom menu styles #149187

Update custom menu styles #149187

Conversation

daviddossett commented May 10, 2022 • edited by sbatten Loading

Before

After

miguelsolorio left a comment

Choose a reason for hiding this comment

sbatten commented May 12, 2022

sbatten commented May 12, 2022

sbatten commented May 12, 2022

sbatten left a comment

Choose a reason for hiding this comment

daviddossett commented May 12, 2022 • edited Loading

daviddossett commented May 12, 2022 • edited Loading

daviddossett commented May 10, 2022 •

edited by sbatten

Loading

daviddossett commented May 12, 2022 •

edited

Loading

daviddossett commented May 12, 2022 •

edited

Loading