Page MenuHomePhabricator

Add generally applicable `z-index` stack to MediaWiki skin variables and Codex
Open, LowPublic5 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

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 could 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

  • 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

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

@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)
CCiufo-WMF lowered the priority of this task from High to Low.Apr 26 2024, 8:34 PM

(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?

Picking this up again.
@matmarex I assume you mean extensions within non-default WMF-deployed skins, right? As the z-index adaptions are all skin specific.
So if we're changing all deployed skins equivalently and then identify issues in extensions or even user scripts/styles, we would possibly threaten non-default Foundation skins. For a change in Monobook on its own I don't see that threat, or am I missing something.

Was also discussing when to tackle the remaining pieces best-possibly? We only identified 1 issue in MinervaNeue, which is much more complex from a z-index architecture perspective than for example Monobook.
Would be interested in your take with some distance and some of the merged changes going smoothly so far…

Aklapper changed the task status from In Progress to Open.Apr 11 2025, 10:15 PM

Resetting task status from "In Progress" to "Open" as this task has been "in progress" for more than one and a half years (see T380300).

Change #903737 merged by jenkins-bot:

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

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