get rid of all dashed focus rects |
||||||
Issue descriptionWe still use dashed focus rects in a few places in native chrome ui, largely as the default in some Views base classes. All the top chrome has been updated not to use dashed rects (mostly we use the ink drop hover effect). Other places have been updated to use a blue ring. examples: - checkboxes show both a blue rect around the box and a dashed rect around the text. Compare to chrome://settings, which only shows the former. We should match that. - label buttons default to using a dashed rect. Almost all label buttons are using ink drops now, but some like those in the profile bubble still use the default dashed lines. - links still use dashed lines. Links in blink and in chrome:// pages don't; we should use a ring instead. I think Harmony addresses all of these but since there's no solid delivery date that I know of for that project, as a stopgap it would be nice to remove the dashed lines already.
,
Mar 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/525950cc7033074e1a1d3e44e7de0a966345460c commit 525950cc7033074e1a1d3e44e7de0a966345460c Author: estade <estade@chromium.org> Date: Wed Mar 08 18:34:52 2017 Remove dashed focus rect from Views checkboxes. This makes our native checkboxes match our web checkboxes, i.e. focus is indicated only by a blue square around the box on the left. BUG=696003 Review-Url: https://codereview.chromium.org/2734273002 Cr-Commit-Position: refs/heads/master@{#455496} [modify] https://crrev.com/525950cc7033074e1a1d3e44e7de0a966345460c/ui/views/controls/button/checkbox.cc [modify] https://crrev.com/525950cc7033074e1a1d3e44e7de0a966345460c/ui/views/controls/button/checkbox.h
,
Mar 8 2017
X-reffing video for https://codereview.chromium.org/2736613008/
,
Mar 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e5ec3568878feddb6978cb2a352e1226ccb74bd2 commit e5ec3568878feddb6978cb2a352e1226ccb74bd2 Author: estade <estade@chromium.org> Date: Thu Mar 16 18:14:32 2017 Improve appearance of higlighting in profile chooser view. 1. Use NativeTheme menu colors for selected rows on all platforms. This allows us to remove platform ifdefs (only Linux did this before). This has almost no visible effect on a classic theme as the theme menu colors were almost identical to the colors being used in the profile chooser. One thing that does change is that the color of the hovered text is a smidgeon darker, improving contrast. 2. Don't draw a dashed focus rect, instead reuse selection. This matches how menus look. Also make focus actually follow the mouse, also matching how menus work. BUG=696003 Review-Url: https://codereview.chromium.org/2736613008 Cr-Commit-Position: refs/heads/master@{#457494} [modify] https://crrev.com/e5ec3568878feddb6978cb2a352e1226ccb74bd2/chrome/browser/ui/views/profiles/profile_chooser_view.cc
,
Mar 28 2017
notes to self: - links still use dashed rects and will need to until harmony is on - the buttons in the page info popup (PermissionMenuButton) are MenuButtons that rely on dashed focus rects.
,
Nov 7
The main blocker of this is out of the way, which is that we don't support pre-Harmony UI. Over to bsep@ to find an owner. We're still applying a dashed focus rect in ImageButton, although by default it's never shown because ImageButtons aren't focusable, and will only be shown after a call to SetFocusForPlatform. For example, CastDialogNoSinksView does this. That and other code should probably be updated to use ink drops or to apply ink drops by default.
,
Dec 18
,
Dec 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fa532814ee43bb65b6870511f9665a8ae6e85329 commit fa532814ee43bb65b6870511f9665a8ae6e85329 Author: Peter Boström <pbos@chromium.org> Date: Tue Dec 18 21:24:55 2018 Remove DashedFocusPainter These were only used from {ImageButton,LabelButton} and should nominally either be highlighted using the ink drop or FocusRing. Remaining visible uses of DashedFocusPainter, if any, are bugs. PermissionMenuButton which is the last known use of dashed focus have been replaced with HoverButton that does not make use of it. Bug: chromium:696003 Change-Id: Ie33468e542aad58c75a66ae5fa9dc451513f7faa Reviewed-on: https://chromium-review.googlesource.com/c/1381003 Reviewed-by: Michael Wasserman <msw@chromium.org> Commit-Queue: Peter Boström <pbos@chromium.org> Cr-Commit-Position: refs/heads/master@{#617624} [modify] https://crrev.com/fa532814ee43bb65b6870511f9665a8ae6e85329/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc [modify] https://crrev.com/fa532814ee43bb65b6870511f9665a8ae6e85329/chrome/browser/ui/views/frame/hosted_app_menu_button.cc [modify] https://crrev.com/fa532814ee43bb65b6870511f9665a8ae6e85329/chrome/browser/ui/views/hover_button.cc [modify] https://crrev.com/fa532814ee43bb65b6870511f9665a8ae6e85329/chrome/browser/ui/views/media_router/cast_dialog_sink_button.cc [modify] https://crrev.com/fa532814ee43bb65b6870511f9665a8ae6e85329/chrome/browser/ui/views/tabs/new_tab_button.cc [modify] https://crrev.com/fa532814ee43bb65b6870511f9665a8ae6e85329/chrome/browser/ui/views/tabs/tab_close_button.cc [modify] https://crrev.com/fa532814ee43bb65b6870511f9665a8ae6e85329/chrome/browser/ui/views/toolbar/browser_app_menu_button.cc [modify] https://crrev.com/fa532814ee43bb65b6870511f9665a8ae6e85329/chrome/browser/ui/views/toolbar/toolbar_action_view.cc [modify] https://crrev.com/fa532814ee43bb65b6870511f9665a8ae6e85329/chrome/browser/ui/views/toolbar/toolbar_button.cc [modify] https://crrev.com/fa532814ee43bb65b6870511f9665a8ae6e85329/ui/views/controls/button/checkbox.cc [modify] https://crrev.com/fa532814ee43bb65b6870511f9665a8ae6e85329/ui/views/controls/button/image_button.cc [modify] https://crrev.com/fa532814ee43bb65b6870511f9665a8ae6e85329/ui/views/controls/button/image_button_factory.cc [modify] https://crrev.com/fa532814ee43bb65b6870511f9665a8ae6e85329/ui/views/controls/button/label_button.cc [modify] https://crrev.com/fa532814ee43bb65b6870511f9665a8ae6e85329/ui/views/controls/button/md_text_button.cc [modify] https://crrev.com/fa532814ee43bb65b6870511f9665a8ae6e85329/ui/views/painter.cc [modify] https://crrev.com/fa532814ee43bb65b6870511f9665a8ae6e85329/ui/views/painter.h
,
Dec 18
I'll consider this fixed with DashedFocusPainter's removal. views::Link::PaintFocusRing uses Canvas::DrawDashedRect so I don't think we're ready to remove that.
,
Dec 18
.. this explicitly mentions removing the dashed rect from PaintFocusRing. Reopening.
,
Jan 10
bsep@ to take the DashedFocusPainter parts, wip cl at crrev.com/c/1387955 fails to render a focus ring over them at chrome://crash |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by bugdroid1@chromium.org
, Feb 24 2017