refactor: replace raw z-index values with semantic tokens - #13247
Merged
lokesh merged 6 commits intoAug 4, 2026
Conversation
14 tasks
…uards Follow-ups to the token migration in internetarchive#13247. - MergeTable: the sticky header was promoted 300 -> 999 (--z-index-dropdown), moving it into the site-wide band. A shadow root is not a stacking context, so that value leaked into the page's root stacking context and could occlude overlays. Isolate the table instead and use --z-index-raised. - LibraryExplorer: the toolbar dropped 20 -> 10, tying BookRoom's shelf signs and leaving the winner to DOM order. Restore the gap by moving the signs to --z-index-sticky, and note the pair from both files. - OlSegmentedControl: convert the three raw values the sweep missed. The pill's `z-index: 0` had no token, so add --z-index-flat. - tokens/z-index.css: document the two rules this surfaced -- shadow roots do not create stacking contexts, and ties within a band resolve by document order. - Components dev harness loads tokens.css. Without it every tokenised declaration is invalid at computed-value time and drops to `auto`. - tests/e2e/z-index.spec.ts: token canary, per-component computed-value guards, and two shadow-piercing hit tests for the popover tray/scrim pair, whose ordering now survives only by render order.
…okens Phase 1 of internetarchive#12363. Components that use z-index only to order their own children now seal that ordering with isolation: isolate on their root and a new --z-index-local-1..5 ordinal tier, instead of consuming the global semantic bands. The bands stay reserved for page-level chrome. Isolation added: .list-follow-card (avatar overlaps up into the covers strip, so the card is the containment root), .list-card__covers, .stars, .crumbicon, .illustration (makes the .cover-animation z:-1 containment explicit instead of an accident of its z-index), ol-carousel :host and OLMarkdownEditor .editor-wrapper (shadow roots are not stacking contexts, so both leaked to the page), and LibraryExplorer's .book-room. Deliberately NOT isolated: any card/section containing a generic dropper (searchResultItem, .editionCover) - the dropper menu must escape downward over page content. Verified by hit test with the dropdown forced open. Value mapping is mechanical (level-N -> local-N) so relative order and document-order ties are preserved exactly. Exceptions: segmented control shifts 0/1/2 -> local-1/2/3 (order preserved; 0 has no ordinal), the markdown editor's three 999s become local-1 (equal, as before), and the slick arrows drop 10 -> local-1 (only competitor is the track's transform context at auto). .down-chevron's z-index is deleted outright - it is static and not a grid item, so the declaration was inert. --z-index-flat loses its only consumer (the pill) to the local tier; the token stays as page-band vocabulary but is currently unused. e2e: token canary covers the local tier; segmented-control guards track the +1 shift. All 4 z-index specs pass; every touched surface verified in-browser (computed values + dropper-escape hit test).
Collaborator
|
Hi @Armansiddiqui9 — thanks for the token sweep! Building on it, I've pushed two follow-up commits directly to your branch (review fixes + the |
--z-index-flat lost its only consumer (the segmented-control pill) when internal layering moved to the local tier, and --z-index-level-zero only existed to feed it. Neither earns its keep: versus `auto`, z-index: 0 orders nothing — its sole effect is creating a stacking context, and the explicit spelling for that is isolation: isolate. The docblock now says so, so the next person reaching for a zero finds the right tool instead.
Collaborator
|
Confirming everything continues to render correctly on testing server: 
|
The e2e suite is not wired into CI, so these never ran on a PR, and two of the three tests re-encoded token values that already live in tokens.css. Revisit with tests that cover the parts a diff cannot show — isolation containment and dropper escape.
npm run test:e2e writes playwright-report/ and test-results/ into the repo root, where they showed up as untracked noise in git status.
lokesh
approved these changes
Aug 4, 2026
lokesh
left a comment
lokesh
left a comment
Collaborator
There was a problem hiding this comment.
LGTM!
Manually tested the following: search modal, carousels, popovers, merge works, my list, mobile popovers, library explorer.
50 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contributes to #12363 (Phase 0 + Phase 1)
refactor
Technical
This PR replaces hardcoded z-index values in the affected Vue, Lit, and CSS files with the shared design system z-index tokens. The goal is to make layering behavior more consistent across components and avoid relying on magic numbers, while keeping the existing visual behavior unchanged.
Testing
Screenshot
UI behavior is unchanged.
Stakeholders
@lokesh
Maintainer additions (@lokesh)
Pushed on top of the original token swap, per the phased plan now described in #12363:
tokens.csssovar()lookups resolve undernpm run serve.isolation: isolateon their root and use a new--z-index-local-1..5ordinal tier instead of global tokens. Cards containing droppers are deliberately not isolated — their menus must escape.No tests here. I had added an e2e spec and then pulled it back out: the Playwright suite isn't wired into CI, so it would never have run on a PR, and most of it re-asserted values that already live in
tokens.css. The stacking behaviour worth covering is the part a diff can't show — isolation containment, and droppers still escaping the cards we deliberately left un-isolated. That's a follow-up, alongside deciding whether the e2e suite should run in CI at all.