Change 486385 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/MobileFrontend@master] Add #loadModuleAndGetOverlay: a nicer way to use LoadingOverlay

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

Change 486386 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/MobileFrontend@master] Improve loading screens by using #loadModuleAndGetOverlay

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

Change 486387 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/skins/MinervaNeue@master] Improve loading screens by using #loadModuleAndGetOverlay

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

Page underneath the overlay flashes for 1 frame when switching between loading overlay and real overlay

Yeh, this is unnecessary and only because of the way we use inheritance in our codebase. The good news is we have a plan for dealing with that that will be commenced post-all hands. We've been looking at the TalkOverlay but it can apply to any overlay:
We're working towards a composition based approach to end up with code like:
https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MobileFrontend/ /471334/

When that's done, we'll be able to move the remaining code into skins.minerva.scripts onto the critical path as it will be using code that's available from page load. I'd love to get you up to speed on those plans and get your help with making that happen!

Neat. So, compared to my proposed approach:

• Show the loading overlay (handled by OverlayManager)
• Start loading modules. Once loaded:
  • Replace the loading overlay in-place by the real overlay (handled by OverlayManager)

Your approach instead is:

• Show a generic overlay (handled by OverlayManager) and put a placeholder spinner inside of it
• Start loading modules. Once loaded:
  • Replace the placeholder contents of the existing overlay by real contents

I think that's just as elegant. I proposed my solution mostly because it did not require refactoring the existing code for different overlays, except for the one line that loads them. I didn't know you had another approach in the works.

I'll note that it doesn't really have much to do with inheritance. Certainly, you could implement it without the ugly flash with inheritance (as I did), and you could implement it with the ugly flash without inheritance, if you just put a setTimeout(), promise resolution, or something equivalent, between removing the placeolder and inserting real contents. The solution is just to replace the overlay in a single "tick".

Exactly! Also means you can exit the overlay before you finish loading (right now if your connection drops mid overlay load you become stuck!)

The main problem with the inheritance is it means the extended Overlay is not available until its associated code is loaded and if you dont code split you get a larger initial load of JS.

Will probably need some help from you with code review for the refactoring the editor overlay when we tackle that next month.

Change 486385 abandoned by Bartosz Dziewoński:
Add #loadModuleAndGetOverlay: a nicer way to use LoadingOverlay

Reason:
Jon is working on an alternative solution as part of T212465.

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

Change 486386 abandoned by Bartosz Dziewoński:
Improve loading screens by using #loadModuleAndGetOverlay

Reason:
Jon is working on an alternative solution as part of T212465.

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

Change 486387 abandoned by Bartosz Dziewoński:
Improve loading screens by using #loadModuleAndGetOverlay

Reason:
Jon is working on an alternative solution as part of T212465.

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

@matmarex I think this is fixed for the editor? Can you confirm?

It will soon be fixed for talk too which just leaves the categories feature which may be removed.

Yes, we solved it for visual editor in T210630, and then also for wikitext editor in T228159.

Change 565709 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Clicking the category overlay instantly displays the category overlay

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

Change 565710 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Dev: Pave the way for getting rid of rlModuleLoader

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

Change 565711 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Dev: rlModule BE GONE!

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

The above three patches should take care of this problem for the last remaining overlay and then prevent it from every happening again.

Change 565710 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Dev: Pave the way for getting rid of rlModuleLoader

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

Change 569699 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Categories: Messages do not need to be passable

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

Change 569755 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Expose headers on mobile.startup object

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

Change 569834 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] CategoryAddOverlay: Build consistently with other Overlays

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

Change 569699 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Categories: Messages do not need to be passable

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

Change 569755 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Expose headers on mobile.startup object

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

Change 565709 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Clicking the category overlays is synchronous

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

Change 565711 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Dev: rlModule BE GONE! OverlayManager routes must always be synchronous

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

Change 569834 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] CategoryAddOverlay: Build consistently with other Overlays

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

This is done now. The category overlay now loads synchronously.

While this was a technical change...
From an @alexhollender pov this is good because clicking any button that opens an overlay now will feel more responsive then before - no more spinner.
These changes targeted the categories overlay only which is in AMC and beta. The only differences is now when clicking categories, the overlay opens instantly. In addition to this clicking next should open much more smoothly without an intermediate flash. I'm not sure how important that feature is but we can QA the categories feature if you feel that's important. Let me know @ovasileva if you need me to write QA steps.

In T214641#5877798, @phuedx wrote:

Per T214641#5861539, this is a technical change that has affected the UX of the categories overlay. It should go through design review.

1 I'm also noticing that this task went somewhat around our usual process and is in need of an estimate. Perhaps we can discuss in standup today. Also, since we have user-facing changes, we should go through QA after design review.