fix(ui): the first-run screen used four tokens from the marketing site's palette, not this one - #352
Merged
Merged
Conversation
…e's palette, not this one
It was never a flaky test. I called it transient twice and re-ran CI to green,
which is the exact behaviour a real failure hiding behind an intermittent one
depends on.
TestColourUtilitiesNameADeclaredToken was right every time it failed:
pages/AuthScreen.tsx bg-raised/40, border-line, bg-cross, bg-line
these name no --color-* token, so Tailwind emits nothing
All four are real tokens -- in web/src/styles/global.css, the MARKETING SITE's
stylesheet. ui/src/index.css is a different palette with different names, and I
carried the wrong vocabulary across when writing the password meter and its
notice in #346. Tailwind emits nothing for a class whose token does not exist,
so the notice box rendered with no border and no background, and two of the four
meter segments had no colour at all.
border-line -> border-border
bg-raised/40 -> bg-card-raised/40 (already used 3x in this UI)
bg-cross -> bg-warn
bg-line -> bg-border
WHY IT LOOKED INTERMITTENT, which is the part worth recording. #346 introduced
it and merged on a re-run that went green -- so the bug reached main. #348 then
inherited it by updating its branch, and failed on a job its own diff could not
possibly break: it touches only scripts/install.sh. That mismatch is what made
it read as infrastructure noise rather than a regression, and it is exactly the
signal I should have followed instead of dismissing.
The guard did its job. I overrode it twice.
Verified across three consecutive runs at 115/115 rather than one, because "it
passed once" is the evidence that produced this in the first place.
|
There was a problem hiding this comment.
Pull request overview
This PR fixes incorrect Tailwind color-token class names on the UI first-run/auth screen by switching from the marketing site palette vocabulary to the UI palette tokens, ensuring the password meter segments and the notice box render with the intended colors/border/background.
Changes:
- Replace non-existent Tailwind classes in the password strength meter from
bg-cross/bg-linetobg-warn/bg-border. - Replace non-existent notice box tokens from
border-line/bg-raised/40toborder-border/bg-card-raised/40.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
It was never a flaky test. I called it transient twice and re-ran CI to green — which is precisely the behaviour a real failure depends on to reach
main.theme.test.tswas right every time it failed:All four are real tokens — in
web/src/styles/global.css, the marketing site's stylesheet.ui/src/index.cssis a different palette with different names, and I carried the wrong vocabulary across when writing the password meter and its notice in #346.Tailwind emits nothing for a class whose token doesn't exist, so the notice box rendered with no border and no background, and two of the four meter segments had no colour.
border-lineborder-borderbg-raised/40bg-card-raised/40(already used 3× here)bg-crossbg-warnbg-linebg-borderWhy it looked intermittent
#346 introduced it and merged on a re-run that went green, so the bug reached
main. #348 then inherited it via a branch update and failed on a job its own diff cannot break — it touches onlyscripts/install.sh.That mismatch is what made it read as infrastructure noise. It was the opposite: the strongest available signal that the failure was real and came from somewhere other than the PR under test. I should have followed it rather than dismissed it.
The guard did its job. I overrode it twice.
Verified across three consecutive runs at 115/115, not one — "it passed once" is the evidence that produced this in the first place.