Page MenuHomePhabricator

Deprecate use of M.require( 'mobile.startup/toast' )
Closed, ResolvedPublic5 Estimated Story Points

Description

This continues the conversation in T209188#4836237

On mobile, VE uses the same base class as the source editor, EditorOverlayBase, and so they both use the toast’s showOnPageReload function to render the toast after the page has reloaded.

Given we only use this Toast on the editor and Special:MobileOptions let's rename src/mobile.startup/toast.js to showOnPageReload.js and deprecate and remove the Toast class to use mw.notify from now on. Following on from T209188 let's continue the conversation about the future of this code.

Acceptance criteria

  • showOnPageReload will be continue to be used in the 2 places it is currently being used (MobileOptions and the editor)
  • Any existing usages of Toast and Toast.prototype.show will be replaced with mw.notify

Sign off steps

  • Decide whether the resulting functionality showOnPageReload should be upstreamed to core and whether we can consolidate it with the wgPostEdit code mentioned in T209188#4836237

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 21 2018, 11:14 PM
Jdlrobson triaged this task as Medium priority.Jan 11 2019, 12:54 AM
ovasileva set the point value for this task to 5.Jan 16 2019, 5:43 PM
Jdlrobson raised the priority of this task from Medium to Needs Triage.Feb 26 2019, 7:10 PM
Jdlrobson triaged this task as Medium priority.Mar 15 2019, 7:44 PM
Soda claimed this task.Apr 26 2020, 6:30 PM
Soda added a subscriber: Soda.

Is amcOutreach a part of Special:MobileOptions ?

Is amcOutreach a part of Special:MobileOptions ?

I'm not sure I understand the question. Can you elaborate on the reason behind this question? AMC outreach is the drawer that shows to users who can opt into AMC who haven't when they click certain interface elements. It looks like this > https://doc.wikimedia.org/MobileFrontend/master/js/ui/?path=/story/amc--outreachdrawer

Special:MobileOptions is a page which allows opting into AMC and other features.

Soda added a comment.Apr 29 2020, 4:14 PM

@Jdlrobson src/mobile.startup/amcOutreach/amcOutreachDrawer.js:77 uses the toast.showOnPageReload() function to display a message, also like it uses the Special: MobileOptions as a backend to set AMC. I was confused as to whether I should remove the showPageOnReload() function and replace it with mw.notify() since it isn't technically a part of the Special: MobileOptions page...

To keep things simple you can leave showOnPageReload inside the file src/mobile.startup/toast.js - you probably want to rename that file to src/mobile.startup/showOnPageReload.js The goal is not to remove Toast altogether, but to change the implementation of showOnPageReload so that it calls mw.notify

Change 594766 had a related patch set uploaded (by Sohom Datta; owner: Sohom Datta):
[mediawiki/extensions/MobileFrontend@master] Replace Toast.show() with mw.notify()

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

Soda added a comment.May 6 2020, 5:55 PM

@Jdlrobson, made the changes... :)

Moving in to track code review status.

Change 596185 had a related patch set uploaded (by Sohom Datta; owner: Sohom Datta):
[mediawiki/skins/MinervaNeue@master] Replacing instances of toast.show() with mw.notify()

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

Change 596185 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Replacing instances of toast.show() with mw.notify()

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

Change 594766 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Replace Toast.show() with mw.notify()

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

Jdlrobson updated the task description. (Show Details)May 19 2020, 5:08 PM
Jdlrobson closed this task as Resolved.May 19 2020, 5:12 PM

Thanks @Soda !
Have setup T253133 per sign off steps. There are a few references to the now unused "Toast" in documentation but hopefully will get updated as we encounter them.