fix(theme): rename variables.css to brand.css to stop replacing mdBook's bundled vars#15
Merged
Merged
Conversation
…k's bundled vars
The actual cause of the broken sidebar and the both-arrows-on-the-
same-side bug was not the mdBook version. It was a file-name
collision.
mdBook treats any file in `theme/css/` whose name matches one of
its bundled theme files (variables.css, general.css, chrome.css,
print.css) as a FULL REPLACEMENT for the bundled file, not as an
override that cascades on top. Our `theme/css/variables.css` had
the same name as mdBook's bundled `theme/css/variables.css`, so
mdBook used our file instead of its own. Our file only contained
the dark-palette overrides; every other mdBook variable
(`--page-padding`, `--sidebar-width`, `--sidebar-resize-indicator-width`,
`--menu-bar-height`, etc) was wiped out.
The visible consequences:
1. `.next { right: var(--page-padding) }` resolved to
`right: auto` because the var no longer existed. Both
`.previous` and `.next` fell back to the same default position,
producing the screenshot where both chapter arrows sat on the
right edge.
2. The sidebar width calculation `min(var(--sidebar-target-width),
80vw)` had no `--sidebar-width` aggregator to consume it, so
the sidebar collapsed to its content-min-width (about 30px),
wrapping each chapter title one character per line.
3. The chrome's resize-handle positioning relied on
`--sidebar-resize-indicator-width`, which was also gone.
Fix: rename `theme/css/variables.css` to `theme/css/brand.css`. No
mdBook bundled file uses that name, so mdBook keeps its own
variables.css in the cascade and adds ours on top via
`additional-css`. Verified locally:
- book/css/variables-*.css (mdBook's) now defines --page-padding,
--sidebar-target-width: 300px (mdBook default), --sidebar-width
formula, --menu-bar-height, etc.
- book/theme/css/brand-*.css (ours) overrides --sidebar-target-width
to 290px and the colour palette.
- The two files no longer have identical content (they had
matching md5 in the broken build because mdBook was using our
file as both the bundled file AND the additional file).
- The `.next { right: var(--page-padding) }` rule now resolves to
a real 15px and our `nav.nav-wide-wrapper a.nav-chapters.previous
{ left: var(--page-padding) }` rule wins specificity and pins the
previous arrow to the left edge.
Also documented the rule in book.toml so a future contributor does
not repeat the mistake.
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
1 participant
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.
Summary
Single-commit follow-up to #13. Renames the brand-palette override file from
theme/css/variables.csstotheme/css/brand.css. That is the actual fix for the broken sidebar and the both-arrows-on-the-same-side bug.What was happening
mdBook's theme override mechanism is full-file replacement, not a cascade. Any file you drop into
theme/css/whose name matches one of mdBook's bundled theme files (variables.css,general.css,chrome.css,print.css) replaces the bundled file entirely.Our
theme/css/variables.csscollided with mdBook's bundledtheme/css/variables.css. mdBook used ours instead of its own, which wiped out every default variable the layout depends on:--page-padding(used by.next { right: var(--page-padding) }to pin the next-chapter arrow to the right edge)--sidebar-width: min(var(--sidebar-target-width), 80vw)(the formula that computes the actual sidebar width)--sidebar-resize-indicator-width--menu-bar-heightVisible symptoms:
--page-paddinggone, the.nextrule resolved toright: auto. Both arrows fell to the same default fixed position.--sidebar-widthundefined, the sidebar fell back to its content min-width and wrapped each chapter title character by character.--sidebar-resize-indicator-width, which was also gone.The deeper version-bump in PR #13 (v0.4.40 → v0.5.3) and the dark-mode CSS changes were necessary but not sufficient. This rename is the actual root cause fix.
What this PR does
One file rename plus a single
book.tomlline:theme/css/variables.css→theme/css/brand.cssbook.tomladditional-cssupdated to point at the new namebook.tomlexplaining the collision rule so a future contributor does not repeat the mistakeConfirmed locally
After the fix:
book/css/variables-*.cssdefines--page-paddingbook/css/variables-*.cssvsbook/theme/css/brand-*.css--sidebar-target-width.next { right: var(--page-padding) }resolves toright: autoright: 15pxnav.nav-wide-wrapper a.nav-chapters.previous { left: var(--page-padding) }left: 15pxStacking
Targets
feat/mdbook-siteso it lands cleanly once #13 merges. If you merge #13 first, this becomes a no-op (the fix is already inside #13). If you prefer to land the fix as a separate change, merge this first, then rebase #13 onto main and the file rename is no longer in its diff.Test plan
bash scripts/build.shfrom a clean checkoutbook/css/variables-*.cssis mdBook's bundled file (contains--page-padding)book/theme/css/brand-*.cssis the brand override (contains the void palette)