1703821 - The separator space is missing when multiple notifications are displayed in the Hamburger menu

Status: VERIFIED FIXED

(Firefox :: Menus, defect)

Description

[Rares Doghi]

Attachment: multiple_Banners.png

[Affected platforms]:
Platforms: Windows 10

[Preconditions]:

Reach about:config and set browser.proton.enabled = true

[Steps to reproduce]:

1. Launch the Firefox Browser and have multiple notifications triggered.
2. Reach the Hamburger menu.

Expected Result:
There should be a separator space between the banner notifications.

Actual Results:
There is no space between the banner notifications.

Comment 2

[Molly Howell (she/her) [:mhowell]]

Rares, do you know of a spec for how we should present multiple banners like this? I haven't been able to find any.

Comment 3

[Rares Doghi]

I haven't seen any specs related to this, to be honest I was comparing with an older version of Firefox where you could clearly see a bit of space between notifications, maybe Meridel can help us with this ?

Comment 4

[Meridel [:meridel]]

Yes, I think space is needed between notifications but I don't think we have a spec for this. I would like to re-direct to Katie Caldwell but I don't see her handle here. I went ahead and Slacked her about it.

Comment 5

[Meridel [:meridel]]

Attachment: image-18.png

There should be 4px between items.

Comment 6

[Molly Howell (she/her) [:mhowell]]

Attachment: Bug 1703821 - Add some space between multiple app menu banners. r=#desktop-theme-reviewers

This patch takes a very rudimentary approach to solving the problem of spacing
out multiple notification banners, but I believe it should work because of the
following properties of those banners:

• There can be any number of addon permission change notifications.
• There can only be zero or one app update notification, and those are the only
  other type.
• The addon notifications are always at the top of the menu.

So, this patch adds space after every addon banner, and that's enough to
guarantee there will be space between any two banners. It also means that when
the only banners are addon ones, there will be more than the normal amount of
space after them and before the accounts menu item; that's not desirable, but
it doesn't look bad, and I'm not sure how to prevent it in CSS given the logic
required to make the decision.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/43fa107e5536
Add some space between multiple app menu banners. r=desktop-theme-reviewers,Itiel

Comment 8

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/43fa107e5536

Comment 9

[Rares Doghi]

Hi Molly, this issue is indeed fixed for the separator space between the Addon banners but, if the user also logs in with a fresh account, the Finish Account Setup notification stretches to the Permission banner one, Should I mark this one as verified as fixed and log a new issue for the space between Finish account and the Extension request banner ?

Comment 10

[Rares Doghi]

Attachment: notFixed.png

Comment 11

[Molly Howell (she/her) [:mhowell]]

(In reply to Rares Doghi from comment #9)

Hi Molly, this issue is indeed fixed for the separator space between the Addon banners but, if the user also logs in with a fresh account, the Finish Account Setup notification stretches to the Permission banner one, Should I mark this one as verified as fixed and log a new issue for the space between Finish account and the Extension request banner ?

Yes, please file another bug. Thanks!

Comment 12

[Rares Doghi]

This issue is verified as fixed in our latest beta build 89.0b3, I added a separate issue for the other case with Bug 1707203.