Implement new toolbar button hover and active background styles

VERIFIED FIXED in Firefox 55

Status

()

enhancement
P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: nhnt11, Assigned: nhnt11)

Tracking

Trunk
Firefox 55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed, firefox57 verified)

Details

(Whiteboard: [photon-visual][p1][57])

Attachments

(1 attachment)

No description provided.
Assignee: nobody → nhnt11
Status: NEW → ASSIGNED
Iteration: --- → 55.5 - May 15
Flags: qe-verify?
Comment on attachment 8866514 [details]
Bug 1363842 - Implement new toolbar button hover and active background styles.

https://reviewboard.mozilla.org/r/138138/#review141664

::: browser/themes/osx/browser.css:43
(Diff revision 2)
>    --toolbarbutton-hover-boxshadow: 0 1px 0 hsla(0,0%,100%,.5),
>                                     0 1px 0 hsla(0,0%,100%,.5) inset;
>  
> +%ifndef MOZ_PHOTON_THEME
>    --toolbarbutton-active-background: hsla(0,0%,0%,.02) linear-gradient(hsla(0,0%,0%,.12), transparent) border-box;
> +%endif

Please reorder the above lines such that you have this only once:

%ifdef MOZ_PHOTON_THEME
  ...
%else
  ...
%endif

::: browser/themes/osx/browser.css
(Diff revision 2)
> -  }
> -  #forward-button:-moz-window-inactive > .toolbarbutton-icon {
> -    box-shadow: 0 1px 0 0 rgba(0,0,0,0.2) inset,
> -                0 -1px 0 0 rgba(0,0,0,0.2) inset !important;
> -  }
> -}

We'll want to keep this in an %ifndef MOZ_PHOTON_THEME

::: browser/themes/shared/toolbarbuttons.inc.css:36
(Diff revision 2)
> +%endif
>    --toolbarbutton-hover-bordercolor: rgba(0,0,0,.2);
>  
> +%ifndef MOZ_PHOTON_THEME
>    --toolbarbutton-active-background: rgba(70%,70%,70%,.25);
> +%endif

See my comment on browser/themes/osx/browser.css

::: browser/themes/windows/browser.css:37
(Diff revision 2)
>    --toolbarbutton-hover-bordercolor: rgba(0,0,0,.2);
>    --toolbarbutton-hover-boxshadow: none;
>  
> +%ifndef MOZ_PHOTON_THEME
>    --toolbarbutton-active-background: rgba(0,0,0,.15);
> +%endif

ditto
Attachment #8866514 - Flags: review?(dao+bmo)
Comment on attachment 8866514 [details]
Bug 1363842 - Implement new toolbar button hover and active background styles.

https://reviewboard.mozilla.org/r/138138/#review143012
Attachment #8866514 - Flags: review?(dao+bmo) → review+
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/374e39d64190
Implement new toolbar button hover and active background styles. r=dao
Iteration: 55.5 - May 15 → 55.6 - May 29
Flags: qe-verify? → qe-verify+
QA Contact: brindusa.tot
https://hg.mozilla.org/mozilla-central/rev/374e39d64190
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
No longer blocks: 1365572
Depends on: 1367047
Depends on: 1368500
Hi Nihanth,

Can you please help us here and tell us what we need to test? Thanks
Flags: needinfo?(nhnt11)
QA Contact: brindusa.tot → ovidiu.boca
(In reply to ovidiu boca[:Ovidiu] from comment #8)
> Hi Nihanth,
> 
> Can you please help us here and tell us what we need to test? Thanks

The background of toolbar buttons when the mouse is moved over them and when they are clicked should match the spec (http://design.firefox.com/people/shorlander/photon/Mockups/macOS.html, this particular styling should be consistent across platforms)
Flags: needinfo?(nhnt11)
Hi Nihanth,

Thanks for your help. I retested and on Mac OS X 10.10 and 10.12 the actual results match the specification. On Windows, I have other colours for the same actions. When the mouse is over: #515152 and when the button is clicked: #656566 and the expected from the spec are, when the mouse is over: #E1E1E2 and when the button is clicked: #D5D5D6. We used xscope to determine the colour code. Please tell us your opinion. Thanks
Flags: needinfo?(nhnt11)
(In reply to ovidiu boca[:Ovidiu] from comment #10)
> Hi Nihanth,
> 
> Thanks for your help. I retested and on Mac OS X 10.10 and 10.12 the actual
> results match the specification. On Windows, I have other colours for the
> same actions. When the mouse is over: #515152 and when the button is
> clicked: #656566 and the expected from the spec are, when the mouse is over:
> #E1E1E2 and when the button is clicked: #D5D5D6. We used xscope to determine
> the colour code. Please tell us your opinion. Thanks

Interesting. As far as I know the toolbar background color is the same on Windows and MacOS, so I don't know why this is happening. I'll check on my Windows laptop later this evening.
I double checked this and the color values for the background are identical on macOS and Windows: rgba(12, 12, 13, 0.1) for hover and rgba(12, 12, 13, 0.15) for active.

The toolbar background is the same too, so I'm not sure why the colours are different. Anyway, I don't think we will change this, because the colours are opacity-based and programmatically, they match the spec.
Flags: needinfo?(nhnt11)
I can confirm that the toolbar background is the same. The difference appears on the toolbar buttons when the mouse is over or when the buttons are clicked. But from what I see this bug deals the toolbar background, correct?
Flags: needinfo?(nhnt11)
(In reply to ovidiu boca[:Ovidiu] from comment #13)
> I can confirm that the toolbar background is the same. The difference
> appears on the toolbar buttons when the mouse is over or when the buttons
> are clicked. But from what I see this bug deals the toolbar background,
> correct?

No, this bug deals with the toolbar buttons, when the mouse is over or when they are clicked. I'll check again, but I think that code-wise, things are correct. I'm not sure why the actual computed colour might be varying (on my machines, it's fine). I'll check again and also bring it up in the photon-visual meeting today.
Flags: needinfo?(nhnt11)
(In reply to Nihanth Subramanya [:nhnt11] from comment #14)
> No, this bug deals with the toolbar buttons, when the mouse is over or when
> they are clicked. I'll check again, but I think that code-wise, things are
> correct. I'm not sure why the actual computed colour might be varying (on my
> machines, it's fine). I'll check again and also bring it up in the
> photon-visual meeting today.

Nihanth, did you get a chance to look at this?
Flags: needinfo?(nhnt11)
I verified this on the latest Nightly 57.0a1(2017-09-14) and I can't reproduce it anymore. Based on this and comment 14 I will mark this as verified fixed.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(nhnt11)
You need to log in before you can comment on or make changes to this bug.