Closed Bug 1363842 Opened 4 years ago Closed 4 years ago

Implement new toolbar button hover and active background styles

Categories

(Firefox :: Theme, enhancement, P1)

enhancement

Tracking

()

VERIFIED FIXED
Firefox 55
Iteration:
55.6 - May 29
Tracking Status
firefox55 --- fixed
firefox57 --- verified

People

(Reporter: nhnt11, Assigned: nhnt11)

References

Details

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

Attachments

(1 file)

No description provided.
Assignee: nobody → nhnt11
Status: NEW → ASSIGNED
Blocks: 1363869
Iteration: --- → 55.5 - May 15
Flags: qe-verify?
Depends on: 1363840
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)
Blocks: 1365195
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: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Blocks: 1365572
No longer blocks: 1365572
Blocks: 1365901
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.