MacViews material menus |
||||||||||||||
Issue descriptionImplement the material menu look & feel for MacViews menus. The mock is here: <https://drive.google.com/file/d/0BxMIIGI80eU-U1JzRVhrQjJscEU/view> I'm currently planning to implement the cross-platform mock as given with these Mac-specific deviations: 1) Use of Mac symbolic names for accelerators, so ⌘D instead of Cmd-D 2) Support for "increase contrast" by: a) Drawing a black border around the menu b) Increasing separator alpha from 15% to 100% c) Increasing minor text alpha from 40% to 100% d) Drawing a black border around the highlighted item 3) The ripple animation on click (which we don't use on Mac) replaced by the animation Mac menus use (flicker the selected item).
,
Apr 5 2018
1.) SGTM 2a.) I presume this means doing what other "menus" like Page info do today? (https://screenshot.googleplex.com/ApOrKrfE9MS)If so, that SGTM. If not, I'd need more explanation. 2b.) To match other separators in the Core UI[1], the current spec is Google Grey 900 at 16%, or #202124 at 16% 2c.) what is minor text? 2d.) need more info 3.) SGTM >> checkmark layout I prefer third image: no special indent, the checkbox starts "crowding" the label.
,
Apr 5 2018
Sorry, I was unclear about #2: I was proposing what we'd do when the system has the "Increase contrast" accessibility setting enabled, which causes strokes to be drawn in black rather than grey across the system. If you turn that setting on and look at how the tabstrip changes you can see what it looks like. #1 and #3 ack :)
,
Apr 5 2018
Three more things from chatting with bettes@ just now: 4) We're going to do 8pt rounded corners instead of 2pt on the menus 5) We're going to exempt combobox menus from the 320pt minimum 6) We're going to use the system highlight color (ala issue 829467 ) for hover, not always grey
,
Apr 5 2018
I snagged the system highlight blue and drew it at 17% alpha and I think it looks gorgeous. Screenshots of WIP CL <https://chromium-review.googlesource.com/c/chromium/src/+/998016> attached. Still need to figure out how to achieve #3 (the Mac dismissal animation).
,
Apr 6 2018
One other deviation I ran into while fixing <https://chromium-review.googlesource.com/c/chromium/src/+/998016>: 7) On Mac, comboboxes use checkmarks to denote the selected item.
,
Apr 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6c0aed57e2e803aadf029450180782e0471b8baa commit 6c0aed57e2e803aadf029450180782e0471b8baa Author: Elly Fong-Jones <ellyjones@chromium.org> Date: Fri Apr 06 16:37:05 2018 macviews: materialize macviews menus This change: 1) Implements the Material menu spec, as linked in the attached bug; 2) Adds support to NativeThemeMac for using Aqua highlights in Aqua mode; 3) Removes the logic in BridgedNativeWidget to support translucent/blurred menu windows, since Material menus are opaque Future work: 1) Removing MenuRunnerImplCocoa, which can be rendered dead; 2) Fixing the synchronous-ness of MacViews menus Bug: 829347 Change-Id: I6219edf82c36cc0380dc9073835d8071a2709496 Reviewed-on: https://chromium-review.googlesource.com/998016 Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#548806} [modify] https://crrev.com/6c0aed57e2e803aadf029450180782e0471b8baa/ui/native_theme/native_theme_mac.mm [modify] https://crrev.com/6c0aed57e2e803aadf029450180782e0471b8baa/ui/views/cocoa/bridged_native_widget.mm [modify] https://crrev.com/6c0aed57e2e803aadf029450180782e0471b8baa/ui/views/controls/menu/menu_config.cc [modify] https://crrev.com/6c0aed57e2e803aadf029450180782e0471b8baa/ui/views/controls/menu/menu_config.h [modify] https://crrev.com/6c0aed57e2e803aadf029450180782e0471b8baa/ui/views/controls/menu/menu_config_mac.mm [modify] https://crrev.com/6c0aed57e2e803aadf029450180782e0471b8baa/ui/views/controls/menu/menu_controller.h [modify] https://crrev.com/6c0aed57e2e803aadf029450180782e0471b8baa/ui/views/controls/menu/menu_item_view.cc [modify] https://crrev.com/6c0aed57e2e803aadf029450180782e0471b8baa/ui/views/controls/menu/menu_runner_impl_cocoa.mm
,
Apr 11 2018
,
Apr 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fb3061b1ebb591cdada694fa762f5c358176aeec commit fb3061b1ebb591cdada694fa762f5c358176aeec Author: Elly Fong-Jones <ellyjones@chromium.org> Date: Thu Apr 12 12:45:45 2018 views: support Mac menu closure animations This change: 1) Adds MenuClosureAnimationMac, which implements the Mac menu closure animation; 2) Adds logic to MenuItemView to force drawing the item in a selected or unselected state as needed, for use in this animation; 3) Adds support to MenuController for disabling handling input events, so that the closure animation can appear to be "synchronous"; 4) Makes MenuController::Accept possibly asynchronous; 5) Updates unit tests to account for (4) Bug: 829347 Change-Id: I8676718e6a5e9704b422ed4cb08e8d10002bf45a Reviewed-on: https://chromium-review.googlesource.com/999803 Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#550160} [modify] https://crrev.com/fb3061b1ebb591cdada694fa762f5c358176aeec/chrome/browser/ui/views/menu_test_base.cc [modify] https://crrev.com/fb3061b1ebb591cdada694fa762f5c358176aeec/chrome/browser/ui/views/toolbar/toolbar_action_view_interactive_uitest.cc [modify] https://crrev.com/fb3061b1ebb591cdada694fa762f5c358176aeec/ui/views/BUILD.gn [add] https://crrev.com/fb3061b1ebb591cdada694fa762f5c358176aeec/ui/views/controls/menu/menu_closure_animation_mac.h [add] https://crrev.com/fb3061b1ebb591cdada694fa762f5c358176aeec/ui/views/controls/menu/menu_closure_animation_mac.mm [modify] https://crrev.com/fb3061b1ebb591cdada694fa762f5c358176aeec/ui/views/controls/menu/menu_controller.cc [modify] https://crrev.com/fb3061b1ebb591cdada694fa762f5c358176aeec/ui/views/controls/menu/menu_controller.h [modify] https://crrev.com/fb3061b1ebb591cdada694fa762f5c358176aeec/ui/views/controls/menu/menu_controller_unittest.cc [modify] https://crrev.com/fb3061b1ebb591cdada694fa762f5c358176aeec/ui/views/controls/menu/menu_host_root_view.cc [modify] https://crrev.com/fb3061b1ebb591cdada694fa762f5c358176aeec/ui/views/controls/menu/menu_host_root_view.h [modify] https://crrev.com/fb3061b1ebb591cdada694fa762f5c358176aeec/ui/views/controls/menu/menu_item_view.cc [modify] https://crrev.com/fb3061b1ebb591cdada694fa762f5c358176aeec/ui/views/controls/menu/menu_item_view.h [modify] https://crrev.com/fb3061b1ebb591cdada694fa762f5c358176aeec/ui/views/controls/menu/menu_runner_unittest.cc [modify] https://crrev.com/fb3061b1ebb591cdada694fa762f5c358176aeec/ui/views/test/menu_test_utils.cc [modify] https://crrev.com/fb3061b1ebb591cdada694fa762f5c358176aeec/ui/views/test/menu_test_utils.h
,
Apr 12 2018
"Increase contrast" disabled & enabled for <https://chromium-review.googlesource.com/#/c/chromium/src/+/1010245>
,
Apr 12 2018
I asked lpalmaro@ (a11y PM) for feedback on #10. She suggested two further changes: 1) 2pt separator lines instead of 1pt 2) Bold font for the selected item I'll do those in a followup CL, since sorting out the font could be hard - Mac doesn't have a bold menu system font.
,
Apr 12 2018
This bug is in a good place at the moment: The default behaviors are implemented. High contrast is here: <https://chromium-review.googlesource.com/c/chromium/src/+/1010245> but does not block M67. I'm kicking the remainder of this bug to M68.
,
Apr 13 2018
,
Apr 13 2018
Issue 832316 has been merged into this issue.
,
Apr 13 2018
Issue 832665 has been merged into this issue.
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a0d2adffc4857d639c39c9f5c01c32cc5fc932a7 commit a0d2adffc4857d639c39c9f5c01c32cc5fc932a7 Author: Elly Fong-Jones <ellyjones@chromium.org> Date: Tue Apr 17 14:36:58 2018 macviews: support high contrast in menus This change: 1) Draws exterior borders on menus, even if MenuConfig says not to use them, if the NativeTheme is in high contrast mode; 2) Draws separators in black instead of grey in high contrast mode on Mac; 3) Draws a heavy (2pt) border around the selected item in high contrast mode on Mac Screenshots are attached to the bug as comment #10. Bug: 829347 Change-Id: I8ffd7adbae5b35751c18b77a9e8b6564926f7012 Reviewed-on: https://chromium-review.googlesource.com/1010245 Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#551324} [modify] https://crrev.com/a0d2adffc4857d639c39c9f5c01c32cc5fc932a7/ui/native_theme/native_theme_mac.h [modify] https://crrev.com/a0d2adffc4857d639c39c9f5c01c32cc5fc932a7/ui/native_theme/native_theme_mac.mm [modify] https://crrev.com/a0d2adffc4857d639c39c9f5c01c32cc5fc932a7/ui/views/controls/menu/menu_scroll_view_container.cc
,
Apr 17 2018
,
Apr 18 2018
Tested the issue on latest chrome version 68.0.3399.0 using Mac 10.12.6 with steps mentioned below: 1) Launched chrome version and enabled MacViews-Browser flag in chrome://flags 2) Observed the UI of Three-dot menu, Combo box and Bookmarks bar as per screenshots provided in comment#5 @Reporter: Please find the attached screenshots for your reference and help us in verifying the fix. Thanks!
,
May 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/53a5165fc4916460625ea6bd3eb7953fd5a5e0ab commit 53a5165fc4916460625ea6bd3eb7953fd5a5e0ab Author: Elly Fong-Jones <ellyjones@chromium.org> Date: Mon May 21 15:38:59 2018 views: reduce corner radius on context menus Context menus and combobox menus are both "auxiliary" in the UI sense, so they have a reduced corner radius to fit better with their parent controls. Bug: 829347 Change-Id: Ie0ecfcbab5161071d5cf61cc8e7647172122dad5 Reviewed-on: https://chromium-review.googlesource.com/1066470 Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#560275} [modify] https://crrev.com/53a5165fc4916460625ea6bd3eb7953fd5a5e0ab/ui/views/controls/menu/menu_config.cc [modify] https://crrev.com/53a5165fc4916460625ea6bd3eb7953fd5a5e0ab/ui/views/controls/menu/menu_config.h [modify] https://crrev.com/53a5165fc4916460625ea6bd3eb7953fd5a5e0ab/ui/views/controls/menu/menu_config_mac.mm [modify] https://crrev.com/53a5165fc4916460625ea6bd3eb7953fd5a5e0ab/ui/views/controls/menu/menu_controller.cc [modify] https://crrev.com/53a5165fc4916460625ea6bd3eb7953fd5a5e0ab/ui/views/controls/menu/menu_controller.h
,
May 22 2018
ellyjones@ Could you please help us with the repro steps to verify the fix in comment #19 Thank You...
,
May 22 2018
,
Jun 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/79c8511fb8801f7fc23c4cbe81311b6065daef15 commit 79c8511fb8801f7fc23c4cbe81311b6065daef15 Author: Erik Chen <erikchen@chromium.org> Date: Tue Jun 12 16:46:57 2018 Remove dead code: MenuRunnerImplCocoa Bug: 829347 Change-Id: I56275cb690dc9aba050ae62ff3ca403dd4a601ba Reviewed-on: https://chromium-review.googlesource.com/1087349 Commit-Queue: Erik Chen <erikchen@chromium.org> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#566462} [modify] https://crrev.com/79c8511fb8801f7fc23c4cbe81311b6065daef15/ui/views/BUILD.gn [modify] https://crrev.com/79c8511fb8801f7fc23c4cbe81311b6065daef15/ui/views/controls/menu/menu_runner_cocoa_unittest.mm [modify] https://crrev.com/79c8511fb8801f7fc23c4cbe81311b6065daef15/ui/views/controls/menu/menu_runner_impl.cc [delete] https://crrev.com/f39d086cbdbb1c00307912f6eadf1556fb0d99cb/ui/views/controls/menu/menu_runner_impl_cocoa.h [delete] https://crrev.com/f39d086cbdbb1c00307912f6eadf1556fb0d99cb/ui/views/controls/menu/menu_runner_impl_cocoa.mm
,
Jul 3
Hi, is there any possibility to reconsider the design decision for the spacing between the checkmark and label? The context menu is already very spacious. It has a lot of padding between menu items and the between the edge of the context menu and its contents. I feel that making the label nest right beside the checkmark does not take full advantage of the amount of space available. So everything in the context menu is very spread apart, except the checkmark which makes it stand out in a bad way. Lots of space on the left but almost none on the right. This is just my initial feedback on the MacViews design after using it for a while. By the way, what was the reason for keeping the checkmark and label "crowded"?
,
Jul 3
FYI: I attached c#23 and c#24 out of this closed report to issue 832257 , so this can decided there.
,
Jul 6
#23: Yes - some other history came to light about menu spacing, so I filed issue 860673. Please feel free to star or follow that bug.
,
Jul 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fb02feee251aa6be7ccc2d180167ae9fb6cdd2d1 commit fb02feee251aa6be7ccc2d180167ae9fb6cdd2d1 Author: Avi Drissman <avi@chromium.org> Date: Tue Jul 31 17:02:06 2018 Remove unused definitions. Their use was removed in 6c0aed57e2e803aadf029450180782e0471b8baa. BUG= 829347 Change-Id: Ia5e99e49ba53cb4a416fda37407fdf5faa516f27 Reviewed-on: https://chromium-review.googlesource.com/1156845 Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Commit-Queue: Avi Drissman <avi@chromium.org> Cr-Commit-Position: refs/heads/master@{#579440} [modify] https://crrev.com/fb02feee251aa6be7ccc2d180167ae9fb6cdd2d1/ui/views/cocoa/bridged_native_widget.mm |
||||||||||||||
►
Sign in to add a comment |
||||||||||||||
Comment 1 by ellyjo...@chromium.org
, Apr 5 2018