Skip to content

feat(styles): match lecture content typography to the existing builds - #155

Merged
mmcky merged 13 commits into
mainfrom
feature/quantecon-stylesheet
Sep 3, 2026
Merged

feat(styles): match lecture content typography to the existing builds#155
mmcky merged 13 commits into
mainfrom
feature/quantecon-stylesheet

Conversation

@DrDrij

@DrDrij DrDrij commented Aug 28, 2026

Copy link
Copy Markdown
Member

Matches the lecture content typography and spacing to the existing Sphinx builds, and fixes the config bug that was rendering inline code with literal backticks.

What's in here

A QuantEcon stylesheet, styles/quantecon.css. Imported last from styles/app.css, after @myst-theme/styles, so there is nothing upstream to conflict with when myst-theme is updated. Rules are taken one at a time out of the existing theme's bootstrap styles config, each value checked against the live Sphinx build: paragraph, list and figure rhythm; inline literals in the Sphinx colour (with the Sphinx dark-mode value under .dark) at a root-relative size; and the .auto / .terminal figure-sizing classes lecture sources already use. The file header documents how each rule outranks the typography plugin without !important, and why the sizing classes must stay unlayered.

Options considered and rejected: extending the typography plugin via tailwind.config.js (content styles end up in a JS config); replacing @myst-theme/styles/typography.css with our own copy (we'd own an upstream file that drifts every update); importing the individual @myst-theme/styles partials instead of the barrel (fragile if upstream adds or renames one); dropping prose off .article and writing the content styles from scratch (too much of the plugin to reimplement).

The tailwind.config.js fix. It was spreading themeExtensions.typography, which is a function, so the spread yielded {} and every upstream typography setting was silently dropped. That is why inline code rendered with backticks. Removing the key fixes it; the project settings it held moved into the new stylesheet.

Site footer and back-to-top button. The footer site part renders inside .article and inherited the content typography; it is now styled to the Sphinx footer, with every value measured off python-programming.quantecon.org (14.4px text at 70% opacity, inline licence badge, 36px clearance, content-column width matching its own rule, and a readable link colour in dark mode). BackToTop becomes the Sphinx ↑ Top pill, with aria-label="Back to top" kept for screen readers.

Metric-matched font fallback. A local-only Source Sans 3 Fallback face with size-adjust and ascent/descent overrides is declared in the critical CSS and appended to the sans stack, so the swap to the self-hosted webfont no longer reflows the page (measured text width goes from about 8% off target to under 0.5%).

Visual fixture. Gains a footer part so the footer is covered by the snapshots.

Content base size

.article is 1.125rem (18px), matching the Sphinx builds at desktop widths. The deciding evidence is the side-by-side comparison in the thread: at 1280px the line lengths and rhythm line up at 18px, while at 16px the column reads denser and the opening quotation wraps to four lines instead of three.

The root stays at 16px so the browser's own font-size preference still applies. The Sphinx builds reach 18px by stepping the root at 992px instead, which is why they ship their own font-size controls to compensate. A one-line .article pin in CRITICAL_CSS stops the first-paint jump from the 16px UA default.

Flat rather than matching the 992px breakpoint, so the size never shifts under the reader. The trade-off is that content runs larger than the live site below 992px, where Sphinx drops back to a 16px root.

DrDrij added 2 commits August 28, 2026 17:56
Introduce styles/quantecon.css as the single place to hand-edit lecture
content styling as plain CSS, imported last from styles/app.css. Content
design no longer needs to go through tailwind.config.js.

Brings content styles into line with the Sphinx lecture builds:

- base content size 1rem
- paragraphs 1rem margins; list items 0.5rem, nested paragraphs flush
- ul/ol vertical margins removed, so the surrounding elements set the
  rhythm and adjacent margins collapse to a single 1rem step
- figures 0 above, 1rem below, 0.5em padding
- inline literals at 1rem in the body font, #af5f5f, backticks suppressed
- .auto/.terminal sizing classes for figures, horizontally centred

The file header documents how to override @tailwindcss/typography without
!important: the plugin writes every rule as .article :where(x), and
:where() carries no specificity, so a plain .article x at (0,1,1) wins.

The two sizing classes are deliberately unlayered. Rules in
@layer components are tree-shaken unless Tailwind finds the class while
scanning `content`, and that list covers this theme and the myst-theme
packages but never lecture markdown -- so .terminal was being dropped
from the build entirely.
theme.extend.typography spread mystTheme.themeExtensions.typography,
which is a function -- spreading a function yields {}, because functions
have no enumerable own properties. Every upstream typography setting was
therefore discarded, silently: spreading a function is legal JavaScript,
so there was no error and no warning.

The key has been wrong since it was added, not broken by a later
upgrade. @myst-theme/styles has exported typography as a function in
every version this theme has depended on, 0.13.7 included.

Removing the key lets the earlier ...mystTheme.themeExtensions pass the
function through unchanged. That restores upstream's code, blockquote,
li, li > p and h5/h6 rules, among them the code::before/::after fix that
suppresses the literal backticks around inline code.

The project settings the key also carried move to styles/quantecon.css.
They keep their original (0,1,0) specificity via :where(), which matters:
heading anchors carry a font-normal utility and .link carries its own
colour and underline, and both won that tie before. A plain .article a
would be (0,1,1) and would wrongly override them.

The prose font-size and font-weight the key set are not carried over.
Both are already covered -- font-size by the .article rule in
quantecon.css, font-weight by the inherited default.

Computed colour, weight and text-decoration were captured for all
anchors in light and dark mode before and after; both match.
@github-actions

github-actions Bot commented Aug 28, 2026

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

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

🎭 Visual regression results

passed  17 passed
skipped  3 skipped

Details

stats  20 tests across 1 suite
duration  39.3 seconds
commit  29c04b1

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 29, 2026

Copy link
Copy Markdown
Contributor

Reviewed with Claude Code — the approach looks right (a last-imported styles/quantecon.css avoids forking anything upstream), and the tailwind.config.js fix is verified: themeExtensions.typography is indeed a function, so the old spread yielded {} and dropped all upstream typography settings — which explains the backtick bug exactly as described. The specificity notes in the file check out against the built preview CSS, including the unlayered .auto/.terminal surviving the purge.

Three things to confirm before merge:

  1. 16px vs 18px — the removed config set prose at 1.125rem (18px, matching the live Sphinx builds); the new sheet pins .article at 1rem. The UNITS comment shows it's deliberate, but it's the most visible change here — lecture text will render smaller than python-programming.quantecon.org. Just want an explicit sign-off on that.

  2. Light-mode link colour dropped — the old config gave article anchors rgb(0 73 121 / 0.8); the new .article :where(a) rule ports the font-weight/underline but not the colour, so anchors without .link now fall back to prose-stone's near-black --tw-prose-links. Possibly invisible in practice, but should be a conscious choice.

  3. Dark-mode inline code contrast--qe-literal-color: #af5f5f has no .dark variant and the rule beats prose-invert's pink token, so dark mode shows #af5f5f on stone-900 (~3.9:1, below WCAG AA 4.5:1). The DESIGN TOKENS comment already describes the fix — redefine the token under .dark with a lighter value.

The 12 visual-regression failures are expected (intentional typography change, stale baselines) — once the above are settled, eyeball the Playwright diffs and refresh with /update-snapshots.

@mmcky
mmcky requested a balanced review from Copilot August 29, 2026 00:12
@mmcky

mmcky commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

/update-snapshots

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 aligns lecture content typography and spacing in the MyST theme with the existing Sphinx lecture builds (python-programming.quantecon.org). It introduces a dedicated, plain-CSS stylesheet (styles/quantecon.css) imported last so QuantEcon overrides win the cascade among the app's imports, and it removes a broken typography extension from tailwind.config.js. The broken extension spread mystTheme.themeExtensions.typography (a function) into an object, yielding {} and silently dropping upstream prose settings (which is why inline code was rendering with literal backticks); removing the key restores the upstream typography, and the project-specific values it held were re-expressed as CSS.

Changes:

  • Add styles/quantecon.css with @layer base/@layer components rules (paragraph rhythm, anchors, inline-literal color, figure/list spacing) plus unlayered .auto/.terminal figure-sizing classes, all matched to the Sphinx build.
  • Remove the ineffective typography extension block from tailwind.config.js, fixing the dropped upstream prose settings (backtick removal).
  • Import the new stylesheet last in styles/app.css.

Reviewed changes

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

File Description
styles/quantecon.css New QuantEcon content-typography stylesheet, documented and matched to the Sphinx build.
tailwind.config.js Removes the no-op typography spread that was dropping upstream prose settings.
styles/app.css Adds the @import './quantecon.css' as the final import so overrides win the cascade.

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

@github-actions

Copy link
Copy Markdown
Contributor

🎭 Refreshed visual baselines in eaa7a23:

  • tests/visual/snapshots/desktop-chrome-linux/features.png
  • tests/visual/snapshots/desktop-chrome-linux/history-open.png
  • tests/visual/snapshots/desktop-chrome-linux/intro.png
  • tests/visual/snapshots/desktop-chrome-linux/launch-open.png
  • tests/visual/snapshots/desktop-chrome-linux/lists.png
  • tests/visual/snapshots/desktop-chrome-linux/notebook.png
  • tests/visual/snapshots/desktop-chrome-linux/sidebar-open.png
  • tests/visual/snapshots/mobile-chrome-linux/features.png
  • tests/visual/snapshots/mobile-chrome-linux/history-open.png
  • tests/visual/snapshots/mobile-chrome-linux/intro.png
  • tests/visual/snapshots/mobile-chrome-linux/lists.png
  • tests/visual/snapshots/mobile-chrome-linux/notebook.png
  • tests/visual/snapshots/mobile-chrome-linux/sidebar-open.png

@mmcky

mmcky commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

thanks @DrDrij this is looking heaps better

tests/visual/snapshots/desktop-chrome-linux/lists.png -- HORRAY!

I left a few minor comments.

Addresses review feedback on #155.

Typography
- `.article` is a flat 18px. The Sphinx builds step the root size by
  viewport (16px below 992px, 18px at and above); this theme keeps a 16px
  root so the browser's own font-size preference still applies, and pins
  the content size instead.
- Paragraph, list and figure spacing move from `rem` to `em` so they scale
  with the content. These reproduce the Sphinx values at 18px: `p` 18px
  (its UA `margin: revert`), `li` 9px (`.5rem` against an 18px root).
- Inline code stays root-relative. The Sphinx build sizes it `.9rem` off
  its own root (16.2px), fixed regardless of the surrounding text, so `em`
  here would wrongly inherit the heading scale.
- Dark-mode inline literals take the Sphinx theme's own dark value. The
  light #af5f5f scores 3.85:1 on stone-900, under the 4.5:1 AA floor;
  #d78787 is 6.42:1.

No `--tw-prose-links`: the anchor colour dropped from `tailwind.config.js`
reads as a regression but never reached the page. Every content anchor
carries a class that recolours it from the utilities layer, and the one
classless anchor is the project title, which `ProjectFrontmatter.tsx`
deliberately neutralises with `prose-a:text-inherit`. Forcing the token to
a sentinel changed 0 of 35 anchors across four fixture pages.

First-paint reflow
- `CRITICAL_CSS` pins the content size, so the article no longer paints at
  the UA default 16px and jumps to 18px when the Tailwind bundle lands.
- Adds a metric-matched `Source Sans 3 Fallback` face. The webfont ships
  `font-display: swap`, and raw Helvetica/Arial run ~8% wider than Source
  Sans 3, which is what made that swap a visible reflow. With the woff2
  blocked, measured text width goes from 8.16% off target to 0.26%. The
  face is `local()`-only so it costs no request, and it is declared in
  `CRITICAL_CSS` so it applies from the first paint.
@DrDrij

DrDrij commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

/update-snapshots

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

🎭 Refreshed visual baselines in cc7775b:

  • tests/visual/snapshots/desktop-chrome-linux/features.png
  • tests/visual/snapshots/desktop-chrome-linux/history-open.png
  • tests/visual/snapshots/desktop-chrome-linux/intro.png
  • tests/visual/snapshots/desktop-chrome-linux/launch-open.png
  • tests/visual/snapshots/desktop-chrome-linux/lists.png
  • tests/visual/snapshots/desktop-chrome-linux/notebook.png
  • tests/visual/snapshots/desktop-chrome-linux/sidebar-open.png
  • tests/visual/snapshots/mobile-chrome-linux/features.png
  • tests/visual/snapshots/mobile-chrome-linux/history-open.png
  • tests/visual/snapshots/mobile-chrome-linux/intro.png
  • tests/visual/snapshots/mobile-chrome-linux/lists.png
  • tests/visual/snapshots/mobile-chrome-linux/notebook.png
  • tests/visual/snapshots/mobile-chrome-linux/sidebar-open.png

The `footer` site part renders inside `.article`, so it inherited the content
typography -- 18px text on an 18px rhythm, prose link colours and a badge
centred by the prose image rules. These rules put it back to the Sphinx
footer, with every value measured off python-programming.quantecon.org
rather than eyeballed:

- 14.4px/1.5 text at #444, 14.4px paragraph margins
- #0072bc links, no underline
- inline badge, no auto-centring margins
- 36px above the rule and below the last line
- `opacity: .7`, which the Sphinx footer carries on the whole element

That last one is easy to miss. Without it the declared colours are correct
and still render wrong: the rule samples rgb(0 114 188) against the Sphinx
build's rgb(75 155 207), because everything there is drawn at 70%.

Selectors are `.article .qe-site-footer`, (0,2,0), so they outrank the
utilities already on the element (`text-qetext-light`, `text-opacity-80`,
and `.link` on the badge anchor) on specificity rather than relying on
source order. Both edge gaps are padding, not margin: `styles/app.css` sets
`margin-top: 0 !important` on every `.simple-center-grid > *`, and a bottom
margin on the last grid child would not extend the document.

`SiteFooter` also moves its content column from `col-screen` to `col-body`.
The 5px rule above it is already `col-body`, so the text was running wider
than its own rule.

Note this is dormant for now: the footer renders only when a site supplies
`site.parts.footer`, and no current consumer does. PLAN.md lists the
content-driven footer at parity, which holds for the mechanism but not for
anything a reader sees.
`BackToTop` rendered a bare, unclassed `<a>` inside `.article`, so it picked
up the 18px content size and the surrounding stone text colour. Styled to
match the Sphinx build's `.back-to-top-btn`, measured live rather than read
off the stylesheet: #0072bc, 15.3px, weight 600, line-height 1.5, 9px 18px
padding, 4px radius, no underline. The rendered box comes out 73x41,
identical to the original.

`line-height` is the one that does not fall out of the other values. Prose
otherwise gives the link 1.75 (26.8px) and the box lands at 45px against the
Sphinx build's 41px -- visible as a taller pill, but only if the box is
measured.

The label matches too: U+2191 followed by " Top", with `title="Back to top"`
as in the Sphinx markup. `aria-label` is added rather than copied. The
original leans on `title` alone, which screen readers announce
inconsistently, so shortening the visible label to an arrow and a word would
otherwise shrink the accessible name along with it.

The colour selector is deliberately only (0,1,1). `.dark .article :where(a)`
is (0,2,0) and so still wins in dark mode, keeping the existing white link
rather than painting this dark blue on a dark background.
Nothing exercised the site footer. It renders only when a project supplies
`site.parts.footer` (PageContent.tsx), and no consumer does, so the footer
styling shipped in f1a4585 had no coverage at all -- a later change could
break it with nothing going red.

The fixture is the one piece of content this repo owns, so it declares the
part here. Content mirrors the Sphinx build's footer: licence badge, the
CC BY-SA 4.0 line, and the theme credit.

The badge is vendored rather than hot-linked to licensebuttons.net. This is
a visual-regression fixture, and a remote image makes the snapshots depend
on a third party being reachable and byte-stable; an outage or a silent
re-encode upstream would fail the suite for reasons unrelated to the theme.
It was also the fixture's only build-time network fetch -- the other two
URLs in the fixture are hyperlinks, never fetched -- so vendoring restores
a hermetic build. Verified: 0 requests to licensebuttons.net, badge still
80x15 natural and rendered.

The en dash is a literal U+2013 rather than `&ndash;`, matching what the
Sphinx build renders without routing prose through an HTML entity.

This changes every full-page baseline: the footer now appears at the bottom
of each screenshot, so the snapshots need refreshing with /update-snapshots
once this reaches CI.
Reverts the content base to 16px, undoing the size half of 0dbfcfe. The
Sphinx builds step their root by viewport (16px below 992px, 18px above);
this theme now keeps a flat 16px at every width rather than matching the
desktop figure. Lecture text is smaller than the Sphinx build above 992px,
which is a deliberate trade -- the root is left alone so the browser's own
font-size preference still applies.

Two consequences worth spelling out.

The `.article` pin in `CRITICAL_CSS` is removed along with it. It existed
only because 18px content differs from the UA default and so jumped on first
paint once the Tailwind bundle landed. At 16px there is no jump to guard, and
a rule that pins a value to the value it already has implies protection it
does not provide. The metric-matched font fallback stays -- that addresses
the font swap, which is a separate problem and still real.

For the same reason the font-size assertion in `fouc.spec.ts` is not kept.
With content at 16px the guarded state and the control state are both 16px,
so the assertion would pass whether or not the rule it guards exists. A check
that cannot fail is worse than no check, because it reads like coverage.

The footer and the back-to-top button move from `em` to `rem`. Both were
sized against the content base to land on the Sphinx build's 14.4px and
15.3px; with the base at 16px an `em` would have quietly shrunk them to
12.8px and 13.6px. Pinning them to the root holds them at the sizes measured
off python-programming.quantecon.org, and says in the comment that they are
chrome and should not track content. Verified after the change: body text
16px on 16px margins, footer 14.4px, button 15.3px in a 41px box.
@DrDrij

DrDrij commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

@mmcky Could you please run another review?

@mmcky

mmcky commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Second review pass (with Claude Code) on e33a494. The cascade reasoning in styles/quantecon.css checks out against the built preview CSS, the footer and back-to-top values match what I measured on python-programming.quantecon.org with Playwright, and the refreshed screenshots look right. Thanks @DrDrij, this is close.

Decision on 16px vs 18px: let's go with 18px. I rendered the numpy lecture three ways at a 1280px viewport: this branch as-is, the same build with .article { font-size: 1.125rem } injected, and the live Sphinx page. Side by side with measurements here: https://claude.ai/code/artifact/677b247f-5d18-4157-beee-635c9fd3d031 (live sources: preview and Sphinx). At 18px the MyST and Sphinx pages share the same line lengths and rhythm; at 16px the column reads denser and the opening quotation wraps to four lines instead of three. Since parity with the live sites is the purpose of this PR, please set .article to 1.125rem and restore the one-line CRITICAL_CSS pin from 0dbfcfe. Happy to hear a counter-argument, but note the UNITS rationale isn't load-bearing: leaving the root alone applies equally to an .article at 1.125rem, and the first-paint jump was already solved by that pin. Please review and comment.

Remaining items before merge:

  1. Inline code font family. .article :not(pre) > code sets font-family: inherit, but the live build renders inline literals in Source Code Pro (computed on numpy.html: "Source Code Pro", monospace, 16.2px, #af5f5f). Dropping that declaration restores the monospace stack. Self-hosting Source Code Pro could be a follow-up in the style of perf: self-host the Source Sans 3 webfont #140.

  2. Footer links in dark mode. .article .qe-site-footer a is (0,2,1), so it beats both .link's dark colour and .dark .article :where(a), leaving #0072bc at 70% opacity on stone-900, about 2.3:1. Real footers do carry a text link ("A theme by QuantEcon"). A .dark .article .qe-site-footer a rule using the theme's blue-400 fixes it.

  3. CHANGELOG entry. CONTRIBUTING asks for one under Unreleased. This PR has four user-visible changes to record: the typography sheet plus the backtick fix, the footer styling, the back-to-top button, and the metric-matched fallback face.

  4. Comments that went stale with the 16px revert. Moving to 18px makes some of these true again, but please re-read the file once the size is settled: the footer header ("18px text on an 18px rhythm"), the back-to-top header ("inherits the 18px content size"), the li rule reasoning from 18px next to an /* 8px */ annotation, the p rule's "reproduces it exactly", and the footer color: #444 note, since text-opacity-80 is dead once the colour is overridden and the comment shouldn't imply it still applies.

  5. PR description covers only the first two commits. The squash message comes from it, so please add the footer, back-to-top and font fallback.

Then one more /update-snapshots (the current linux baselines predate the footer and back-to-top commits, hence the 12 failures on e33a494), and mark the PR ready for review.

One aside, not for this PR: the preview renders the lecture's raw qe-notebook-header HTML block as literal text under the title, and the title appears twice. I haven't checked whether main does the same; worth its own issue either way.

@mmcky

mmcky commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

@DrDrij just about to push some suggested updates as a commit to address the minor feedback items.

Addresses the second review pass on #155.

- Inline literals drop `font-family: inherit`. The Sphinx build sets them
  in Source Code Pro, monospace (measured on python-programming.quantecon.org:
  "Source Code Pro", monospace, 16.2px), so the monospace family already on
  `code` from Tailwind's preflight is the right one. The comment now says so.
- Footer links get a dark-mode colour. `.article .qe-site-footer a` is
  (0,2,1) and beat both `.link`'s dark utility and `.dark .article :where(a)`,
  leaving #0072bc at 70% opacity on stone-900 (about 2.3:1). blue-300 lands
  near 5.3:1 at the same opacity.
- Comments that argued from 18px after the 16px revert are made size-neutral,
  and the footer `color: #444` note no longer implies `text-opacity-80` still
  applies once the colour is overridden.
- CHANGELOG entries under Unreleased for the typography sheet, the backtick
  fix, the footer and back-to-top styling, and the fallback face.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@mmcky

mmcky commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

/update-snapshots

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

🎭 Refreshed visual baselines in 868a992:

  • tests/visual/snapshots/desktop-chrome-linux/features.png
  • tests/visual/snapshots/desktop-chrome-linux/history-open.png
  • tests/visual/snapshots/desktop-chrome-linux/intro.png
  • tests/visual/snapshots/desktop-chrome-linux/launch-open.png
  • tests/visual/snapshots/desktop-chrome-linux/lists.png
  • tests/visual/snapshots/desktop-chrome-linux/notebook.png
  • tests/visual/snapshots/desktop-chrome-linux/sidebar-open.png
  • tests/visual/snapshots/mobile-chrome-linux/features.png
  • tests/visual/snapshots/mobile-chrome-linux/history-open.png
  • tests/visual/snapshots/mobile-chrome-linux/intro.png
  • tests/visual/snapshots/mobile-chrome-linux/lists.png
  • tests/visual/snapshots/mobile-chrome-linux/notebook.png
  • tests/visual/snapshots/mobile-chrome-linux/sidebar-open.png

@mmcky
mmcky marked this pull request as ready for review September 2, 2026 05:26
@mmcky

mmcky commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

@DrDrij over to you now.

@mmcky

mmcky commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

@DrDrij to save you a round trip I've landed the uncontroversial items from the review above in 4667d9e (with Claude Code), refreshed the snapshots (868a992), and rewritten the PR description to cover the footer, back-to-top and font fallback. CI is green.

  • Item 1: inline literals drop font-family: inherit and use the monospace stack, matching the Sphinx build's Source Code Pro. Confirmed in the new screenshots.
  • Item 2: .dark .article .qe-site-footer a sets blue-300, about 5.3:1 at the footer's 70% opacity.
  • Item 3: CHANGELOG entries under Unreleased (Added, Changed, Fixed).
  • Item 4: the size comments are now size-neutral, and the color: #444 note no longer implies text-opacity-80 still applies.
  • Item 5: PR description updated.

Still yours: the 16px vs 18px decision (recommendation is 18px, see the comparison linked above), then mark the PR ready for review. If 18px lands, please re-read the size annotations in the stylesheet comments and run /update-snapshots once more.

Settles the content base size at 18px, per the review on e33a494. The
rendered comparison there is the deciding evidence: at 1280px the MyST and
Sphinx pages share line lengths and rhythm at 18px, and at 16px the column
reads denser with the opening quotation wrapping to four lines instead of
three. Parity with the live builds is the purpose of this branch.

The UNITS rationale that argued for 16px does not survive scrutiny and has
been rewritten. Leaving the root at 16px so the browser's font-size
preference still applies is true of `.article: 1.125rem` just as much as
`1rem` -- the theme sets the content element, not `html` -- so that argument
never distinguished the two sizes.

Restores the one-line `.article` pin in `CRITICAL_CSS`. At 18px the content
size differs from the UA default again, so without it the article paints at
16px and jumps when the Tailwind bundle lands.

The Sphinx builds reach 18px by stepping the *root* at 992px, so they render
16px below that while this stays flat at 18px. Flat is deliberate -- the size
never shifts under the reader -- but it does mean content runs larger than the
live site on narrow viewports, and the UNITS comment now says so rather than
implying parity at every width.

Nothing else moves: the footer, the back-to-top button and inline code are
all root-relative, so they hold at 14.4px, 15.3px and 16px. Verified after
the change: body 18px on 18px margins, list items 9px.
@DrDrij

DrDrij commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

@/tmp/pr-comment.md

@DrDrij

DrDrij commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

/update-snapshots

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

🎭 Refreshed visual baselines in 29c04b1:

  • tests/visual/snapshots/desktop-chrome-linux/features.png
  • tests/visual/snapshots/desktop-chrome-linux/history-open.png
  • tests/visual/snapshots/desktop-chrome-linux/intro.png
  • tests/visual/snapshots/desktop-chrome-linux/launch-open.png
  • tests/visual/snapshots/desktop-chrome-linux/lists.png
  • tests/visual/snapshots/desktop-chrome-linux/notebook.png
  • tests/visual/snapshots/desktop-chrome-linux/sidebar-open.png
  • tests/visual/snapshots/mobile-chrome-linux/features.png
  • tests/visual/snapshots/mobile-chrome-linux/history-open.png
  • tests/visual/snapshots/mobile-chrome-linux/intro.png
  • tests/visual/snapshots/mobile-chrome-linux/lists.png
  • tests/visual/snapshots/mobile-chrome-linux/notebook.png
  • tests/visual/snapshots/mobile-chrome-linux/sidebar-open.png

@mmcky

mmcky commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

@DrDrij looking good. I'll merge this in the morning.

@mmcky
mmcky merged commit 0842d0d into main Sep 3, 2026
4 checks passed
@mmcky
mmcky deleted the feature/quantecon-stylesheet branch September 3, 2026 00:30
mmcky added a commit that referenced this pull request Sep 3, 2026
…nt, harden tests

Follow-up to the 2026-08-25 review of #144, applying the items that need no
design decision. Light dismiss (`popover="auto"`) is accepted as-is.

- Close the drawer when a modal dialog mounts. A popover paints in the top
  layer above the search dialog's backdrop and panel, and light dismiss cannot
  cover the Cmd/Ctrl+K route because it fires on pointerdown. A shallow
  childList observer on <body> catches Radix's portal container and calls
  hidePopover(). New test `drawer-closes-when-search-opens` pins it.
- Hide `.qe-toc-toggle__close` in CRITICAL_CSS so both icons no longer paint
  side by side on the pre-app.css frame, and make the "needs no rule here"
  comment true. The FOUC guard now samples the icon: hidden in the main test,
  rendered in the control, which proves the inline rule does work.
- `drawer-opens-without-javascript` moves into a `test.use({ javaScriptEnabled:
  false })` describe: the ordinary page fixture with the project's own baseURL,
  and a `toHaveCount(1)` presence guard before `toBeHidden`, which otherwise
  passes on zero matches.
- The drawer title is a styled div again, not an <h2> that precedes every
  page's <h1> in DOM order while the drawer is open.
- Reword the toggle comment: the browser exposes the expanded state in the
  accessibility tree; the DOM `aria-expanded` attribute reads null by design.
- Note in app.css that the rem widths scale with the A+/A- control on purpose.
- Changelog entries under [Unreleased] for the rebuild, its behaviour and
  browser-support changes, and the three toolbar/a11y fixes.

Also merges main so the branch carries #155's critical CSS and changelog.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
mmcky added a commit that referenced this pull request Sep 4, 2026
Move the [Unreleased] entries under ## [2.4.0] - 2026-09-04 with a headline
blockquote, add the compare link, re-point the [Unreleased] compare base at
v2.4.0, and bump package.json (npm version 2.4.0 --no-git-tag-version).
template.yml is stamped by release.yml at build time.

Minor rather than patch: #155 adds the site footer part and back-to-top
styling alongside its fixes.

The release carries three commits that have been on main since 2026-09-03:

- #151 completes the static-build asset fix that v2.3.1 began. #139 covered
  the stylesheets in _assets/, but Remix also emits route and shared-chunk
  CSS, and four such files shipped in 2.3.1 still pointing --jp-icon-plotly
  at an absolute /myst_assets_folder/ URL. A production build now emits zero
  absolute asset URLs and all 78 references resolve.
- #155 brings lecture content typography into line with the Sphinx builds and
  fixes inline code rendering wrapped in literal backticks — a regression that
  reached every code span on every page.
- #161 bumps fast-uri 3.1.5 to 3.1.7.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
mmcky added a commit that referenced this pull request Sep 4, 2026
* refactor: rebuild the contents drawer on the Popover API

Every bug this panel has produced — the flash on load, the animated
correction, the platform-dependent guard assertion, the icon fade in
#127, the width trap in #130 — came from one premise: the panel was
always rendered and always painted, with visibility expressed as a
transform that only exists once app.css has loaded. That guarantees an
unstyled frame where it is visible and a correction afterwards that a
transition will animate. Each fix so far has patched a symptom of it.

A popover is hidden by the UA stylesheet while closed, so it is already
invisible on the very first paint with no author CSS at all. There is no
unstyled frame to correct, and so nothing for a transition to animate.
The failure mode stops existing rather than being guarded against.

What this removes:

- `useNavOpen` from both components. There is no open/closed state in
  React at all now, so the drawer works before hydration and the button
  cannot desync from the panel.
- The `useMounted` transition gate — nothing left to gate.
- The `.qe-contents-sidebar` critical-CSS rule and its sync-list entry.
- `useSidebarHeight` from both call sites. It bails unless one call site
  holds both its refs; ours each attached one, so both instances
  early-returned on every scroll event and did nothing.
- The duplicate width utilities, so #130 cannot recur — the three widths
  are expressed once in CSS with no dependence on emission order.
- The icon cross-fade, replaced by a `display` swap driven off
  `:popover-open` via `:has()`, so it cannot animate on first paint.

Behaviour the browser now supplies for free: Escape, click-outside
dismissal, the closed panel kept out of the tab order and the
accessibility tree, and the invoker's expanded state (verified in the AX
tree as false -> true). Both icons are aria-hidden and the button holds
the single accessible name; previously each icon carried its own label,
so a screen reader could announce both "Show" and "Hide" for one control.

Verified against a real build in Chromium and WebKit: hidden on an
unstyled first paint (0 boxes, display none), nothing animates when
app.css arrives, slides 18-19 frames to x=0 on open, sized 250x750 in
both, Escape and light dismiss both close it. The existing visual
baselines pass untouched in both projects, so the rendered result is
pixel-identical to what it replaces; test:fouc passes 2/2.

Two details worth keeping in mind. The UA stylesheet sizes popovers
`fit-content` in both axes, so height has to be reset to `auto` or the
panel stops at its content instead of stretching to `inset-block`. And
React 18's JSX types predate the API, so `popover`/`popovertarget` need
the declarations in app/popover.d.ts (react-dom renders them correctly —
both survive renderToString on 18.3.1).

Non-supporting browsers (iOS <= 16, Firefox < 125) treat the attribute
as inert and would render the panel permanently open, so it is hidden
outright there via @supports; loading a popover polyfill would restore
it if that tail matters.

* fix(a11y): give the contents toggle a visible focus ring

Closing the drawer with Escape returns focus to the toggle, and the
browser matches :focus-visible because the interaction was a keypress.
The UA default outline is a tight 1px square that reads as a rendering
glitch against the toolbar, so restyle it in the theme's blue with an
offset and a small radius. Removing it was the alternative, but that
leaves keyboard users on an unmarked control (WCAG 2.4.7); pointer users
never see it, since a click does not match :focus-visible.

* fix(toolbar): stop the logo distorting and the controls overflowing

Two related layout bugs in the band just above the md breakpoint.

Every desktop control switches on at 768px, but at 20px spacing the
row's intrinsic width is about 856px, so between 768 and 856 the flex
row had no slack. The only item that could absorb it was the logo: an
img with a pinned height and an auto width, which Tailwind's preflight
also gives max-width: 100%. It shrank with its flex item and the fixed
height then distorted the aspect ratio, down to 0.93 against a natural
3.5 at 770px.

Pin the logo with shrink-0 so it can never distort, and widen the item
gap, the actions-cluster separator and the container padding at lg
rather than md, which returns about 148px to the narrow band. Without
that second half, shrink-0 alone just moves the deficit to the end of
the row and pushes the last icons off the right edge.

Measured 360-1400px: aspect holds at 3.5 throughout, no overflow at any
width, and 58px spare at the tightest point — enough for the 32px the
live-compute toggle adds on notebook pages.

* test: anchor the no-thebe sanity check to the heading

The check that the notebook page actually loaded used
getByText("Notebook outputs").first(), which resolved to the contents
drawer's link to that page rather than its heading. It passed only
because the closed drawer used to be rendered and translated off-screen,
which Playwright counts as visible; as a popover the closed drawer is
display: none, so the link is correctly hidden and the assertion failed.

Match the heading instead, which is what the check was always after.

* fix(a11y): stop offering a contents toggle that cannot open

Review follow-ups on the popover drawer.

The @supports fallback hid the panel on browsers without the Popover API
but left the toggle visible, so those users got a button with an
accessible name that did nothing when pressed. Hide the toggle with the
panel: an absent control is better than a dead one. Its comment also
claimed a polyfill in entry.client.tsx restored the drawer there — no
such polyfill exists, and adding one is not a drop-in, since
:popover-open is a pseudo-class and cannot be polyfilled. The comment
now states the trade-off and what reversing it would cost, instead of
describing work that was never done.

The affected set is Firefox < 125 and Safari < 17, which includes every
device that cannot be upgraded past iOS 16. Those browsers keep the
in-page outline and the site navigation, so content stays reachable.

Also:
  - the panel no longer imports its id from the button that opens it;
    both take it from a shared module
  - the drawer's <nav> is labelled by its own heading rather than
    repeating the button's "Table of contents", which screen readers
    announced twice
  - the lg/2xl breakpoints come from theme(screens.*) rather than being
    transcribed as 1024px/1536px, which would desync silently
  - reduced motion drops the transition entirely instead of shortening
    it to 1ms; display/overlay only need transitioning so the slide can
    outlast display:none, and under the preference there is no slide

* test: assert the contents drawer opens without JavaScript

The reason the drawer is a popover is that the browser owns the toggle,
so it works on the server-rendered HTML before or without hydration —
that is what removes the open/closed flash. Every other test runs with
JavaScript on, where a React-state implementation passes identically,
so the property was undefended: adding an onClick or a client-only
wrapper would have lost it silently.

Verified the test can fail: with the markup as shipped it passes, and
with popovertarget stripped from the served HTML the drawer stays shut
and the assertion goes red.

* docs: trim drawer comments to what future work needs

The comments explained the change rather than the code — how the old
implementation behaved, what was measured, why one approach was rejected.
That belongs in commit messages and PR history, not in files someone
reads to make the next change.

Cut to the constraints that still bind: what must stay in sync, what
must not be added, what breaks if a rule is dropped. Roughly 70 lines
removed, and the drawer's CSS commentary is about half its previous
size.

Also drops the `inset` prop from NavigationAndArticleWrapper. It existed
only to feed useSidebarHeight, no caller ever passed it, and with that
hook gone it was dead plumbing kept alive by the comment above it.

* fix(drawer): close on modal dialogs, hide the close icon at first paint, harden tests

Follow-up to the 2026-08-25 review of #144, applying the items that need no
design decision. Light dismiss (`popover="auto"`) is accepted as-is.

- Close the drawer when a modal dialog mounts. A popover paints in the top
  layer above the search dialog's backdrop and panel, and light dismiss cannot
  cover the Cmd/Ctrl+K route because it fires on pointerdown. A shallow
  childList observer on <body> catches Radix's portal container and calls
  hidePopover(). New test `drawer-closes-when-search-opens` pins it.
- Hide `.qe-toc-toggle__close` in CRITICAL_CSS so both icons no longer paint
  side by side on the pre-app.css frame, and make the "needs no rule here"
  comment true. The FOUC guard now samples the icon: hidden in the main test,
  rendered in the control, which proves the inline rule does work.
- `drawer-opens-without-javascript` moves into a `test.use({ javaScriptEnabled:
  false })` describe: the ordinary page fixture with the project's own baseURL,
  and a `toHaveCount(1)` presence guard before `toBeHidden`, which otherwise
  passes on zero matches.
- The drawer title is a styled div again, not an <h2> that precedes every
  page's <h1> in DOM order while the drawer is open.
- Reword the toggle comment: the browser exposes the expanded state in the
  accessibility tree; the DOM `aria-expanded` attribute reads null by design.
- Note in app.css that the rem widths scale with the A+/A- control on purpose.
- Changelog entries under [Unreleased] for the rebuild, its behaviour and
  browser-support changes, and the three toolbar/a11y fixes.

Also merges main so the branch carries #155's critical CSS and changelog.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Matt McKay <mmcky@users.noreply.github.com>
Co-authored-by: Matt McKay <mamckay@gmail.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
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.

3 participants