Page MenuHomePhabricator

[EPIC] Re-define the contract for displaying drawers and overlays in MobileFrontend
Open, HighPublic

Description

We continue to get issues with the Drawers and overlays displaying or not displaying when they are supposed to be. The OverlayManager code is bit rotting and the Drawer code is not centralised. We need to rewrite these 2 essential parts of our infrastructure to make this system easier to work with and less error prone.

Drawers:

We have 2 types of drawers in the mobile site:
ReferenceDrawer and Drawer.x

Currently the creation of a drawer also appends it to the DOM. This has
to stop!

var M = mw.mobileFrontend;
var mobile = M.require( 'mobile.startup' );
var ReferenceDrawer = mobile.ReferencesDrawer;
var CtaDrawer = mobile.CtaDrawer;
var Drawer = mobile.Drawer;
var d = new Drawer()
var d2 = new ReferenceDrawer()

Going forward we will add and remove drawers like so.
This code (which currently does not work)
when run should show all 3 types of drawer, hiding and showing
them with animations 3s apart:

var M = mw.mobileFrontend;
var mobile = M.require( 'mobile.startup' );
var ReferenceDrawer = mobile.ReferencesDrawer;
var CtaDrawer = mobile.CtaDrawer;
var Drawer = mobile.Drawer;
var d = new Drawer()
var d2 = new ReferenceDrawer()
var d3 = new CtaDrawer()
d.appendTo( document.body );
setTimeout( function () {
    d.hide().then( function () {
        d2.appendTo( document.body );
        setTimeout( function () {
            d2.hide().then( function () {
                d3.appendTo( document.body );
            } );
        }, 3000 );
    } );

}, 3000 );
// Make it so that clicking on interface elements hides the drawer
$( 'body.drawer, #ca-edit, #mainmenu' ).one( 'click', function () {
    d3.hide();
});

Specification

  • When a Drawer is created it is not added to the DOM (T214045).
  • Each drawers needs to be explicitly added to the DOM by the creator
  • Each drawer when added to the DOM is
  • A drawer cannot access parent nodes and has no side effects on other elements. To get the existing

effect of making the entire skin opaque while the drawer is open, a drawer will have an associated shield
that covers the entire article (see Drawer pseudo-html and T214049)

  • Each drawer has a hide method which handles the animation of hiding an element. It returns a Deferred object

so that the developer can react to the completion of a hide.

  • It is possible to open multiple drawers at a time. When multiple drawers are opened,

the last drawer should be on top of the last opened drawer. It is assumed that it is up to the client
to check if multiple drawers are opened. In Minerva we will manage this by emptying and
appending to a specific element in the DOM to assert that only one is open at a time.

  • CSS relating to drawers is in a single file inside mobile.startup/Drawer.less and the CSS file contains

no references to other elements or classes that does not belong to it.

  • Changes to the Drawers must ensure not to break the existing 3 Drawers. This might be a good opportunity

to create a new generic Drawer and migrating all Drawer's to use this.

  • A drawer should not need to emit any events. It is up to whatever creates the Drawer to wire up its behaviour (T214049)

Drawer pseudo-html

<div class="drawer">
    <div class="drawer__shield"><!-- covers entire screen and is opaque --></div>
    <div class="drawer__drawer">
    <!-- this is the current implementation of the drawer -->
    </div>
</div>

Details

Related Gerrit Patches:
mediawiki/extensions/MobileFrontend : masterBREAKING: Cleanup relationship between OverlayManager and Overlay

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 24 2019, 10:09 PM
Jdlrobson triaged this task as Medium priority.Apr 4 2019, 10:13 PM
Jdlrobson renamed this task from [EPIC] Re-define the contract for drawers in MobileFrontend to [EPIC] Re-define the contract for displaying drawers and overlays in MobileFrontend.Jul 31 2019, 8:20 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson raised the priority of this task from Medium to High.Aug 2 2019, 5:03 PM

Change 528595 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] BREAKING: Cleanup relationship between OverlayManager and Overlay

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