fix(theming): route the React grey palette reads through --pie-* tokens PIE-856 - #88
Merged
Merged
Conversation
…ns PIE-856 Carries pie-lib #2310 and pie-elements #3102 into the generated packages. `theme.palette.grey[N]` does not follow `--pie-*`, so all 42 of those reads across twelve element packages and four pie-lib packages held one fixed hex under every colour scheme. Measured against each scheme's own `--pie-background`, the worst case per site ran between 1.01:1 and 1.72:1, below the 3:1 non-text floor in every case -- the answer-choice separator George reported at about 1.1:1 on black-on-rose is the visible end of one defect, not a special case. Worst case per site is now at least 3.17:1. Each site takes the accessor matching its role rather than one replacement token: strokes and connectors `color.border()`, heavy card outlines `color.borderDark()`, fills `color.backgroundDark()` with selected fills `color.dropdownBackground()`, text and interactive icons `color.text()`, quieter glyphs `color.borderGray()`, disabled affordances `color.disabled()`. See the upstream commits for the per-site reasoning. Four surfaces move with their strokes -- `palette.common.white` is MUI's #fff under every scheme, so a tokenised stroke on a fixed white card would vanish under white-on-black, where `--pie-border` is #ffffff. Remapping `palette.grey` itself in the theme was the obvious alternative and does not work: MUI 7.3.11's Tooltip evaluates `alpha(theme.palette.grey[700], 0.92)` when its popper mounts, which throws `MUI: Unsupported var(...) color`, and Tooltip appears in 21 files. AppBar and collapsed Breadcrumbs have the same hazard on shades 100/200/600/900. Leaving MUI's own greys as real hex and moving the call sites avoids all of it. Note the bridge the ticket names, `packages/shared/theming-mui/src/mui-integration.ts`, has no callers -- `createPieMuiTheme` is never invoked, so adding `grey` there would have changed nothing at runtime. The live theme mounts are render-ui's `ui-layout.tsx` and config-ui's `config-layout.tsx`, both generated. Produced by `upstream:sync --element` per package against local pie-elements and pie-lib checkouts, so every file here regenerates identically once both upstream PRs merge. An earlier pass could not say that: the sync was deleting this repo's `isSelected` / `onChoiceClick` / `onPlacementClick` select-then-place affordance from eight match-list and placement-ordering files, because upstream develop did not carry it. It does now -- pie-elements #3098 and #3099, PIE-769 and PIE-644 -- and rebasing the upstream branch onto that resolved the conflict in favour of keeping both the affordance and the colour change. Re-syncing both packages against the rebased upstream then reproduced this tree exactly, so nothing is hand-applied. `tests/pie-token-contract.test.ts` gains the React half of the contract: every `color.*()` accessor resolves to a registered token, no React source names a `--pie-*` outside the registered set, and no file reads `palette.grey` again. Each assertion was mutation-tested. Nine names predate this change and no scheme can reach them -- the four `--pie-keypad-*`, `--pie-keyboard-focus-indicator`, `--pie-primary-text`, `--pie-secondary-text`, `--pie-prompt-holder-max-width` and `--pie-zoom` -- so they are recorded as known gaps rather than silently tolerated. Closing them needs registry entries in pie-players, the same cross-repo blocker PIE-857 hit. Visible change in the default light theme: strokes that were #e0e0e0 or #bdbdbd now resolve to #8f8f8f. Deliberate -- those values were under 3:1 before any scheme applied.
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
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.
Closes PIE-856.
What
theme.palette.grey[N]does not follow--pie-*, so all 42 of those reads across twelveelement packages and four
@pie-libpackages held one fixed hex under every colourscheme. This carries the fix into the generated packages.
Depends on both upstream PRs, which have to merge first:
@pie-libcall sites, plus the newcolor.buttonFocusOutline()accessor this branch needs.Scope
Reported by George Schneiderman with screenshots: the answer-choice separator in
multiple-choicemeasures about 1.1:1 against theblack-on-rosebackground, where thatscheme sets
--pie-borderto#000000.The ticket asks which of the other sites are genuinely broken. All of them. Measured
against each scheme's own
--pie-backgroundacross the ten schemes inpie-players—packages/theme/src/color-schemes.css— MUI's grey shades 100–500 fall below the 3:1non-text minimum in six of the ten, and 600–800 in three or four. Per site the worst case
ran between 1.01:1 and 1.72:1; it is now at least 3.17:1 everywhere.
multiple-choiceanswer separatormatchdividersmath-inline/math-templatedcard strokelikertcolumn header (text)Why not the bridge the ticket names
packages/shared/theming-mui/src/mui-integration.tshas no callers —createPieMuiThemeis never invoked anywhere in this repo — so adding
greyto that mapping would havechanged nothing at runtime. The live theme mounts are
render-ui/src/ui-layout.tsxandconfig-ui/src/layout/config-layout.tsx, both generated from upstream.Remapping
palette.greyin those themes was the next idea and also does not work. MUI7.3.11's
Tooltipevaluatesalpha(theme.palette.grey[700], 0.92)when its poppermounts and throws
MUI: Unsupported var(...) color;Tooltipappears in 21 files.AppBar(getContrastText) and collapsedBreadcrumbs(emphasize) have the samehazard on shades 100/200/600/900. Leaving MUI's own greys as real hex and moving the call
sites through
color.*()avoids all of it, which is also the pattern PIE-818 establishedin
stimulus-tabs.tsx.Regeneration
Produced by
upstream:sync --elementper package against localpie-elementsandpie-libcheckouts, so every file here regenerates identically once both upstream PRsmerge.
That was not true of an earlier pass, and it is worth recording why. The sync was
deleting this repo's
isSelected/onChoiceClick/onPlacementClickselect-then-placeaffordance from eight
match-listandplacement-orderingfiles, because upstreamdevelopdid not carry it. It does now — pie-framework/pie-elements#3098 and #3099,PIE-769 and PIE-644 — so rebasing the upstream branch onto that resolved the conflict in
favour of keeping both the affordance and the colour change. Re-syncing both packages
against the rebased upstream then reproduced this tree exactly, so nothing here is
hand-applied.
Contract test
tests/pie-token-contract.test.tsgains the React half of the contract it alreadyenforced for the Svelte packages: every
color.*()accessor resolves to a registeredtoken, no React source names a
--pie-*outside the registered set, and no file readspalette.greyagain. Each of the three assertions was mutation-tested — reintroducing apalette.greyread, naming an unregistered token, and adding an unregistered accessoreach fail exactly one test.
Nine names predate this change and no scheme can reach them: the four
--pie-keypad-*,--pie-keyboard-focus-indicator,--pie-primary-text,--pie-secondary-text,--pie-prompt-holder-max-widthand--pie-zoom. They are recorded as known gaps ratherthan silently tolerated. Closing them needs registry entries in
pie-players, the samecross-repo blocker PIE-857 hit, so it is not attempted here.
Verification
tsc --noEmitclean.biomeat baseline parity — 0 errors, the same 19 pre-existingwarnings.
turbo run build79/79.vitest run548 passing. Dependency integrity cleanwith
--check-peer-gaps --fail-on-hoist --fail-on-peer.smoke-matrix.spec.tspasses: every element and view renders without a critical runtimeor build failure.
baseline-all-elements.spec.tsis 30 elements, 167 passing, 11 failing. All 11 arepre-existing, established by running each affected element at
HEADand again withpackages/reverted to the parent commit:multiple-choice— three gather-mode delivery checks time out, identically before andafter, while its evaluate-mode delivery check and both author views pass in both runs.
Reproduces in isolation, so it is not long-run degradation.
extended-text-entry,likert,math-inline— one failure each, the same"no visible correct-answer signal in evaluate mode" assertion, identical before and
after. That assertion also fails for
drawing-response,matrix,multi-trait-rubricandnumber-line, none of which this branch touches.Visible change
Strokes that were
#e0e0e0or#bdbdbdnow resolve to--pie-border, i.e.#8f8f8finthe default light theme. Deliberate — those values were under 3:1 before any scheme was
applied — but it is a design-visible change and worth a look before this ships.