Closed Bug 1296331 Opened 4 years ago Closed 3 years ago

Fix hover style of messageCloseButton in toolbox-notificationbox

Categories

(DevTools :: General, defect, P2)

defect

Tracking

(firefox50 affected, firefox51 affected)

RESOLVED FIXED
Tracking Status
firefox50 --- affected
firefox51 --- affected

People

(Reporter: magicp.jp, Unassigned)

References

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20160818030226

Steps to reproduce:

1. Start Nightly
2. Go to about:home
3. Open DevTools > Canvas
4. Reload and Recording > toolbox-notificationbox will be displayed


Actual results:

- Hover style is different from gcli global-notificationbox close icon
- X is not center

Regression range:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=cf23addcfa4d9224a7d186481ecfb48194bb37be&tochange=0cb2c7646aa072218accaf14615a2d4ba4408133


Expected results:

Hover style is same as gcli global-notificationbox close icon
Regression range for hover style (without centering)
Blocks: 1288963
Has Regression Range: --- → yes
Has STR: --- → yes
Component: Untriaged → Developer Tools
OS: Unspecified → All
Hardware: Unspecified → All
The new hover style is intentional, so we avoid inverting the icon on hover and active. Helen, are you fine with the new hover state, or should I revert the change?

About the centering, it's a regression of bug 1289506, I can fix that, the background position of the icon needs to change from 4px to center.
Flags: needinfo?(hholmes)
For the centering, the svg is now well centered, but the css that goes with it needs to be changed.
(In reply to Tim Nguyen :ntim (use needinfo?) from comment #2)
> The new hover style is intentional, so we avoid inverting the icon on hover
> and active. Helen, are you fine with the new hover state, or should I revert
> the change?

magicp, is the screenshot you attached with the bug a hover state? It does seem rather light for a hover state.
Flags: needinfo?(hholmes)
(In reply to Helen V. Holmes (:helenvholmes) (:✨)(pls ni?) from comment #4)

> magicp, is the screenshot you attached with the bug a hover state? It does
> seem rather light for a hover state.

It is a hover state of bug. Did you reproduce this?
(In reply to Helen V. Holmes (:helenvholmes) (:✨)(pls ni?) from comment #4)
> (In reply to Tim Nguyen :ntim (use needinfo?) from comment #2)
> > The new hover style is intentional, so we avoid inverting the icon on hover
> > and active. Helen, are you fine with the new hover state, or should I revert
> > the change?
> 
> magicp, is the screenshot you attached with the bug a hover state? It does
> seem rather light for a hover state.

I can darken it if that's a problem (or even revert it, although I want to avoid that so we can keep the code simple and avoid inverting the icon on hover).
(In reply to Tim Nguyen :ntim (use needinfo?) from comment #6)
> (In reply to Helen V. Holmes (:helenvholmes) (:✨)(pls ni?) from comment #4)
> > (In reply to Tim Nguyen :ntim (use needinfo?) from comment #2)
> > > The new hover style is intentional, so we avoid inverting the icon on hover
> > > and active. Helen, are you fine with the new hover state, or should I revert
> > > the change?
> > 
> > magicp, is the screenshot you attached with the bug a hover state? It does
> > seem rather light for a hover state.
> 
> I can darken it if that's a problem (or even revert it, although I want to
> avoid that so we can keep the code simple and avoid inverting the icon on
> hover).

Looks like we can just center it and call it a day, I think.
See Also: → 1276675
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.