fix(theme): remove targeted the wrong theme, switching orphaned a block, dark topbar unreadable - #104
Merged
Merged
Conversation
…ck, dark topbar unreadable
Three defects reported from the RssReader test build (MXCLI-FINDINGS 15-17),
each fixed with a regression test proven to fail against the old behaviour.
15. `mxcli theme remove -p app.mpr` — the invocation the docs show — targeted
the built-in default rather than the theme actually installed. On a project
themed with ledger or console it reported every file as unchanged, exited 0,
and left the theme fully in place: a silent no-op on the documented command.
Both apply and remove now resolve the target from the mxcli:theme markers.
Remove has no fallback — an unthemed project is an error, not a no-op.
Apply keeps one, since a project with no theme is exactly when installing
the default is right; a bare apply on a themed project now refreshes that
theme instead of silently switching it to signal.
16. Switching themes left the outgoing theme's block in _mxcli-atlas-map.scss
and appended the incoming one beside it, doubling the file. Mine: the
protected-path branch in remove() returned without ever writing the
truncation it described. Harmless while the three Atlas maps are identical,
but it broke the "only one theme at a time" invariant that exists precisely
so two maps cannot fight in the cascade.
17. The topbar language selector measured 1.13:1 contrast in every dark palette
— invisible, not merely low. Two mistakes stacked. The guard was a bare
.current-language-text at (0,1,0) against Atlas's
.navbar-brand .widget-language-selector .current-language-text at (0,3,0),
so it never won. And `color: inherit` was the wrong value anyway: it
inherits body ink, which is dark, while the rail is dark in both palettes.
Now re-declared at matching specificity and resolved through the rail token.
Measured in a browser: 17.79:1 light, 19.47:1 dark.
The command-level bug in #15 needed a command-level test. cmd_theme_test.go
drives the real cobra command, because a test calling theme.Resolve directly
would keep passing while the CLI stayed broken — the same shape as the
"grep the call sites, not the helper" lesson already in fix-issue.md.
My own verification of #17 was too shallow to catch it: it read
getComputedStyle(el).color once, saw white and stopped. The probe now computes
the WCAG ratio against the first non-transparent ancestor background, which is
what turns "looks fine" into a number. Recorded in fix-issue.md.
Also documents a behaviour the report flagged but which is not a defect: apply
appends its block to the end of main.scss, after any @import the project already
had there, which matters to a project relying on import order rather than on
specificity.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JEZmExJUvn2nWTWE9mrd4i
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.
Follow-up to #102, which merged before these landed. Three defects reported from an independent test build (Feedline, in
ako/mxcli-rssreader— ~1,900 lines of MDL and ~1,400 lines of custom SCSS), each fixed with a regression test proven to fail against the old behaviour.theme removewith no name removed nothing and exited 0ledgerproject removes ledger; a second one errors_mxcli-atlas-map.scssremove()returned without writing the truncation it describedsignal → ledger → console, one block and 207 lines throughout (was 197 → 395)color: inheritwas the wrong value regardlessOn #17
The guard named the right element and still lost. Atlas's rule is
.navbar-brand .widget-language-selector .current-language-textat (0,3,0); mine was a bare.current-language-textat (0,1,0), which only appeared to work on layouts that do not nest the selector under.navbar-brand. Andcolor: inheritwas the wrong value regardless — it inherits body ink, which is dark, while the rail is dark in both palettes. Now re-declared at matching specificity and resolved through the rail token.My original verification was too shallow to catch this: it read
getComputedStyle(el).color, saw white, and stopped. The Playwright probe now computes the WCAG ratio against the first non-transparent ancestor background — the two lines that separate "looks fine" from 1.13 vs 19.47. Recorded as a symptom row infix-issue.md.Behaviour changes worth flagging
theme removewith no name resolves the installed theme from themxcli:thememarkers, and errors on an unthemed project rather than exiting 0.theme applywith no name refreshes the theme the project already has, falling back to the default only when it has none. Silently switching aledgerproject tosignalwas the same class of surprise as the remove bug.Testing
The #15 bug lived in the command's argument handling, not in the theme package, so a test calling
theme.Resolvedirectly would keep passing while the CLI stayed broken.cmd/mxcli/cmd_theme_test.godrives the real cobra command — the same shape as the "grep the call sites, not the helper" lesson already infix-issue.md.Also documented, as a caveat rather than a defect:
applyappends its block to the end ofmain.scss, after any@importthe project already had there. That matters to a project relying on import order rather than on specificity.Full suite,
check-mdl,gofmtandvetpass.🤖 Generated with Claude Code
https://claude.ai/code/session_01JEZmExJUvn2nWTWE9mrd4i
Generated by Claude Code