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.

Sign up for GitHub

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

Jump to bottom

Hover/Focus effect on tab close button is too small #7276

Closed
simonhong opened this issue Dec 9, 2019 · 2 comments · Fixed by brave/brave-core#4178
Closed

Hover/Focus effect on tab close button is too small #7276

simonhong opened this issue Dec 9, 2019 · 2 comments · Fixed by brave/brave-core#4178
Assignees
@simonhong
Labels
QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-Plan-Specified QA/Yes regression release-notes/exclude
Milestone
1.1.x - Release

Comments

@simonhong
Copy link
Member

simonhong commented Dec 9, 2019 •
edited by bsclifton
Loading

Test plan

Hovering over the close button should look like this:
Screen Shot 2019-12-09 at 10 39 04 AM

NOT like this:
Screen Shot 2019-12-09 at 10 38 40 AM

Description

Screen Shot 2019-12-09 at 8 31 56 PM

This looks regression from C79 bump.
Only 1.0 is fine. 1.2, 1.3 and 1.4 all have this issue.
1.1 could have same issue.
@simonhong simonhong added this to the 1.4.x - Nightly milestone Dec 9, 2019
@simonhong simonhong self-assigned this Dec 9, 2019
simonhong added a commit to brave/brave-core that referenced this issue Dec 9, 2019
@simonhong
Fix hover/focus circle is too small in tab close button
87a90bc 
After c79 bump, close button uses ink drop for hover effect and
highlight generator for focus ring.
Both uses LayoutProvider::GetCornerRadiusMetric() with EMPHASIS_MAXIMUM
for calculating radius for their circle shape effect.
However, we modified to return 4 always. Because of this, small circle
was drawn. So, orignal maximum radius calculation logic is applied
by subclassing TabCloseButton.

Fix brave/brave-browser#7276
@simonhong simonhong mentioned this issue Dec 9, 2019
Merged
32 tasks
@bsclifton
Copy link
Member

Pulling into 1.1 since this is a regression

@LaurenWags
Copy link
Member

LaurenWags commented Dec 10, 2019 •
edited by srirambv
Loading

Verified passed with

Brave 1.1.20 Chromium: 79.0.3945.74 (Official Build) (64-bit)
Revision 99b6d241cf84f7fa7434ae8a59f9e74eb1464280-refs/branch-heads/3945@{#888}
OS macOS Version 10.13.6 (Build 17G5019)

Screen Shot 2019-12-10 at 8 48 10 AM

Screen Shot 2019-12-10 at 8 48 24 AM

Verification passed on

Brave 1.1.20 Chromium: 79.0.3945.74 (Official Build) (64-bit)
Revision 99b6d241cf84f7fa7434ae8a59f9e74eb1464280-refs/branch-heads/3945@{#888}
OS Ubuntu 18.04 LTS

image
image

Verification passed on

Brave 1.1.20 Chromium: 79.0.3945.74 (Official Build) (64-bit)
Revision 99b6d241cf84f7fa7434ae8a59f9e74eb1464280-refs/branch-heads/3945@{#888}
OS Windows 10 OS Version 1803 (Build 17134.228)

image
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@simonhong simonhong

Labels
QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-Plan-Specified QA/Yes regression release-notes/exclude
Projects
None yet
Milestone
1.1.x - Release
Development

Successfully merging a pull request may close this issue.

Fix hover/focus circle is too small in tab close button brave/brave-core
5 participants
@bsclifton @simonhong @srirambv @LaurenWags @btlechowski