Page MenuHomePhabricator

Editor and category overlays should not use OverlayManager.replaceCurrent
Open, Needs TriagePublic

Description

The replaceCurrent method has been an anti-pattern for some time. It replaces the existing overlay and updates OverlayManager internals. It can be avoided by using a wrapping component e.g. associate a single Overlay with a single element and use the promisedView to alter views within the overlay itself.

The only overlays using replaceCurrent are the category overlay (taken care of in the refactor T212465) and the editor overlays.

A proof of concept demonstrates what's possible without the replaceCurrent method in the codebase:
https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/616175

I (@Jdlrobson) can provide code if others who know the editor better can provide review.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 24 2020, 10:59 PM
JTannerWMF moved this task from To Triage to Triaged on the VisualEditor board.Jul 28 2020, 3:39 PM
JTannerWMF added a subscriber: JTannerWMF.

Hey @Jdlrobson , feel free to write the code and the Editing team will perform review when ready

Change 620127 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] WIP: Simplify editor switching to not use overlay manager replaceCurrent

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