1381117 - Identity icon should have a darker hover state

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Johann Hofmann [:johannh]]

According to the interactive mockup (http://design.firefox.com/people/shorlander/photon/Mockups/macOS.html) the identity icon should get a little darker when hovering the identity block. It currently does not, at least on my machine.

Comment 1

[Dão Gottwald [:dao]]

Attachment: Bug 1381117 - Increase opacity of icons in the identity block on hover.

Review commit: https://reviewboard.mozilla.org/r/157578/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/157578/

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 8886789 [details]
Bug 1381117 - Increase opacity of icons in the identity block on hover.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/157578/diff/1-2/

Comment 3

[Johann Hofmann [:johannh]]

Comment on attachment 8886789 [details]
Bug 1381117 - Increase opacity of icons in the identity block on hover.

https://reviewboard.mozilla.org/r/157578/#review162846

::: browser/themes/shared/identity-block/connection-mixed-active-loaded.svg:1
(Diff revision 2)
>  <?xml version="1.0" encoding="utf-8"?>

You can remove the xml declarations.

Comment 4

[Johann Hofmann [:johannh]]

Sorry, the r- was not because of the xml declarations, but rather because the notification icons are not colored correctly, you probably need to add context-fill-opacity here:

https://searchfox.org/mozilla-central/source/browser/themes/shared/notification-icons.svg#4

mozreview was crashing on me

Comment 5

[Dão Gottwald [:dao]]

(In reply to Johann Hofmann [:johannh] from comment #3)
> Comment on attachment 8886789 [details]
> Bug 1381117 - Increase opacity of icons in the identity block on hover.
> 
> https://reviewboard.mozilla.org/r/157578/#review162846
> 
> ::: browser/themes/shared/identity-block/connection-mixed-active-loaded.svg:1
> (Diff revision 2)
> >  <?xml version="1.0" encoding="utf-8"?>
> 
> You can remove the xml declarations.

I'll leave that for another bug. Doing one drive-by cleanup doesn't mean all of it should happen here. ;)

Comment 6

[Dão Gottwald [:dao]]

Comment on attachment 8886789 [details]
Bug 1381117 - Increase opacity of icons in the identity block on hover.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/157578/diff/2-3/

Comment 7

[Johann Hofmann [:johannh]]

Comment on attachment 8886789 [details]
Bug 1381117 - Increase opacity of icons in the identity block on hover.

https://reviewboard.mozilla.org/r/157578/#review163384

Looks good, thanks.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d43602ea02ec
Increase opacity of icons in the identity block on hover. r=johannh

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d43602ea02ec

Comment 10

[Matthew N. [:MattN]]

This bug seemed to be about the identity block but also changed the icon darkness within the control center panel, was that intentional?

https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=5e73b9798464c3f7106f0161dc9a49b234f42f9c&newProject=mozilla-central&newRev=8ff4f17b266db9a780efe06f7fbdae629e49f5bc&filter=controlCenter

Comment 11

[Dão Gottwald [:dao]]

(In reply to Matthew N. [:MattN] (huge backlog; PM if requests are blocking you) from comment #10)
> This bug seemed to be about the identity block but also changed the icon
> darkness within the control center panel, was that intentional?
> 
> https://screenshots.mattn.ca/compare/?oldProject=mozilla-
> central&oldRev=5e73b9798464c3f7106f0161dc9a49b234f42f9c&newProject=mozilla-
> central&newRev=8ff4f17b266db9a780efe06f7fbdae629e49f5bc&filter=controlCenter

Yes, updated for consistency.

Comment 12

[Deac Alin]

Verified on Windows 10 x 64 and MAC OSX 10.12.5 on latest Nightly 57.0a1. Increased the opacity of icons in the identity block on hover.Looks good. Marked as verified fixed. Thank you