1706148 - Badge arrows and Icons from the Menu notification banners are misaligned by a few pixels

Status: VERIFIED FIXED

(Firefox :: Menus, defect, P2)

Description

[Rares Doghi, Desktop QA]

Attachment: HighContrast_download_banner.png

[Affected platforms]:
Platforms: All

[Preconditions]:
Have the following prefs set in about:config - browser.proton.enabled = true

[Steps to reproduce]

1. Launch the Firefox browser and trigger the Update available - Download now Menu notification
2. Dismiss the panel and Open the Hamburger menu.

[Expected result]
The Icons themselves should be centered inside the Menu notification banner.
The "UP" Arrow inside the Icon should be centered.

[Actual result]
The Icon seems a pixel higher and not fully centered inside the Hamburger menu notification.
The UP arrow inside the icon seems a little bit to the right.
The Icon circle might not be entirely round, might be a little smaller on the left side. (More noticeable on the Information (i) icon)
I will attach some screenshots maybe they show what I mean a little better than what I am able to explain.

Comment 1

[Rares Doghi, Desktop QA]

Attachment: light_download_update_banner&badge.png

Comment 2

[Rares Doghi, Desktop QA]

Attachment: dark_hamburger_finish_account_banner.png

The Information circle icon looks like its a pixel smaller on the left side, making it look like its fading or maybe that it has a thinner line ?

Comment 3

[katieC]

Attachment: Light Theme Badges - mini UX spec

Icons for menu notifications look incorrect, possibly not yet landed in the UI.
[ Attaching a mini ux spec for reference ]

Can we confirm that for the app menu the correct badges are used?

App Menu Badges: [ 12 x 12 ]
• update-circle-fill-12.svg
• warning-fill-12.svg

...and that we have the right icons for menu notifications: [ 16 x 16 ]
• update-circle-fill.svg
• warning-fill.svg
• information-fill.svg

Thanks!

Comment 4

[Sam Foster [:sfoster] (he/him)]

Rather than shuffling things around pixel by pixel, it would be good to use the new assets. See Katie's spec in comment #3.

Comment 5

[Molly Howell (she/her) (mostly inactive)]

It looks like bug 1699889 landed the new icons but didn't switch these locations over to use them? I can do that here.

Comment 6

[Molly Howell (she/her) (mostly inactive)]

Sam, do you know if we have a 12x12 version of the new warning.svg? The 16px one shrunk to badge size isn't ideal, because the exclamation point detail is a little too small to remain easily readable. The new update and info icons do look good to me.

Comment 7

[Sam Foster [:sfoster] (he/him)]

(In reply to Molly Howell (she/her) [:mhowell] from comment #6)

Sam, do you know if we have a 12x12 version of the new warning.svg? The 16px one shrunk to badge size isn't ideal, because the exclamation point detail is a little too small to remain easily readable. The new update and info icons do look good to me.

Yeah, there is a warning-fill-12.svg in the google drive folder, made for this purpose. It needs both context-fill and context-stroke colors as it has an outline on it.
There is a badge icon also for update (update-circle-fill-12.svg) but I don't think I have a badge-sized info icon - what does that look like in context?

Comment 8

[Sam Foster [:sfoster] (he/him)]

(In reply to Sam Foster [:sfoster] (he/him) from comment #7)
an outline on it.

There is a badge icon also for update (update-circle-fill-12.svg) but I don't think I have a badge-sized info icon - what does that look like in context?

nm, I see the screenshot on comment #2. Let me ask around.

Comment 9

[Molly Howell (she/her) (mostly inactive)]

(In reply to Sam Foster [:sfoster] (he/him) from comment #7)

There is a badge icon also for update (update-circle-fill-12.svg) but I don't think I have a badge-sized info icon - what does that look like in context?

Sorry I wasn't clearer about that, I don't know of anywhere we use the info icon as a badge, only at 16px (as in comment 2).

Comment 10

[Sam Foster [:sfoster] (he/him)]

oh ok, sorry I'm just reading too fast. Yeah, the info icon in the menu item should be the regular 16x16 sized information-fill.svg

Comment 11

[Molly Howell (she/her) (mostly inactive)]

Okay. What I'll do then is add update-circle-fill-12.svg and warning-fill-12.svg in this bug. I don't think they have existing equivalents, we were just using the 16px ones as badges before. I'll also add those files to the spreadsheet since they don't have entries there at all right now.

Comment 12

[Molly Howell (she/her) (mostly inactive)]

Actually I didn't have to do that because bug 1704461 did it for me (thanks, mconley!). The info icon for Finish Account Setup does still need to get replaced though (the icon file is already landed).

Comment 13

[Molly Howell (she/her) (mostly inactive)]

Attachment: Bug 1706148 - Switch to updated icons for extensions and accounts app menu notifications. r=#desktop-theme-reviewers

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3eab9234ab57
Switch to updated icons for extensions and accounts app menu notifications. r=desktop-theme-reviewers,harry

Comment 15

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/3eab9234ab57

Comment 16

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 9217820 [details]
Bug 1706148 - Switch to updated icons for extensions and accounts app menu notifications. r=#desktop-theme-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Required for MR1 / Proton
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Trigger a "finish account setup" state, as shown in comment 2, and verify that the icon displayed is now the correct one; it should be a solid filled circle, instead of a smaller circle with an outline as shown in comment 2.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This change only affects icons.
• String changes made/needed:

Comment 17

[Rares Doghi, Desktop QA]

This issue is verified as fixed in our latest Nightly build 90.0a1 (2021-04-27).

Comment 18

[Pascal Chevrel:pascalc]

Comment on attachment 9217820 [details]
Bug 1706148 - Switch to updated icons for extensions and accounts app menu notifications. r=#desktop-theme-reviewers

Approved for 89 beta 6, thanks.

Comment 19

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/a0c0f53e831f

Comment 21

[Rares Doghi, Desktop QA]

This issue is Verified as fixed in 89.0b9 on Windows, Mac and Ubuntu.