Maniphest T202466

[Bug] toast messages not visible in overlays - errors caused by talk overlay are not visible
Closed, ResolvedPublic5 Estimated Story Points
Actions

Assigned To

Authored By

	Jdlrobson
	Aug 21 2018, 10:54 PM

Description

Currently when an error is thrown by the talk overlay it is displayed as a "toast error"

Given toasts are currently positioned below the overlay, these errors are not visible to the user so the error is not visible and the editor has no idea there has been a problem.

Developer notes

If I manually make the overlay opaque (for debugging purposes only) I can see that the toast is present but hidden under the overlay:

Screen Shot 2018-08-21 at 3.50.43 PM.png (774×612 px, 107 KB)

Currently the mw-notification-area is inserted inside #content
If this area is instead added as a child of the body element this seems to work just as well.

Acceptance criteria

Make sure the mw-notification-area element appears above overlays either via z-index or adjustments to the DOM

Testing criteria

Visit https://reading-web-staging.wmflabs.org/wiki/Sandwich
Login
Open talk overlay and click "I like sandwiches"
Drop internet connection
Type yes in reply box and hit save
Confirm you see a toast message above the talk overlay saying the error failed to save.

Details

	Subject	Repo	Branch	Lines +/-
	Add a notifications overlay container to the skin	mediawiki/skins/MinervaNeue	master	+1 -0
	Allow skins to place notification container for mw.notify	mediawiki/core	master	+12 -6

Customize query in gerrit

Related Objects
Search...

		Status	Subtype	Assigned	Task
		Resolved		Esanders	T202620 Mobile notification toast appears under surface text
		Resolved		ovasileva	T202466 [Bug] toast messages not visible in overlays - errors caused by talk overlay are not visible

Event Timeline

Jdlrobson created this task.Aug 21 2018, 10:54 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 21 2018, 10:54 PM

Jdlrobson moved this task from Incoming to Triaged but Future on the Web-Team-Backlog board.Aug 21 2018, 10:54 PM

@Jdlrobson - I'm not sure I'm following entirely here. What is the opaqueness for?

Jdlrobson updated the task description. (Show Details)Aug 22 2018, 2:55 PM

Jdlrobson renamed this task from [Bug] Errors caused by talk overlay are not visible to [Bug] toast messages not visible in overlays - errors caused by talk overlay are not visible.Aug 28 2018, 3:09 PM

Jdlrobson moved this task from Triaged but Future to Upcoming on the Web-Team-Backlog board.

Jdlrobson updated the task description. (Show Details)Aug 28 2018, 4:24 PM

ovasileva set the point value for this task to 5.Aug 28 2018, 4:29 PM

We could update the code in core to look for an existing #mw-notification-area before creating a new one - this would give a skin the option to set a designated container.

Is this the cause of T202620?

And this is possibly a duplicate of T143837

Esanders added a parent task: T143837: Append mw.notify div to the body instead of content so it is always on top.Aug 31 2018, 4:58 PM

Esanders added a subtask: T202620: Mobile notification toast appears under surface text.

Change 456665 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/skins/MinervaNeue@master] Add a notifications overlay container to the skin

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

gerritbot added a project: Patch-For-Review.Aug 31 2018, 5:00 PM

Yup both of those patches are dupes!

Pulling into sprint for visibility and for us to assist with code review!

Jdlrobson moved this task from To Do to Needs Code Review on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q1) board.Sep 5 2018, 12:37 AM

Jdlrobson moved this task from Needs Code Review to Blocked on Others on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q1) board.Sep 5 2018, 12:41 AM

Jdlrobson moved this task from Blocked on Others to Needs Code Review on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q1) board.

Change 306560 had a related patch set uploaded (by Jdlrobson; owner: Mooeypoo):
[mediawiki/core@master] Allow skins to place notification container for mw.notify

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

Change 306560 merged by jenkins-bot:
[mediawiki/core@master] Allow skins to place notification container for mw.notify

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

Jdlrobson moved this task from Needs Code Review to Needs Design Review on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q1) board.Sep 6 2018, 4:19 PM

Change 456665 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Add a notifications overlay container to the skin

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

@alexhollender this is ready for testing on https://reading-web-staging.wmflabs.org/wiki/Sandwich#/talk/1

ReleaseTaggerBot added a project: MW-1.32-notes (WMF-deploy-2018-09-18 (1.32.0-wmf.22)).Sep 6 2018, 5:00 PM

Jdforrester-WMF removed a parent task: T143837: Append mw.notify div to the body instead of content so it is always on top.Sep 6 2018, 5:32 PM

Jdforrester-WMF removed a subtask: T202620: Mobile notification toast appears under surface text.

Jdforrester-WMF added a parent task: T202620: Mobile notification toast appears under surface text.

Ryasmeen reassigned this task from Ryasmeen to ABorbaWMF.Sep 12 2018, 6:11 PM

Ryasmeen subscribed.

Looks good to me across a few devices on Staging.

Jdlrobson reassigned this task from ABorbaWMF to ovasileva.Sep 12 2018, 10:05 PM

Jdlrobson added a subscriber: ABorbaWMF.

toasts are now there!

Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptSep 13 2018, 2:24 PM

ovasileva updated the task description. (Show Details)Sep 13 2018, 2:25 PM

• Petar.petkovic removed a project: Patch-For-Review.Oct 1 2018, 4:13 PM

	F25207405: Screen Shot 2018-08-21 at 3.50.43 PM.png
	Aug 21 2018, 10:54 PM

	F25778178: toast-error.jpg
	Sep 11 2018, 7:03 PM

[Bug] toast messages not visible in overlays - errors caused by talk overlay are not visibleClosed, ResolvedPublic5 Estimated Story PointsActions