Make the editor usable with a finger, not just reachable by one - #139
Conversation
…lows The mobile scaffolding here is real — drawer, bottom sheet, an md: breakpoint, an overflow menu — so it is easy to read the layout as "done". Driving it on emulated iPhone 13 / Pixel 5 / iPhone SE / landscape / iPad Mini and measuring every visible control in the live DOM says otherwise: the layout reflows fine and no width overflows, but the controls inside it are still desktop controls at desktop density. The measurements are the point, so they are recorded rather than described. The primary editing control — a NumberInput row — is 18px tall. The four toolbar buttons that are a phone user's entire navigation are 32x24. Only the palette tiles clear 44px. Tree reparenting is HTML5 drag-and-drop, which does not fire from touch at all, so the core operation of a CSG modeler has no mobile path. And the first-run mobile screen is blank, because the "No model yet" copy lives inside a drawer hidden behind an icon. Screenshots are committed alongside the findings for the claims that are easier to see than to read: the blank empty state, the duplicated NODE TREE header, the property sheet opening at 33%, and iPad portrait landing on the desktop layout with a ~200px viewport and a wrapped Sign In button. Findings only — no behaviour changes. The recommendations are ordered by what a user hits first, and the last one is a mobile Playwright project, since there is no mobile e2e coverage today and every one of these would pass CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The mobile layout already reflowed correctly — drawers, a bottom sheet, a
breakpoint. What it did not do was change the controls inside that layout,
so a phone got desktop density: an 18px number input, 32x24 navigation
buttons, and 28px viewport tools. Measured in docs/mobile-audit.
Sizing is gated on `pointer: coarse`, not on width. What matters is whether a
fingertip is doing the pointing — a touchscreen laptop should get the bigger
targets and a narrow desktop window should not — and it leaves desktop
density untouched by construction. `.tap` is opt-in per site rather than a
blanket `button { min-height }`, because several of these buttons live in
fixed-height rows where growing the button breaks the row instead of the row
growing with it.
Three things were not size problems and needed real work:
Reparenting was impossible on a phone. The tree drags with HTML5 DnD, which
is never generated from touch input, so restructuring a tree could only be
done by deleting and rebuilding it. Move mode picks a node up and places it
with a second tap; it works the same with a mouse or a keyboard, and it lives
in its own store so a half-finished move cannot reach a project file or an
undo step.
A phone's first run was a blank screen. The "No model yet" copy existed, but
inside the tree drawer — visible on desktop, where the tree is always on
screen, and behind an icon everywhere else.
iPad portrait sat exactly on the desktop side of a 768px breakpoint: two
sidebars and a ~200px viewport, on a touch device. The layout switch moves to
1024, which is the first width where both sidebars and a usable viewport fit.
e2e/mobile.spec.ts is the part that keeps this from rotting. The sweep
asserting nothing on screen is under 44px is deliberately not a set of
per-button assertions — individual sizes get adjusted and those rot, whereas
a sweep fails on the next control someone adds at desktop density. It caught
a 42px input here: the row's border sits outside its content box, so an input
stretched to h-full lands two pixels short.
Closes #126, #127, #128, #129, #130, #131, #132, #133, #134, #135, #136
Refs #137
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kmatzen
left a comment
There was a problem hiding this comment.
Reviewed this against origin/main and it's solid overall — breakpoints (md:→lg:, isMobile() 768→1024) are consistent across Toolbar/NodeTreePanel/PropertyPanel/ChatDrawer/BottomSheet/MobilePanel, viewport-fit=cover is in index.html, pb-safe/pl-safe/pr-safe are defined, useKeyboardInset cleans up its listeners with correct offset math, moveNode guards against cycles, and the BottomSheet snap-point recompute on resize looks right.
One correctness bug I'd fix before merge:
The #129 iOS-zoom fix doesn't cover three of the controls it's meant to fix
src/index.css (the @media (pointer: coarse) block):
input, select, textarea,
input[type="text"], input[type="number"], input[type="password"] {
font-size: 16px;
}The comment above it says this has to "beat the utility classes on the inputs themselves." The enumerated input[type="..."] selectors do have the specificity for that (0,1,1 beats a text-sm/text-[12px] Tailwind class at 0,1,0). But the bare input, select, textarea selectors are only 0,0,1 and lose to any text-* class — Tailwind isn't in important mode here. So any control that (a) has no type attribute, or (b) is a <select>, keeps its Tailwind text size and still triggers iOS Safari's zoom-on-focus.
Three always-visible mobile controls hit this:
src/components/toolbar/Toolbar.tsx:151— project-name<input>has notype+text-sm→ stays 14px. Always on screen on mobile (notlg:hidden). This is audit §3's "Project name — 14px" item.src/components/chat/ChatDrawer.tsx:131— chat composer<input>has notype+text-sm→ stays 14px. Audit §3's "Chat input — 14px".src/components/toolbar/Toolbar.tsx:259— export-resolution<select>+text-[12px]→ stays 12px (only the bareselectselector can match a<select>at all).
Only NumberInput is actually rescued, since it carries an explicit type="text".
This also looks like it would fail the PR's own new e2e/mobile.spec.ts — "text entry does not zoom the page" scans every visible input/select/textarea for font-size < 16, and the project-name input is on screen throughout that test.
Smallest fix I can see: add type="text" to the two type-less <input>s so they hit the high-specificity rule, and give the base rule enough specificity to also catch <select> — e.g. input:not([type]), input[type="text"], input[type="number"], input[type="password"], select, textarea { font-size: 16px; } — or just drop the text-sm/text-[12px] classes from these three controls.
(Minor, possibly intentional: MobileEmptyState.tsx:32 is absolute inset-0 z-10 with no pointer-events-none, so while it's shown it sits over the ViewportToolbar buttons. Flagging for awareness only, since there's no model to act on at that point anyway.)
Generated by Claude Code
kmatzen
left a comment
There was a problem hiding this comment.
Correction to my earlier review comment: the CSS-specificity bug I reported for the project-name input, chat composer, and export <select> doesn't actually exist. I got the cascade wrong — sorry for the noise.
I analyzed the @media (pointer: coarse) { input, select, textarea, ... } rule as if it were competing with .text-sm under plain CSS specificity rules. It isn't. This project imports Tailwind v4 (@import "tailwindcss" in index.css), which expands to @layer theme, base, components, utilities; and puts every utility class — .text-sm included — inside @layer utilities. The coarse-pointer rule in index.css is not inside any @layer. Per the CSS cascade-layers spec, an unlayered rule always beats a layered one, regardless of specificity — so the existing rule already wins outright, and the specificity concern I raised doesn't apply.
I confirmed this two ways before retracting:
- Ran the PR's own
e2e/mobile.spec.ts"text entry does not zoom the page" test against the unmodified branch (no fix applied) — it passes. - Dumped
getComputedStyle(...).fontSizefor the project-name input directly in a real page load:16px, not the14pxmy specificity analysis predicted.
No action needed on this point — the existing rule is correct as written. The other notes in my first comment (breakpoint consistency, safe-area CSS, useKeyboardInset, etc. all checking out; the minor MobileEmptyState z-index observation) stand.
Generated by Claude Code
|
🚀 Preview deployed: https://mobile-touch-usability.sinter.pages.dev (updates on every push to this PR) |
Fixes the mobile audit findings. Stacked on #125 (the audit itself) — review that first, or read this diff against
worktree-mobile-usability-audit.Closes #126, #127, #128, #129, #130, #131, #132, #133, #134, #135, #136. Partially addresses #137.
The approach
Touch sizing is gated on
@media (pointer: coarse), not on a width breakpoint. What matters is whether a fingertip is doing the pointing — a touchscreen laptop should get bigger targets, a narrow desktop window should not — and it leaves desktop density untouched by construction..tap/.tap-h/.tap-ware opt-in per site rather than a blanketbutton { min-height }, because several of these buttons sit in fixed-height rows where growing the button breaks the row instead of the row growing with it.What changed
MobileEmptyStatein the viewport with the two ways in as buttons — instead of a black void.inputMode="decimal"+enterKeyHintonNumberInput(staystype="text"so10*2+5still parses).top-11to match the toolbar, anduseKeyboardInsetlifts it clear of the iOS keyboard.viewport-fit=cover, safe-area helpers,100dvh,overscroll-behavior, tap-highlight, text-size-adjust,user-select.NODE TREEheader removed; Escape closes both mobile containers.e2e/mobile.spec.ts+ amobilePlaywright project on iPhone 13.The test that matters
The sweep asserting nothing on screen is under 44px is deliberately not a set of per-button assertions — individual sizes get adjusted and those assertions rot, whereas a sweep fails on the next control someone adds at desktop density. It already earned its keep: it caught a 42px input, because the row's 1px border sits outside its content box so an input stretched to
h-fulllands two pixels short.Verification
npm test— 503 passednpx playwright test(serial, as CI runs it) — 81 passed acrosschromium,mobile,goldenNote: running the three projects with local parallel workers produces spurious failures — the config already documents WebGL oversubscription on multi-worker runs, and adding a third project makes it worse. Serial is clean.
Deliberately not done
🤖 Generated with Claude Code