Page MenuHomePhabricator

[design] Notifications (mw.notify) are a long way from the content on wide screens
Open, MediumPublic0 Estimated Story Points

Description

Observed:

image.png (818×2 px, 308 KB)

Expected:
The notification is closer to the content area, for example it is within the white part of mw-page-container:

image.png (297×1 px, 76 KB)

Event Timeline

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

[mediawiki/skins/Vector@master] mw.notify: Limit width of overlay to max-width-page-container

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

(If you have arrived here from an on-wiki comment about AddThis gadget, the link should've pointed here: T230124)

Jdlrobson added subscribers: JScherer-WMF, Jdlrobson.

@JScherer-WMF does the proposal in the task description (see also demo link) look good to you?

@JScherer-WMF does the proposal in the task description (see also demo link) look good to you?

Looks good! Thanks.

Change 968650 merged by jenkins-bot:

[mediawiki/skins/Vector@master] mw.notify: Limit width of overlay to max-width-page-container

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

Jdlrobson set the point value for this task to 0.

I like this change, it makes sense and looks good. But: The placement is a bit awkward when you use Vector-2022 in full-screen mode . Could that be taken into account?

Screenshot_20231111_011541.png (397×568 px, 40 KB)

Edtadros added a subscriber: Edtadros.

Test Result - Beta

Status: ❓Need More Info
Environment: beta
OS: macOS Sonoma
Browser: Chrome
Device: MBA
Emulated Device:NA

Test Artifact(s):

QA Steps

❓ AC1: Notifications should appear in the content area when on wide screens. See demo and Expected image in task description.

I couldn't get the same notification as the example in the description. I was able to get some notifications however. On limited width they work as expected. On full width they do not work as expected. If the functionality of limited width is the only scope of this task then it's a pass, otherwise a fail. See below:

screenshot 116.mov.gif (724×2 px, 180 KB)
screenshot 114.mov.gif (724×2 px, 438 KB)
screenshot 115.mov.gif (724×2 px, 563 KB)
screenshot 117.mov.gif (724×2 px, 456 KB)
Jdlrobson added a subscriber: ovasileva.

@Esanders it looks like your patch to https://gerrit.wikimedia.org/r/c/968650 didn't work as expected (see comment from @jhsoby and @Edtadros )
@ovasileva not sure if this constitutes a regression in your perspective. Should we revert the patch or do you see this as an overall improvement?

Jdlrobson removed the point value for this task.Fri, Nov 17, 5:19 PM

@Esanders it looks like your patch to https://gerrit.wikimedia.org/r/c/968650 didn't work as expected (see comment from @jhsoby and @Edtadros )
@ovasileva not sure if this constitutes a regression in your perspective. Should we revert the patch or do you see this as an overall improvement?

If possible, I'd say let's revert until we can get it to look slightly better. I agree that the current positioning feels a bit awkward. @JScherer-WMF - wondering what you think as well.

@Esanders it looks like your patch to https://gerrit.wikimedia.org/r/c/968650 didn't work as expected (see comment from @jhsoby and @Edtadros )
@ovasileva not sure if this constitutes a regression in your perspective. Should we revert the patch or do you see this as an overall improvement?

If possible, I'd say let's revert until we can get it to look slightly better. I agree that the current positioning feels a bit awkward. @JScherer-WMF - wondering what you think as well.

I agree. Sorry I didn't have a closer look at various viewport widths with the initial query. Thanks for flagging it @jhsoby !

Somewhere above the title probably makes more sense, but just for due diligence, could someone outline the steps a user has to complete to get this notification?

No worries, these things happen. :-)

The easiest way to trigger a notification via the normal UI is probably to make an edit, and it should notify you that the edit was saved successfully. But if you're testing its appearance, it would probably be even easier to just enter mw.notify( 'Random text here' ); in the browser console. :-)

Change 975042 had a related patch set uploaded (by Jforrester; author: Jdlrobson):

[mediawiki/skins/Vector@master] Revert "mw.notify: Limit width of overlay to max-width-page-container"

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

Change 975042 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Revert "mw.notify: Limit width of overlay to max-width-page-container"

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

Change 975366 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@wmf/1.42.0-wmf.5] Revert "mw.notify: Limit width of overlay to max-width-page-container"

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

@Jdrewniak When you're finished reverting this, please assign to me, and I'll work out a design that doesn't break at smaller viewports.

Change 975366 merged by jenkins-bot:

[mediawiki/skins/Vector@wmf/1.42.0-wmf.5] Revert "mw.notify: Limit width of overlay to max-width-page-container"

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

Mentioned in SAL (#wikimedia-operations) [2023-11-20T21:58:00Z] <catrope@deploy2002> Started scap: Backport for [[gerrit:975366|Revert "mw.notify: Limit width of overlay to max-width-page-container" (T349622)]]

Mentioned in SAL (#wikimedia-operations) [2023-11-20T21:59:22Z] <catrope@deploy2002> jdlrobson and catrope: Backport for [[gerrit:975366|Revert "mw.notify: Limit width of overlay to max-width-page-container" (T349622)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Jdlrobson added a subscriber: Jdrewniak.

Patch has been reverted. Over to Justin for design.

Mentioned in SAL (#wikimedia-operations) [2023-11-20T22:15:41Z] <catrope@deploy2002> Finished scap: Backport for [[gerrit:975366|Revert "mw.notify: Limit width of overlay to max-width-page-container" (T349622)]] (duration: 17m 40s)

Presumably just limiting the patch to fixed width Vector 2022, and leaving as-is in other situations, would be fine (an improvement on the status quo)?

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

[mediawiki/skins/Vector@master] Reapply "mw.notify: Limit width of overlay to max-width-page-container"

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

Jdlrobson removed the point value for this task.Mon, Nov 27, 6:25 PM
Jdlrobson renamed this task from Notifications (mw.notify) are a long way from the content on wide screens to [design] Notifications (mw.notify) are a long way from the content on wide screens.Mon, Dec 4, 6:54 PM
Jdlrobson set the point value for this task to 0.Tue, Dec 5, 11:05 PM