| Florian | |
| Aug 6 2015, 9:43 PM |
| F654276: Unbenannt2.PNG | |
| Aug 6 2015, 9:43 PM |
| F654282: Unbenannt.PNG | |
| Aug 6 2015, 9:43 PM |
The toast module creates a new object of itself while initializing. It takes only some ms (2,7ms), but isn't really needed most of the time. Maybe we should revisit that (I think it's mostyl done to render a pending toast, e.g. after an edit was saved) and separate the pending toast rendering from the toast itself, so it's only rendered, if really neded.
It also creates an empty toast element in the DOM (which is normal while rendering):
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| Don't render a toast on page load | mediawiki/extensions/MobileFrontend | master | +108 -64 |
I remember we talked about a similar problem in Lyon, maybe you mean that? I'm not sure, if that was the toast, too :/
I think this has always been the case for Drawers
Drawer.js has a line:
$( function () {
self.appendTo( self.appendToElement ); } );
Really they shouldn't be doing this as they should not know about jQuery global.
It would be great to make these more explicitly added to the DOM.
But drawers get first initialized and rendered, when they are used, not on page load (like toasts) :)
See:
toast.js:
M.define( 'toast', new Toast() );
Drawer.js:
M.define( 'Drawer', Drawer );
I think the easiest move would be to move showOnPageReload and _showPending out of the toast module to not need to create a new object of toast on pageload. That would need, that we rewrite all usages of toast to create a new object by themself.
Another way would be to wrap the toast itself into a new object, which creates a toast object itself, but only, if needed. Something like:
toast = {
_getToast: function () {
if ( this.toast === undefined ) {
this.toast = new toastOverlay();
}
return this.toast;
}
show: function ( msg, className ) {
this._getToast().show( msg, className );
}
// ... and the save and show pending toast functions
}Where ToastOverlay is the actual class, which we call "toast", which extends Drawer.js. As the toast module, we could set M.define( 'toast', new Toast() ); without any problems, because it's a simple object now with some wrapper functions and two functions to save and show a pending toast.
Quick note: showOnPageReload and friends are used in MF and also in Gather.
Were we to change them be sure we're not breaking other extensions.
Change 230112 had a related patch set uploaded (by Florianschmidtwelzow):
Don't render a toast on page load
I actually think Drawers adding themselves to the Dom is broken. It's inconsistent with all our other views which have to manually be added.
At very least appending to Dom should happen on the first call to show. I should be able to initialise a drawer without putting it in the Dom.
@Jhernandez @phuedx I would appreciate your thoughts on this subject. To me this seems an unnecessary micro optimisation.
Right now, I'm less interested in shaving off 2.7 ms than I am in having a couple of views that don't behave like all of the others. I also don't like constructors with side effects, so this is doubly irksome.
I agree that the synchronous and unconditional creation of DOM structures and various other DOM methods used in the process does seem something worth optimising away. I think in practice, it'll be more than 2.7ms. It's probably not including all the side-effects from this code path, and presumably it was measured on a desktop machine as well.