Latest patch from @Jdrewniak fixes the tab overlapping the loading box. I did notice a somewhat interesting state when the viewport is resized to the point that the personal tools drops to the next line. When that happens, the loading indicator overlap the personal tools but that might be the expected/desired behavior (see below)? /cc @alexhollender

Also, I'm not sure what effect this would have on the test, but wanted to note that the implementation of T262872 (currently in ready for dev) might play a role in how users perceive the new DOM order (bringing back the ability to jump from the sidebar button directly into the sidebar)

lgtm, just had one thought below:

@Jdrewniak Looks good, but I noticed that part of the loading element is being hidden by the tabs on my local instance:

LGTM! 👍

@Niedzielski the removal of mw-page-base was intentional. It used to push down the content with its height, but now that the absolute positioning is removed from most of the elements, it serves no logical function. If we kept it in the DOM it would be an empty div without any styles.

Update on this task as I'm leaving on vacation tomorrow (and come back next Monday the 14th):

Below add patch on top of the origin/wmf/1.36.0-wmf.6 branch per @Reedy 's request

@Reedy yes, I'm on (nray is my nick)

@Platonides it's important to note that the code in question doesn't seem to affect the rendering of the section headings in the DOM regardless if the regex is there or not - that's server rendered. If you checkout master, the header with links still exists with your example. As far as I know, that's considered a feature (although it might be good to review that later on).

Thank you @Platonides for the patches! I think those patches fix the immediate issue, however after discussing this with @phuedx today, we'd both like to try removing the lines relating to the regex altogether as their continued usage is questionable at best. The small patch below applies this removal

It looks like this line 56 from PageGateway.js [1] is at least somewhat implicated in this:

Update: I have two more metrics to add when the TypeadheadSearch component is complete, but I think those should be pretty easy and the above patches can be reviewed now.

message passing looks good to me 👍 @Niedzielski I noticed part of the acceptance criteria is to create a task for the implementation. Not sure if this is still needed, but lmk if it is and I'm happy to create one. Signing off for now

Thank you for the feedback @Gilles !

Today we discussed this ticket at the end of standup and concluded that it was a bit too nebulous in its current form so we moved it back to needs analysis. Questions that came up during the discussion:

I've spent some time investigating what it would take to de-absolutely position/establish a navigation-first DOM order. While there is considerable risk involved in doing this in large part due to potential caching issues, it is definitely possible.

@Mhurd you can try using the useskinversion=2 query param e.g. https://en.wikipedia.org/wiki/Barack_Obama?useskinversion=2 🙂

@Peter First, great work with Sitespeed.io! I found it incredibly powerful.

This has been deployed now. I verified on https://en.wikipedia.org/wiki/Barack_Obama?quicksurvey=external-survey-growth-study-screener-survey that it shows. Not sure if this needs to go through QA again, but I'll move it there just in case. @ovasileva feel free to move to sign off if you think it can skip QA

I've signed up to turn this on during the 18:00–19:00 UTC backport window tomorrow Wednesday, August 05.

👍👍 Looks great! Great job!

@Jdlrobson sorry for the delay on this. I had the following concerns with the status quo:

Updated title/description because I don't think VE is required for this bug to surface - AFAICT it happens on any page where the viewport is small enough to wrap the personal menu

However, for desktop (Vector) this currently does not include the skin layout., because in Vector that is currently at the end of the HTML payload.

Thanks @Volker_E and @Niedzielski . I dug into the performance aspects of a content-first vs header-first (plus sidebar, article toolbar, personal tools) DOM order more since that was identified by @Volker_E as potentially having the biggest impact:

@ovasileva I think this ticket is pretty important for future DIP work (it certainly played a large role in T246420) and IMO should be prioritized. I'm moving to the upcoming column as I did some analysis of it today.

Now that the max-width ticket has been resolved (T246420, T257518), I think we should make this ticket a very high priority and do a thorough cost-benefit analysis of Vector's "content-first" DOM order status quo as there were many challenges I encountered during this work (e.g. T257518) that made me question its value, and I think the pitfalls associated with this DOM order will continue to pop up as we progress with the Desktop Improvements Project. Additionally, the further we progress through Desktop Improvements and the more hoops we have to jump through to make this DOM order work, the more costly it will be to change later.

Tested VectorDefaultSidebarVisibleForAuthorisedUser flag (true/false) and VectorDefaultSidebarVisibleForAnonymousUserflag (true/false). Both seem to be working as they should!

@Edtadros That's kind of weird that IE11 does that (although it is IE), but I don't think it's very concerning.