Page MenuHomePhabricator

[Spike 10hrs] Determine steps necessary for switch to header-first DOM
Closed, ResolvedPublic

Description

Background

After the research done in T240489: [Epic] Determine the optimum Vector DOM structure for a11y and performance and subsequent discussions, we feel like our best next step is to switch to a header-first DOM structure for early adopter wikis. This task reflects the process and risk assessment necessary for the switch.

Acceptance criteria

  • Identify the process and work required ahead of time for the switch to a header-first DOM
  • Make a note of any risk associated with a given step

Sign off steps

  • Open implementation task

Event Timeline

We plan to wait for Nick to return before picking this spike up.

Change 621794 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/Vector@master] POC: Move navigation before content in DOM and modify jump links

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

Change 622242 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/Vector@master] POC: Force Personal Menu to always be inside header (decoupled from search in header feature)

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

Change 622245 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/Vector@master] POC: Move footer next to content

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

ovasileva renamed this task from [Spike] Determine steps necessary for switch to header-first DOM to [Spike 10hrs] Determine steps necessary for switch to header-first DOM.Aug 25 2020, 4:43 PM

Change 622667 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/Vector@master] Phase 1 POC: Replace @margin-top-header with @padding-top-page-container

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

Change 622668 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/Vector@master] Phase 1 POC: Add styles necessary for mw-article-toolbar-container to be inside mw-workspace-container

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

Change 622669 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/Vector@master] Phase 1 POC: Add styles necessary for mw-sidebar-container to be inside mw-workspace-container

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

Change 622670 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/Vector@master] Phase 2 POC: Move article toolbar inside mw-workspace-container in DOM

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

Change 622671 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/Vector@master] Phase 2 POC: Move sidebar inside mw-workspace-container

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

Change 622672 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/Vector@master] Phase 3 POC: De-absolute-position all the things

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

Change 622675 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/Vector@master] Phase 3 POC: Remove mw-header-placeholder and correctly indent all the things

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

Change 622914 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/Vector@master] Phase 2 POC: Force personal menu to unconditionally be inside header

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

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.

This work includes de-absolutely positioning the header, article toolbar, personal menu. The sidebar was moved to a location in the DOM to support removing its absolute positioning as part of this work which will also be helpful when making the header sticky, but its styles were left as is because it is its own beast with its own considerable logic/risk. It should be done in its own task if removing its absolute positioning is desired.

I've tried to minimize the risk as much as possible with a series of incremental patches divided into three phases:

Phase 1: Preparatory style-only changes

Because it is possible for new HTML to be shipped with cached CSS and cached HTML to be be shipped with new CSS, it is very difficult to make DOM changes and CSS changes together without regressions. Therefore, a possible workaround for this is to add styles that will support the new HTML first and wait for these styles to be cached before introducing HTML changes. Note in this phase styles should mostly or completely be additive (to minimize risk) and completely work with the existing HTML. Also note that we are not removing the absolute positioning yet as we have not made the necessary DOM changes to support that. I propose making the following changes during this phase:

Phase 2: DOM-only changes

Critical: Before merging any commits in Phase 2, ensure that the new styles from Phase 1 have been cached.

Now that the new styles have been cached, we can proceed with making DOM changes that will support de-absolutely positioning elements. Note that the elements will still be absolutely positioned in this phase, however now the DOM structure will support removing their absolute positioning in Phase 3.

PatchRisk/Comments
Phase 2 POC: Force personal menu to unconditionally be inside header
Phase 2 POC: Move navigation before content in DOM and modify jump linksRemoves "Jump to navigation" and "Jump to Search" links with a single "Jump to content" link. Needs more discussion about the value of the "Jump to Search" link and whether we should keep that. Could be contentious.
Phase 2 POC: Move footer next to content
Phase 2 POC: Move article toolbar inside mw-workspace-container
Phase 2 POC: Move sidebar inside mw-workspace-containerThis moves the sidebar out of the header and into the workspace container next to the content. Because this change modifies the tab order (users will no longer be able to tab directly from the sidebar button into the sidebar without JavaScript), it could be contentious and needs more discussion about tab order. This move will be important when making the header sticky if we don't want the sidebar to also be sticky.
Phase 3: Remove styles related to absolute positioning. Remove obsolete DOM elements (e.g. .mw-header-placeholder).

Critical: Before merging any commits in Phase 3, ensure that the new html from Phase 2 has been cached.

Now that the DOM supports removing the absolute positioning, we can remove the styles related to absolute positioning! Note that now a lot of code that relied on the the absolute positioning can be removed or simplified (yay!).

PatchRisk/Comments
Phase 3 POC: De-absolutely position all the things and cleanup FIXMEsGiven that this is the patch that removes absolute positioning for the first time, it is inherently risky for regressions.
Phase 3 POC: Remove mw-header-placeholder and correctly indent all the things

Change 623070 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/Vector@master] POC: DOM order change tied to search in header work

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

Thanks Nick these patches were super helpful. I think if we tie this to the search in header work I'm doing we can simplify its implementation as well as reduce the risk of making this change by supporting the 2 layouts with position absolute and without (which we have to do anyway as part of the search in header roll out). I feel that would allow us to do this without the need for 3 phases. It does, however, tie the move of the search to this change, but I think it's worth it.

This works on the basic that we have two mutually exclusive classes on the body with the new search in header - vector-search-header and vector-search-header-legacy. This allows us to split out the absolute positioning rules.

The above patch demonstrates this approach. What do you think?

@nray and I had a hangout and have a plan for making this happen. I'm going open some tasks around implementation and sign this off.

Change 623070 abandoned by Jdlrobson:
[mediawiki/skins/Vector@master] POC: DOM order change tied to search in header work

Reason:
Talked with Nick today - we'll write a new patch for the implementation. It may look a lot like this but now we've had multiple stabs at this patch, that will hopefully be relatively straightforward.

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

Change 622242 abandoned by Nray:
[mediawiki/skins/Vector@master] Phase 1 POC: Add/Move styles necessary for personal menu to always be inside header

Reason:

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

Change 622667 abandoned by Nray:
[mediawiki/skins/Vector@master] Phase 1 POC: Replace @margin-top-header with @padding-top-page-container

Reason:

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

Change 622668 abandoned by Nray:
[mediawiki/skins/Vector@master] Phase 1 POC: Add styles necessary for mw-article-toolbar-container to be inside mw-workspace-container

Reason:

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

Change 622669 abandoned by Nray:
[mediawiki/skins/Vector@master] Phase 1 POC: Add styles necessary for mw-sidebar-container to be inside mw-workspace-container

Reason:

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

Change 622914 abandoned by Nray:
[mediawiki/skins/Vector@master] Phase 2 POC: Force personal menu to unconditionally be inside header

Reason:

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

Change 621794 abandoned by Nray:
[mediawiki/skins/Vector@master] Phase 2 POC: Move navigation before content in DOM and modify jump links

Reason:

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

Change 622245 abandoned by Nray:
[mediawiki/skins/Vector@master] Phase 2 POC: Move footer next to content

Reason:

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

Change 622670 abandoned by Nray:
[mediawiki/skins/Vector@master] Phase 2 POC: Move article toolbar inside mw-workspace-container

Reason:

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

Change 622671 abandoned by Nray:
[mediawiki/skins/Vector@master] Phase 2 POC: Move sidebar inside mw-workspace-container

Reason:

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

Change 622672 abandoned by Nray:
[mediawiki/skins/Vector@master] Phase 3 POC: De-absolutely position all the things and cleanup FIXMEs

Reason:

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

Change 622675 abandoned by Nray:
[mediawiki/skins/Vector@master] Phase 3 POC: Remove mw-header-placeholder and correctly indent all the things

Reason:

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

I've opened T261802 and am closing this one out! Thanks Nick for working through this one!