Skip to content

fix: make the contents sidebar width independent of Tailwind emission order - #135

Closed
mmcky wants to merge 1 commit into
mainfrom
fix/sidebar-width-emission-order
Closed

fix: make the contents sidebar width independent of Tailwind emission order#135
mmcky wants to merge 1 commit into
mainfrom
fix/sidebar-width-emission-order

Conversation

@mmcky

@mmcky mmcky commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Deletes the stale unprefixed w-[250px] from the Contents sidebar, so the panel's width stops depending on Tailwind's rule-emission order.

The problem

ContentsSidebar.tsx put two unprefixed width utilities on the same element, split across two adjacent class strings:

'w-[350px] lg:w-[250px] 2xl:w-[350px]',
'h-screen w-[250px] z-[20] pt-[40px] pb-[90px] px-9',

The responsive line reads as the intent — 350px at base, 250px from lg, 350px again from 2xl. The trailing w-[250px] is a leftover from before that line existed. Both are unprefixed, both land in Tailwind's base layer, and neither is more specific, so which one applies below lg is decided purely by which rule Tailwind emits last.

The rendered result was correct today, but only by accident: Tailwind sorts arbitrary values as strings, [250px] sorts before [350px], and the 350px rule wins by landing later. That holds only while the two numbers keep their current relative order, and nothing pins it. Change the base width to anything sorting before 250px and the stale class silently takes over — the edit looks like it did nothing, and there is no error anywhere to explain why.

Verification

npm run build:css, then probing the emitted stylesheet:

probe before after
base-layer .w-[250px]{width:250px} present (offset 56418) absent (-1)
.w-[350px]{width:350px} 56468 56443
lg:w-[250px] present present
2xl:w-[350px] present present

The ambiguous base rule is gone and both responsive variants survive, so the bands still resolve 350 / 250 / 350 across base / lg / 2xl and nothing moves on screen. npm run compile is clean.

The visual job against the untouched -linux baselines is the real acceptance criterion here: since this is a no-op at render time, a green run is the proof. If it reports a diff, the diagnosis was wrong and this should not merge.

Locally: npm run compile clean, and the full visual suite is 19 passed, 0 failed with every committed baseline untouched — which is the render-level proof that nothing moved.

Note

If the visual suite behaves strangely for you locally, check for a leftover theme server before suspecting the branch: pkill -f "node ./server.js". A stale process holding port 3000 makes myst start proxy to whatever theme build it was launched with, which presents as widespread snapshot and DOM failures unrelated to the code under test. That is what it did to me here before I spotted it.

Closes #130

🤖 Generated with Claude Code

… order

The panel carried two unprefixed width utilities on the same element,
split across adjacent class strings: `w-[350px]` from the responsive
line and a stale `w-[250px]` on the next one. Both land in Tailwind's
base layer at the same specificity, so which applies below `lg` was
decided purely by which rule Tailwind emitted last.

The rendered result was correct today only by accident: Tailwind sorts
arbitrary values as strings, so `[250px]` sorts before `[350px]` and the
350px rule wins by landing later. Change the base width to anything
sorting earlier and the stale class silently takes over, with no error
anywhere to explain why the edit appeared to do nothing.

Deleting the stale utility leaves `w-[350px] lg:w-[250px] 2xl:w-[350px]`
as the single source of the width. Verified against a Tailwind build:
the ambiguous base-layer `.w-[250px]` rule is gone while the `lg` and
`2xl` variants are untouched, so the bands still resolve 350/250/350 and
no pixels move.

Closes #130

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 20, 2026 05:28

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes an unintended duplicate, unprefixed Tailwind width utility from the Contents sidebar so its base-width no longer depends on Tailwind’s arbitrary-value rule emission order.

Changes:

  • Removed the stale w-[250px] from the Contents sidebar container class list, leaving a single coherent w-[350px] lg:w-[250px] 2xl:w-[350px] width definition.
  • Added an inline comment explaining why duplicate unprefixed width utilities are problematic in this case (Tailwind emission-order dependence).
  • Documented the fix in CHANGELOG.md under Unreleased.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
CHANGELOG.md Adds an Unreleased “Fixed” entry describing the sidebar width emission-order issue and resolution.
app/components/ContentsSidebar.tsx Removes the duplicate unprefixed width utility and adds a comment clarifying the intended width behavior across breakpoints.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-22 00:22 UTC

@github-actions

Copy link
Copy Markdown
Contributor

🎭 Visual regression results

passed  17 passed
skipped  3 skipped

Details

stats  20 tests across 1 suite
duration  37.7 seconds
commit  8a0bc1a

Skipped tests

mobile-chrome › theme.spec.ts › QuantEcon theme — visual regression › launch-colab
mobile-chrome › theme.spec.ts › QuantEcon theme — visual regression › live-compute-toggle
mobile-chrome › theme.spec.ts › QuantEcon theme — visual regression › live-compute-toggle-absent-without-thebe

@mmcky

mmcky commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

Closing: superseded by #144, which deletes the w-[...] class string this PR disambiguates. The drawer widths are now declared once in styles/app.css via theme(screens.*), so the Tailwind emission-order dependency (#130) is gone rather than worked around.

@mmcky mmcky closed this Aug 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Contents sidebar carries two unprefixed width utilities, so its base width depends on Tailwind's emission order

2 participants