Page MenuHomePhabricator

[Unplanned] Notifications (mw.notify) are appearing below OOUI modals
Closed, ResolvedPublic1 Estimated Story Points

Description

In core the notifications overlay has a z-index of 10,000 (which puts it above OOUI modal's 101 and MMV's 1,001 (see T312778), but there is a rule .vector-sticky-header-enabled .mw-notification-area which resets it to z-index of 3.

This rule is applied in Vector classic too due to T312782.

QA steps

  1. log in
  2. Scroll down the page so sticky header shows
  3. Open the user menu
  4. Click the watchstar

Expected: notification appears ABOVE user menu

Previous QA steps

Originally the issue was raised with QA steps which no longer apply from what I can see:

  1. Open an OOUI modal (e.g. the help dialog in VE, Ctrl+?)
  2. Trigger a notification: mw.notify('test')
  3. Observed: The notification appears beneath the translucent overlay and the dialog
  1. Expected: The notification appears above the translucent overlay and the dialog

QA Results - Beta

ACStatusDetails
1T312783#8449544

QA Results - Prod

ACStatusDetails
1T312783#8461292

Event Timeline

Change 812890 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/skins/Vector@master] Restore z-index of mw-notification-area

https://gerrit.wikimedia.org/r/812890

How can we replicate this? Could you provide a screenshot?
I tried your steps but the notification doesn't overlap with the help dialog for me:
{F35317127}

@alexhollender_WMF @Jdrewniak the current patch suggested by Ed would make the notifications appear above the user menu. Looking back at T260338#7554263 it seems we never talked about the correct behaviour when user menu and notifications are present. Perhaps we should close the user menu when that happens to avoid that scenario ever occurring?

Currently:

Screen Shot 2022-07-14 at 8.13.59 AM.png (352×852 px, 37 KB)

After:
Screen Shot 2022-07-14 at 8.14.47 AM.png (310×1 px, 92 KB)

@Jdlrobson it seems like enough of an edge case that we might not need to bother with extra code to make the user menu close, unless we think it will be common for the user menu to be open while a notification popup appears?

I agree with @alexhollender_WMF, notifications are ephemeral and dismissable. There's all sorts of UI or content they could conflict with (especially as you can have multiple notifications which stack), so I don't think we should do anything to de-conflict them beyond giving them a sensible starting position (which we already do).

I tried your steps but the notification doesn't overlap with the help dialog for me:
F35317127

Screenshot not attached.

You may need to make the window narrower to get the notification to actually overlap, but at any width the notification will appears under the white translucent modal mask:

image.png (443×594 px, 40 KB)

Looking back at T260338#7554263 it seems we never talked about the correct behaviour when user menu and notifications are present.

This conversation was more than a month ago so naturally I have no recollection of it! I think the quote in T260338#7504148 was misattributed to me, and was actually by the author of T290270.

Either way I think we weren't considering the other z-index issues with notifications during that discussion, and on reflection I think having notifications be consistently on the top layer is more important, as they can be fired at any time by any piece of code.

If there was a common use case where the two menus conflicted (e.g. clicking something in the menu triggered a notification, or click something in a notification triggered a menu) then we could considered how to have one close the other, but I think this is just an uncommon edge case and not a problem.

Per editing team sync it sounds like this does not relate to any in flight talk pages work. Olga says we can pull it in later in the quarter.

Jdlrobson added a subscriber: ovasileva.

@ovasileva not sure why this ended up in tech backlog. Seems very much a product/design request. The patch looks good to me, so it's just a case of organizing code review.

Hey @Esanders circling back to this now and have merged the patch.

I can't seem to replicate the QA steps for this ticket (as sticky header is disabled in edit mode)

Screen Shot 2022-12-01 at 11.05.49 AM.png (568×1 px, 158 KB)

There does seem some value in making this change to show notifications when the user menu is open (and even though this is an edge case less code seems better):

Screen Shot 2022-12-01 at 11.11.28 AM.png (308×670 px, 26 KB)

Screen Shot 2022-12-01 at 11.12.37 AM.png (173×502 px, 14 KB)

I'd like to add some QA steps, but want to check if the description is still relevant or if I should amend to show the user menu case?

Change 812890 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Restore z-index of mw-notification-area

https://gerrit.wikimedia.org/r/812890

Jdlrobson renamed this task from Notifications (mw.notify) are appearing below OOUI modals to [Unplanned] Notifications (mw.notify) are appearing below OOUI modals.Dec 1 2022, 7:30 PM
Jdlrobson claimed this task.
Jdlrobson updated the task description. (Show Details)

@Edtadros please ignore the current description as I'm waiting for input from @Esanders on whether it's still relevant.

LGoto set the point value for this task to 1.Dec 5 2022, 6:33 PM

Test Result - Beta

Status: ✅ PASS
Environment: beta
OS: macOS Ventura
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

log in
Scroll down the page so sticky header shows
Open the user menu
Click the watchstar
Expected: notification appears ABOVE user menu

Screen Recording 2022-12-06 at 7.23.28 PM.mov.gif (692×996 px, 670 KB)

Edtadros subscribed.

Test Result - Prod

Status: ✅ PASS
Environment: enwiki
OS: macOS Ventura
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

log in
Scroll down the page so sticky header shows
Open the user menu
Click the watchstar
Expected: notification appears ABOVE user menu

Screen Recording 2022-12-12 at 9.56.30 AM.mov.gif (608×1 px, 1 MB)

All done here, resolving.