Generate the render stylesheet so both routes share one source - #3905
Open
rossnelson wants to merge 8 commits into
Open
Generate the render stylesheet so both routes share one source#3905rossnelson wants to merge 8 commits into
rossnelson wants to merge 8 commits into
Conversation
`Preview` renders into an iframe served by `/render`, which pads the page by 1rem and lays out `main`/`p` as blocks. That is right for a document and wrong for a short string: a one-line summary arrives in a padded box, and the padding is counted into the measured frame height. Adds two props: - `compact` drops the page padding and lets the block wrappers flow, so a short string renders as a line of text. Wrapping stays on, so content of a sentence or two still fits its frame rather than being clipped. - `title` sets the frame's accessible name, which was hardcoded to "output". A page with several of these frames announced them all identically. Also sets `border-0` on the iframe, which otherwise picks up the UA default border in some browsers. Both props are additive and default to today's behaviour. The approach here comes from @Tidemand's work in #3865, which needs this same body-class plumbing for inline event summaries. This is deliberately narrower: no shrink-to-fit width measurement, so that PR's inline mode can layer on top rather than conflict with it.
Measuring the first consumer in a browser turned up three things the
initial pass missed.
The reset sizes every element through a `*` selector, which beats
inheritance, so `body.compact { font-size }` never reached `main`, `p` or
the inline elements — content rendered at 12.25px next to 14px text
outside the frame. The metrics are now restated for the elements a short
string can contain.
`main` and `p` go back to blocks with the margins zeroed rather than
`display: inline`. `getRenderedHeight` measures `main`, and an inline box
reports the text box rather than the line box: two pixels short of the
leading, which clips descenders and the background of a code span.
Inline code keeps its horizontal padding and drops the vertical, which
otherwise grows the line box past the height the frame was sized to.
Measured on a one-line string with bold, a code span and a link: the frame
went from 65px to 23px, with nothing clipped and the type matching its
surroundings.
The star selector at the top of the reset sets font-weight on every element, which beats the UA default for strong. Nothing restored it, so `**bold**` has been rendering at normal weight on every markdown surface — event summaries and incident summaries included, not just the compact mode this PR adds. Found while checking a consumer. Also merges main for the Io colour system. The reset now resolves code colours through --color-surface-secondary and --color-content-primary, and the compact rules here still hold against it: they set layout and metrics, not colour.
The reset set no overflow-wrap anywhere, so a long URL in a summary ran past its container - and in compact mode, where the body clips, it simply disappeared off the edge. Same bug as the bold one: it affects every markdown surface, not just the compact mode this PR adds. A consumer hit it, was fixed there, and this is the half that belongs upstream.
The ui-server serves /render from its own Go handler with the stylesheet inlined as a template literal, so the compact changes to src/markdown.reset.css only took effect under the SvelteKit dev server. A packaged binary kept the 1rem page padding and the opaque box the compact class exists to remove. Ports the compact block, reads the compact query param with the same exact-string parsing the SvelteKit route uses, and adds overflow-wrap so a long unbroken token in a 255-character description wraps instead of overflowing the frame. Also pins strong/b to 600. Bold already worked here via the UA default, since unlike the SvelteKit reset this star selector sets no font-weight, but the UA's 700 rendered heavier than the same markdown does in dev. The two stylesheets cannot be fully unified while this handler has no way to read the Io theme variables the SvelteKit copy resolves colors through, so a test holds the colorless compact block to parity and names any selector that goes missing from it.
The compact mode shipped here painted its own canvas, so an embedder on any surface other than the default got an opaque box: white on a dark page. The fix has two halves and needs both. body.compact drops the background, and the root declares a color-scheme matching the theme on body, because a mismatch between the document's scheme and the one the page gave the iframe element makes the browser paint an opaque canvas regardless of what the document asks for. Ports both halves to the Go render route as well, for the same reason the compact styles went there: a packaged ui-server serves /render from Go, not from SvelteKit.
/render has two implementations. A packaged ui-server serves it from Go, and the SvelteKit route only runs where there is a SvelteKit server, so both need the same CSS. The Go one carried a copy pasted in when the route was added in #2430 and it has been drifting ever since: every change to src/markdown.reset.css since #2796 landed on one side only. That is not a difference anyone chose. It means a packaged server is missing #3430's a11y fix for font metrics, #3463's nested-list fix, #3673's table styles, and the whole Io palette from #3849, which is why it still renders markdown in the 2024 Holocene blue. The colours are the reason it could not simply be kept in sync by hand: they only exist as TypeScript, and server/ is a separate Go module, so //go:embed cannot reach src/ either. So the stylesheet is now composed by one pure function, called from two places. The SvelteKit route calls it per request; scripts/generate-markdown-css.ts calls it to write the copy Go embeds. Both routes now serve byte-identical CSS. Drift is no longer possible to merge: editing the reset or an Io theme colour without regenerating fails generate-markdown-css.test.ts, and CI runs both that and the Go tests.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Stacked on #3882 — review that one first.
The problem
/renderhas two implementations. A packagedui-serverserves it from Go (server/server/route/ui.go); the SvelteKit route atsrc/routes/(app)/renderonly runs where there is a SvelteKit server. Both need the same stylesheet, and the Go one has been carrying a copy pasted in when the route was added in #2430.None of the differences were chosen. Every change to
src/markdown.reset.csssince #2796 landed on one side only:fix(a11y): rem font-size, unitless line-heightfix(markdown): nested lists render as blockSo a packaged server is missing two shipped bug fixes (#3430, #3463), has no table styles, and renders markdown in the 2024 Holocene palette because #3849 never reached it.
Why it could not just be hand-synced
Two hard constraints:
ioThemeToCssVariables), which Go cannot import.server/is a separate Go module, so//go:embedcannot reachsrc/.The change
One pure function composes the stylesheet, called from two places:
src/lib/utilities/markdown-stylesheet.ts—composeMarkdownStylesheet(reset), extracted from the routescripts/generate-markdown-css.tscalls it to writeserver/server/route/markdown.gen.css, which Go embedsui.goloses 251 lines of duplicated CSS.Drift is now unmergeable
Editing the reset or an Io colour without running
pnpm generate:markdown-cssfailsscripts/generate-markdown-css.test.ts. CI runs that (lint-and-test.yml) and the Go tests (test.yml→make test).Verification
pnpm test: 251 files, 3209 passed, 2 skippedpnpm check: 885 files, 0 errorsgo test ./server/route/: pass,gofmtandgo vetcleanDeliberately left out
fs.readFileSyncat request time. A build-time?rawimport would be nicer, but vitest stubs.cssimports by default and working around that means changing the shared vitest config — not worth it in this PR.src/markdown.reset.cssstays the hand-edited source.