New issue
Advanced search Search tips

Issue 864594 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jul 18
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug



Sign in to add a comment

Fix wonky focus rings in omnibox

Project Member Reported by pbos@chromium.org, Jul 17

Issue description

There're some wonky things, also remember to check RTL, because that's wrong.
 
Labels: -Pri-2 Group-Avatar_Button Pri-1
Upgrading to P1
Labels: -Group-Avatar_Button
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5aaf72db7bb21a1c5d206f5c129f61b02d59478c

commit 5aaf72db7bb21a1c5d206f5c129f61b02d59478c
Author: Peter Boström <pbos@chromium.org>
Date: Wed Jul 18 14:53:41 2018

Match bookmark focus rings with their ink drops

Makes use of RoundRects similar to ToolbarButton and friends to match
the inkdrop look.

Bug:  chromium:864594 
Change-Id: I91a8edff35624561b1a299ce3fd838493088f578
Reviewed-on: https://chromium-review.googlesource.com/1141296
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576054}
[modify] https://crrev.com/5aaf72db7bb21a1c5d206f5c129f61b02d59478c/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d3df1e3bc46cc98bf2b74e4d137fe2ac6f51b4ed

commit d3df1e3bc46cc98bf2b74e4d137fe2ac6f51b4ed
Author: Peter Boström <pbos@chromium.org>
Date: Wed Jul 18 14:54:07 2018

Fix omnibox focus ring placement

* Makes the PageActionIconView use a RoundRect.
* Uses a mirrored rect for the security chip to fix placement in RTL.

Bug:  chromium:864594 
Change-Id: I726c79752bf8c8601dac994f0386567ac03c840d
Reviewed-on: https://chromium-review.googlesource.com/1141347
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576055}
[modify] https://crrev.com/d3df1e3bc46cc98bf2b74e4d137fe2ac6f51b4ed/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
[modify] https://crrev.com/d3df1e3bc46cc98bf2b74e4d137fe2ac6f51b4ed/chrome/browser/ui/views/page_action/page_action_icon_view.cc

Status: Fixed (was: Assigned)
Ring placement fixed, cosmetic separator issue filed in  issue 865029 .
Labels: Needs-Feedback
@Peter Boström: Could you please provide manual reproducible steps along with excepted and actual behaviour which helps us in verifying the fix from TE end.

Thanks!
Status: Verified (was: Fixed)
The focus rings should match the shapes of the hover effects for the bookmark star and the security chip on the left inside the location bar. This looks right to me on Mac Canary, so I'll mark it as fixed. TE doesn't really need to verify this one as it's sorta subtle.

Sign in to add a comment