fix(layout): drive split-pane orientation from CSS media queries so first paint is correct without JS - #203
Conversation
…irst paint is correct without JS The prerendered HTML shipped the stacked layout (useMediaQuery answers false without matchMedia), so desktop users saw the converter paint stacked and snap to columns when React mounted. Orientation now renders through CSS media queries — flex-col md:flex-row, an orientation-neutral --split custom property consumed by responsive h-/w-/top-/left- classes — so the prerendered HTML is correct at first paint in both orientations. The layout prop survives only for drag math and aria-orientation.
The EditorView mounts from a passive effect after the output-view host div commits, so a test that resolves on the host and queries .cm-editor immediately can lose that race on slow CI schedulers (2x CI failures on outputEditing while the suite passes locally). populate() now waits for the editor before any editor lookup.
![obvious-autobuild-staging[bot]](https://avatars.githubusercontent.com/u/31830338?s=60&v=4){kind=small}
There was a problem hiding this comment.
Obvious Code Review
Verdict: COMMENT
- Blocker: 0
- High: 1
- Medium: 0
- Suggestion: 0
Verified at head fd94253 in an isolated worktree: lint, typecheck, 357/357 tests, and a byte-identical app/dist rebuild all pass. The changelog entry (id 7) follows the repo's bundling convention, and the prerendered HTML carries the new orientation-neutral geometry.
Summary of the open finding: the JS drag-math breakpoint (matchMedia("(min-width: 768px)")) and Tailwind's md: (@media (min-width: 48rem)) only agree while the browser's default font size is 16px — media-query rem resolves against the initial font size, so a user font preference shifts the CSS breakpoint but not the JS one. For other font preferences there is a viewport band where the painted layout and the drag-math/aria-orientation axis disagree. Details and the one-line fix in the inline comment.
|
|
||
| // Responsive orientation is CSS-first in SplitPane (flex-col md:flex-row); | ||
| // this query feeds only drag math and aria-orientation. Keep the 768px | ||
| // breakpoint in sync with the md: variants in SplitPane.tsx. |
There was a problem hiding this comment.
🟠 High (reliability): px-based JS breakpoint vs rem-based Tailwind md: — these only agree while the browser's default font size is 16px.
Tailwind compiles md: to @media (min-width: 48rem) (confirmed in the built CSS; no screen overrides in this repo). In media queries, rem resolves against the initial font size — the user's default-font preference — while this query is fixed at 768px. With a 20px default, md: fires at 960px but this fires at 768px: across the 768–960px band the CSS paints the stacked layout (horizontal seam, cursor-row-resize) while layout="side-by-side" drives pointermove with (clientX − rect.left) / rect.width — the drag stops tracking vertical cursor motion, and aria-orientation reports "vertical" against a horizontal seam. Smaller defaults (e.g. 14px → md: at 672px) produce the mirror band (672–768px): side-by-side paint, stacked drag math.
Before this PR, visible geometry and drag math both derived from the single layout value and could never disagree; the split ownership introduced the gap. This comment documents "keep the 768px breakpoint in sync with the md: variants", but px-sync is not the actual invariant — unit-sync is.
Suggestion:
Query Tailwind's breakpoint in the same units:
// 48rem — Tailwind's `md:`. As a media query this resolves against the same
// initial font size as the compiled CSS, so the two can never diverge.
const desktop = useMediaQuery("(min-width: 48rem)");Reword the comment accordingly ("48rem — Tailwind's md:"), and optionally hoist the query string into a shared constant next to the SplitPane breakpoint note so future breakpoint changes move both sides together.
There was a problem hiding this comment.
Fixed in 2f92a79: the query is now useMediaQuery("(min-width: 48rem)"), matching Tailwind's compiled @media (min-width: 48rem) unit-for-unit, so paint and drag math can no longer diverge under a non-16px default font. The comments in App.tsx and SplitPane.tsx now state the real invariant (unit-sync) instead of px-sync, and PaneShell's stale '768px JS breakpoint' wording — its reflow is pure CSS — was corrected too. Verified: lint, typecheck, 357/357 tests, dist rebuild, verify-seo.
…s md: Tailwind compiles md: to @media (min-width: 48rem), and in media queries rem resolves against the browser's initial font size — a fixed 768px JS query drifts from the CSS breakpoint whenever the default font differs from 16px, leaving a viewport band where painted layout and drag math/aria-orientation disagree. Querying 48rem keeps both on the same unit. Comments updated to state the real invariant (unit-sync, not px-sync).
There was a problem hiding this comment.
Obvious Code Review
Verdict: COMMENT
No findings — clean pass.
The High from the previous review (px-based 768px drag-math breakpoint vs rem-based Tailwind md:) is verified resolved at head 2f92a79: App.tsx now queries useMediaQuery("(min-width: 48rem)"), which as a media query resolves against the same initial font size as Tailwind's compiled CSS, so the paint-time orientation and the drag-math/aria-orientation axis can no longer diverge for any browser default font size. Comments in App.tsx, SplitPane.tsx, and PaneShell.tsx now document the unit invariant correctly.
Verified at this head in an isolated worktree: lint, typecheck, 357/357 tests, and a byte-identical app/dist rebuild all pass.
1 participant
Why
On desktop, csvjson.com painted a stacked converter on load and then snapped into columns once React mounted — the first-paint flash David reported. The prerendered HTML baked in the stacked layout because
useMediaQuery("(min-width: 768px)")answersfalsewhereverwindow.matchMediais unavailable, which is exactly the prerender environment:dist/index.htmlshipped 13×flex-col, zeroflex-row, and stacked inline geometry (height: 50%left pane,top: 50%seam,cursor-row-resize,aria-orientation="horizontal"). A real desktop browser then mounts withcreateRoot,matchMediaanswers correctly, and the whole layout snaps — that snap is the flash. Mobile never saw it, so any JS-side fix that just corrects the prerender guess would move the flash to another viewport, not remove it.What
Orientation is now a CSS decision, not a JS decision. SplitPane renders the same orientation-neutral markup for both
layoutvalues, and media queries pick the geometry at paint time — correct in both orientations, with zero JS dependency, so the prerendered HTML matches React's first paint in every viewport.flex-col md:flex-row(thestacked ? "flex-col" : "flex-row"ternary is gone).height/widthstyle is replaced by an inline--splitcustom property plush-[var(--split)] md:h-auto md:w-[var(--split)]. Drag still updates--splitvia React; CSS consumes it.border-b md:border-b-0 md:border-r.--split+top-[var(--split)] … md:left-[var(--split)]replaces thetop:/left:ternary; zero-dimension axis and cursor (cursor-row-resize md:cursor-col-resize) are classes.touch-action: nonestays inline — it is orientation-neutral.layoutprop still feeds pointer drag math (clientYvsclientX) andaria-orientation, which are not first-paint visual concerns. The 768px coupling between themd:variants andApp.tsx'suseMediaQueryis documented in both files.Rejected alternative: stubbing
matchMediain the prerender script. That fixes desktop but flips the flash to mobile (columns → stacked on mount). CSS-first fixes every viewport and matches the rebuild spec's constraint that prerendered HTML matches first paint.How to Review
app/src/components/SplitPane.tsx— the whole fix. Geometry is expressed as longhand position/size classes (noinset-*shorthands mixed with longhands at the same variant), so every base property is either overridden or explicitly reset undermd:.app/src/components/SplitPane.test.tsx— six new tests pin the contract: identical direction classes for both layouts,--spliton panes and seam, no orientation-ternary inline geometry, drag updating the CSS var, cursor/aria still following the prop.Acceptance mapping
dist/index.htmlcontainsmd:flex-rowand--split(verified after build); zeroheight: 50%/top:inline pane or seam styles remain.dist/assets/*.css, not purged).tsc -b --noEmit,npm run buildwithapp/distrebuilt and committed,verify-seo.shgreen.🔗 Obvious Project · 🧵 Obvious Thread
Test Evidence
Fresh page load: master shows the stacked-to-columns flash, fix build paints side-by-side from the first frame
Fresh page load: master shows the stacked-to-columns flash, fix build paints side-by-side from the first frame — before
Fresh page load: master shows the stacked-to-columns flash, fix build paints side-by-side from the first frame — after