Closed Bug 1706148 Opened 8 months ago Closed 7 months ago

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

Categories

(Firefox :: Menus, defect, P2)

Desktop
Unspecified
defect

Tracking

()

VERIFIED FIXED
90 Branch
Tracking Status
firefox87 --- disabled
firefox88 --- disabled
firefox89 --- verified
firefox90 --- verified

People

(Reporter: rares.doghi, Assigned: mhowell)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [proton-notifications] [proton-icons] [priority:2a] [proton-uplift])

Attachments

(5 files)

[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.

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 ?

Has Regression Range: --- → no
Has STR: --- → yes
Priority: -- → P1
Whiteboard: [proton-notifications]

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!

Flags: needinfo?(sfoster)
Flags: needinfo?(mhowell)

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

Flags: needinfo?(sfoster)
Whiteboard: [proton-notifications] → [proton-notifications] [proton-icons]
Priority: P1 → P2
Whiteboard: [proton-notifications] [proton-icons] → [proton-notifications] [proton-icons] [priority:2a]

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

Assignee: nobody → mhowell
Status: NEW → ASSIGNED
Flags: needinfo?(mhowell)

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.

Flags: needinfo?(sfoster)

(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?

Flags: needinfo?(sfoster)

(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.

(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).

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

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.

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).

Pushed by mhowell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3eab9234ab57
Switch to updated icons for extensions and accounts app menu notifications. r=desktop-theme-reviewers,harry
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch

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:
Attachment #9217820 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Whiteboard: [proton-notifications] [proton-icons] [priority:2a] → [proton-notifications] [proton-icons] [priority:2a] [proton-uplift]
QA Whiteboard: [qa-triaged]

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

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.

Attachment #9217820 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Duplicate of this bug: 1703869

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

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.