Page MenuHomePhabricator

Add generally applicable `z-index` stack to MediaWiki skin variables and Codex
Open, In Progress, HighPublic5 Estimated Story Points

Assigned To
None
Authored By
Volker_E
Jun 26 2021, 10:03 AM
Referenced Files
F37070637: Screenshot 2023-05-26 at 3.01.03 PM.png
May 26 2023, 8:04 PM
F37070605: image.png
May 26 2023, 6:49 PM
Restricted File
May 24 2023, 11:09 PM
F37019463: image.png
May 19 2023, 7:02 AM
F37019461: image.png
May 19 2023, 7:02 AM
F36910728: image.png
Mar 14 2023, 8:33 AM
Tokens
"Love" token, awarded by alistair3149."Love" token, awarded by Mainframe98.

Description

Background & Goal

MediaWiki core, skins, extensions and external libraries are coming from all different contributors with all different z-index internal logics.
We need to provide a unified z-index stack for skins and extensions via central variables.
That would take a lot of burden from developers.

What is to win from a central agreed-on stack:
Such issues should be prevented in future:

image.png (448×1 px, 96 KB)

Proposal

As Less output:

// Page layout tokens
@z-index-bottom: -100;
@z-index-base: 0;
@z-index-above-content: 1;
@z-index-toolbar: 2;
@z-index-dropdown: 50;
@z-index-sticky: 100;
@z-index-fixed: 200;
@z-index-off-canvas-backdrop: 300;
@z-index-off-canvas: 350;
@z-index-overlay-backdrop: 400;
@z-index-overlay: 450;
@z-index-tooltip: 800;
@z-index-toast-notification: 900;
@z-index-top: 9999;

// Stacking-specific tokens for use in components.
@z-index-stacking-0: 0;
@z-index-stacking-1: 1;
@z-index-stacking-2: 2;
@z-index-stacking-3: 3;

We're mixing two different approaches and use cases in the proposal above:

  1. The page layout (everything besides stacking in 2.)
  2. The direct stacking layout context (for example in Codex ButtonGroup)

Developer notes

Note, that we might need to be careful when rolling out the change as it could possibly impact Vector with its “sticky” (fixed) header
Note, while another size token is called -absolute-9999, it seems counter productive here, as it will end up in an arms race like recently in Kartographer with z-index: 9999999.

Acceptance criteria for Done

  • Settle on a clear stack for Codex and Wikimedia products with help of POC patches
  • Bring to Codex
  • Bring to WikimediaUI Base Decided to prioritize on replacing Base with Codex tokens T334934
  • Bring to MediaWiki core by help of Codex design tokens containing mediawiki.skin.variables
  • Use in Wikimedia deployed skins
    • Use in Vector
    • MobileFrontend and
      • MinervaNeue
    • MonoBook
    • Example
    • Other skins?
  • Use exemplarily in an extension (Growth's Newcomer Homepage seems like a good candidate due to big variety of z-indizes in use)


Previous implementations

Current Codex design decision tokens
@z-index-base: 0;
@z-index-overlay: 101;

MinervaNeue
From minerva.less/minerva.variables.less

// z-index:
@z-indexOccluded: -1;
@z-indexBase: 0;
@z-indexAboveContent: 1;
@z-indexDrawer: 2;
@z-indexOverlay: 3;
@z-indexOverOverlay: 4;

ContentTranslation
comes with its own z-index system

Popups
features for example z-index: 1000 on #mwe-popups-settings and

@zIndexPopup: 110;
@zIndexBackground: 111;
@zIndexForeground: 112;
@zIndexThumbnailMask: 113;

External libraries

  • External libraries:
    • Bootstrap v5.3
$zindex-dropdown:                   1000;
$zindex-sticky:                     1020;
$zindex-fixed:                      1030;
$zindex-offcanvas-backdrop:         1040;
$zindex-offcanvas:                  1045;
$zindex-modal-backdrop:             1050;
$zindex-modal:                      1055;
$zindex-popover:                    1070;
$zindex-tooltip:                    1080;
$zindex-toast:                      1090;
'auto': auto
'top': 99999
500: 500
400: 400
300: 300
200: 200
100: 100
0: 0
'bottom': -100

Further resources

I personally don't find the approach here convincing in details.

// Page Layout
export const zLayoutNavigation = above + base;
export const zLayoutFooter = above + base;
export const zLayoutModal = above + zLayoutNavigation;
export const zLayoutPopUpAd = above + zLayoutModal;

Having constants with calculations leads to unexpected results in my experience. Bootstrap's and USWDS try to provide a general expectable solution that still leaves flexibility in context, without relying on contextual computations.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

@Volker_E nice, it's great to see this scale coming together! It all looks good to me at first glance (the stacking tokens make perfect sense to me), but I have some questions:

  • Can you explain why sticky and fixed are ordered as they are?
  • Can you explain what off-canvas and off-canvas-backdrop would be used for?
  • Is "toast" recognizable enough, or should we call this thing "toast notification" in Codex? I fear many people will not know what "toast" means on its own, and it's not a documented pattern/thing on MDN or W3C WAI

Also, since z-index is a bit of a tricky concept, we may want to link to some helpful information or include a visual representation of what they values mean/apply to in the docs. Simply documenting the values in the tokens table might not be sufficient here.

@Volker_E nice, it's great to see this scale coming together! It all looks good to me at first glance (the stacking tokens make perfect sense to me), but I have some questions:

  • Can you explain why sticky and fixed are ordered as they are?

That's practice, also featured in Bootstrap for a good reason. Sticky elements like in an index sectioned context should be under a fixed navigation element. As example all sticky elements should be overlaid by the "sticky" in reality, dynamically position: fixed Vector 2022 header.

  • Can you explain what off-canvas and off-canvas-backdrop would be used for?

theme-wikimedia-ui.json is already giving hints in comments:

		"300": {
			"comment": "Use for off-canvas Menu and sidebar backdrop.",
			"value": "300"
		},
		"350": {
			"comment": "Use for off-canvas Menu and sidebar.",
			"value": "350"
		},

See for example the responsive menu on DSG.

  • Is "toast" recognizable enough, or should we call this thing "toast notification" in Codex? I fear many people will not know what "toast" means on its own, and it's not a documented pattern/thing on MDN or W3C WAI

Good point, will amend the proposal and the patch!

Also, since z-index is a bit of a tricky concept, we may want to link to some helpful information or include a visual representation of what they values mean/apply to in the docs. Simply documenting the values in the tokens table might not be sufficient here.

Indeed. Have already talked to @bmartinezcalvo a while ago about the upcoming need for an illustration.

One other simplifying nomenclature idea, would be to exchange 'dropdown' with 'menu' (there are a variety of menus nonetheless), or to make it longer and call it 'dropdown-menu'

When preparing a Vector proof-of-concept, there are two other thoughts of additions to clarify: VE creates a stacking context as well with z-index: 2 for its toolbar, to overlay for example indicators (1) in Vector.
We could either replace or have aliases for z-index-stacking-1 (above-content) and -2 (toolbar). 3 is nasty, but is only used in button contexts, so could be treated as esoteric stacking-buttons or something. Undecided what's better for dev experience.

Change 898683 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/skins/Vector@master] [POC] styles: Apply Codex `z-index` tokens

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

To test the logic, what z-index token would the FloatingButton receive?
It has to be above all content, toolbars, headers, but below modal overlays, tooltips and toast notifications. This should receive @z-index-fixed from current nomenclature.

Another thing we could consider, similar to size-absolute-1 and size-absolute-9999 to rename top/bottom to absolute-top/absolute-bottom.

Change 898694 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/skins/MinervaNeue@master] [POC] styles: Apply Codex `z-index` tokens

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

@Volker_E thanks for all the info!

When preparing a Vector proof-of-concept, there are two other thoughts of additions to clarify: VE creates a stacking context as well with z-index: 2 for its toolbar, to overlay for example indicators (1) in Vector.
We could either replace or have aliases for z-index-stacking-1 (above-content) and -2 (toolbar). 3 is nasty, but is only used in button contexts, so could be treated as esoteric stacking-buttons or something. Undecided what's better for dev experience.

I like the idea of having aliases for the stacking tokens. The numbered tokens are useful for arbitrary content where the dev just needs to know the numerical order, and the semantic tokens are useful when dealing with commonly stacked elements as you described.

@AnneT Ok, I've updated the patch additionally adding z-index-above-content (not great name) and z-index-toolbar.

Change 897958 merged by jenkins-bot:

[design/codex@main] tokens, styles: Add and apply `z-index` category

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

Change 903653 had a related patch set uploaded (by VolkerE; author: VolkerE):

[design/codex@main] tokens, styles: Expand `z-index` token category by `z-index-stacking-0`

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

Change 903653 merged by jenkins-bot:

[design/codex@main] tokens, styles: Expand `z-index` token category by `z-index-stacking-0`

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

Change 903737 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/extensions/GrowthExperiments@master] [POC] styles: Apply Codex `z-index` tokens

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

Test wiki created on Patch demo by RHo (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/cac34478b6/w

Change 903778 had a related patch set uploaded (by Catrope; author: Catrope):

[mediawiki/core@master] Update Codex from v0.7.0 to v0.8.0

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

Change 903778 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.7.0 to v0.8.0

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

Volker_E changed the task status from In Progress to Stalled.Mar 29 2023, 9:43 AM
Volker_E removed Volker_E as the assignee of this task.

Moving back to “Blocked” on the Design Systems Sprint board because it's necessary to gather feedback from outside teams (Web, Growth and Editing) on the POC patches before putting to sign-off. Their feedback might result in slight amendments before resolving this.

@Volker_E so to confirm, we are waiting for a team to actually use these tokens before marking it resolved, or are we just looking for feedback? If it's Feedback, I'd suggest we prioritize getting that feedback ASAP then close this out, and open a new ticket if there are proposed changes down the line.

@CCiufo-WMF Have updated the three patches and brought them out of proof-of-concept state. Vector and MinervaNeue were chosen to ensure the new z-index architecture is provided in the main interfaces to orient on in extensions to use them. Growth was chosen, as it is an extension with rather complex z-indices, to show that it works on both, desktop and mobile. OOUI as other library with a lot of special treatments or VisualEditor are relatively simple in comparison.

Yes, it's reasonable to wait for those three patches to be merged before resolving this task.

@bmartinezcalvo For the token visualization documentation helper, I liked such ways:

image.png (750×1 px, 129 KB)
or
image.png (734×1 px, 117 KB)

We should go into such direction.

@CCiufo-WMF I'd advise to increase point size too due to external dependencies.

Change 898683 merged by jenkins-bot:

[mediawiki/skins/Vector@master] styles: Apply Codex `z-index` tokens

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

Volker_E updated the task description. (Show Details)
Volker_E updated the task description. (Show Details)

@CCiufo-WMF Have update the description with acceptance criteria for resolving the task in my opinion. The two open patches are concerning Readers Web and Growth team. @bwang has already thankfully reviewed and merged the Vector one. We could move this task to code review, although it could sit there longer than reasonable time for a sprint due to external dependency.

The only outlaws MonoBook is featuring are:

.mobile-menu-active {
	z-index: 10000;
}
.menus-cover {
	z-index: 9999;
}

These are better off, by using

@z-index-off-canvas-backdrop: 300;
@z-index-off-canvas: 350;

Similar to MN's main menu.
{F37031559}

The other values are AFAIAO mostly stacking contexts.

Change 922941 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/skins/MonoBook@master] styles: Apply MediaWiki skin variables `z-index` tokens

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

Volker_E changed the task status from Stalled to In Progress.May 25 2023, 12:41 AM

Change 922950 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/skins/Example@master] skin, styles: Replace `z-index` value by a skin variable

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

The addition of patch for Example skin above is targeting documentation purposes.

Change 923155 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/extensions/MobileFrontend@master] styles: Apply Codex `z-index` tokens

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

Also added MF patch, as MF and MinervaNeue are hardly to think one without the other and this last patch would probably simplify merging the MinervaNeue patch.

Change 923157 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/skins/GreyStuff@master] styles: Apply MediaWiki skin variables `z-index` tokens

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

Change 898694 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] styles: Apply Codex `z-index` tokens

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

Volker_E updated the task description. (Show Details)

Change 923155 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] styles: Apply Codex `z-index` tokens

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

The MobileFrontend patch has been reverted as it broke dialogs in mobile VE.

@Jdrewniak has thankfully provided a patch demo with the MinveraNeue/MobileFrontend and VE and some other extensions enabled:
What dialog breakage have you identified, @Esanders and @DLynch ?

image.png (768×1 px, 95 KB)

Seems to work for me.

@Volker_E Interestingly, that patchdemo does seem fine. However, a patchdemo from just before the revert (2023-05-26 13:10:48 vs the revert at 2023-05-26 15:11:24) shows the issue.

Screenshot 2023-05-26 at 3.01.03 PM.png (1×898 px, 430 KB)

(It doesn't screenshot well as a bug. It's just that nothing that's an overlay is appearing above the editing surface.)

Ah, (I think) it's because you didn't recreate the patch, you just tried to make a patchdemo from master with the reverted patch applied to it. That didn't work because the patch is already applied to master (it's just that a later commit reverts it).

On MobileFrontend's .mw-mf-page-center__mask change, only GrowthExperiments has reused it, and I'll make the existing GrowthExperiments patch dependent. https://codesearch-beta.wmcloud.org/search/?q=.mw-mf-page-center__mask&files=&excludeFiles=&repos=#operations/mediawiki-config

Volker_E changed the point value for this task from 3 to 5.May 26 2023, 11:58 PM

Upping to 5 due to consultation left and right.

(commenting here since you asked for my review on a couple of skin patches)

I like the changes in principle, since I can see them avoiding skin compatibility issues in the future, but they also have a huge potential to cause issues now – similar to the mobile VE problems already reported above, but in all of the non-default skins that don't get so much testing. I can't really commit to testing everything thoroughly myself; basically everything that displays a dialog or a popup can be affected. Maybe we could ask someone from the WMF QA team to test the non-default WMF-deployed skins at least, just this one?

Change 922950 merged by Jdlrobson:

[mediawiki/skins/Example@master] skin, styles: Replace `z-index` value by a skin variable

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

Change 923157 merged by jenkins-bot:

[mediawiki/skins/GreyStuff@master] styles: Apply MediaWiki skin variables `z-index` tokens

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

Referencing T340391: Default Reference Tooltips gadget and new Vector search can overlap because of z-index for some confusion on z-indexes in gadgets. The resolution was to hardcode a z-index value equivalent to the Codex design token value for that use case, but this is obviously not ideal and the standardization can easily drift.

Volker_E renamed this task from Add generally applicable `z-index` stack to MediaWiki skin variables and Codex/WikimediaUI Base to Add generally applicable `z-index` stack to MediaWiki skin variables and Codex.Jan 8 2024, 11:55 PM
Volker_E removed a project: WikimediaUI-Base.
Volker_E updated the task description. (Show Details)