fix(web): remove the layout shifts on the account settings pages#526
Merged
Zach Dunn (zachdunn) merged 15 commits intoJul 25, 2026
Conversation
Adds the .ws-skel CSS primitive plus skeletonBarHtml, renderUsagePlaceholderHtml, renderDetailsPlaceholderHtml, and renderGalleriesPlaceholderHtml — pure HTML builders whose markup mirrors their real counterparts exactly, so the later swap to live data does not shift layout. Task 2 of 8 in the account settings layout-shift fix.
Review findings on Task 2's skeleton placeholders: - move renderDetailsPlaceholderHtml next to its real counterpart renderDetailsHtml in workspace-rail.ts, so they stay editable in view of each other as the module doc requires - give renderUsagePlaceholderHtml a meters param (default 2) instead of hard-coding two rows, since renderUsageHtml can render two meters, one meter, or a capless text line depending on the workspace's plan
The workspace rail's usage/details sections now ship real chrome (meter tracks, dt/dd labels) in the server HTML instead of a "Loading usage…" text node, then paint from the per-workspace snapshot cache synchronously before revalidating through the shared workspace-summary request.
- Don't stomp a Tier 1 warm-painted usage value with "Usage unavailable."
when Tier 2 succeeds but returns no usage (result.usage: null is a
reachable success shape, not a failure) — mirrors the failure branch's
existing warm-value guard.
- Correct the renderUsagePlaceholderHtml doc comment: the meter-to-text
collapse is bounded and uncommon after the snapshot cache is warm, not
impossible — it can recur on TTL expiry, a snapshot version bump, or
sign-out.
- Drop two dead removeAttribute("aria-busy") calls in the rail paint
helper; neither element ever carries that attribute.
- Document data-stale as a diagnostic marker (no styling/read consumer
exists) rather than leaving it unexplained.
Applies Task 5's galleries pattern to the remaining two gated workspace tabs: card chrome (heading, back link, plan-card/member-list shell) now renders on first paint instead of waiting behind #ws-app, with geometry- matched skeleton placeholders standing in until data lands. Both pages gate through the shared loadWorkspaceSummary request (de-duping with the rail's own call) before chaining their existing people/billing fetch.
The CLI command block was the loudest element on the galleries tab despite being instructions, while whether the workspace actually has galleries was the quietest thing on the page. Move the table above the commands, collapse the commands into a closed <details>, trim the explainer to one sentence, and drop `gallery add` (redundant with `put --gallery`). Add renderGalleriesEmptyHtml so the empty state leads with "no galleries yet" rather than a muted footnote under the command block.
Tier 2's failure branch bailed on `if (cached) return`, but a cached snapshot can have details without usage (toWorkspaceSnapshot stores usage: undefined when a prior response had usage: null). When that snapshot exists and revalidation then fails, the usage element was never touched — it stayed on the aria-busy Tier 0 skeleton forever. Gate each section's fallback on that section's own warm value instead of the snapshot's mere existence.
The manageable view's role <select> + Remove button render taller than the read-only view's plain role label, and the placeholder can only match one shape since permission isn't known at first paint. Give .member-row__role and .member-row__role-select the same font/padding/ border box as the existing .text-btn rule (Remove/Revoke buttons already use it), so read-only, manageable, and pending-invite rows all converge on one height instead of drifting with the viewer's role. <select> still renders 2px taller than <button> even with byte-identical CSS (a browser form-control quirk), so the select's height is pinned explicitly to .text-btn's own measured value, with the basis documented inline for future re-measurement.
`pnpm lint:fix` rewrote 13 files across four packages, and one of them stopped compiling. - prefer-set-has rewrites an array literal to `new Set(...)` and `.includes` to `.has`, but leaves the variable's type annotation alone. In console-mode.ts that produced `const MODES: readonly ConsoleMode[] = new Set([...])`, which does not typecheck. - no-array-sort and no-array-reverse fire on the `[...arr].sort(...)` idiom used throughout this repo. That spread already copies, so the rules' whole premise (an accidental in-place mutation) does not apply here, and their toSorted/toReversed rewrite just adds a second copy. oxlint's --fix/--fix-suggestions/--fix-dangerously are global, so a rule's diagnostic cannot be kept while suppressing only its fix; disabling is the only lever. The pre-commit hook is unaffected either way -- lint-staged runs oxlint without --fix, so it only ever reported these. Drops the now-redundant no-array-sort override for test files, and records the reasoning in AGENTS.md so the rules are not re-enabled without re-checking that lint:fix leaves a clean tree. Verified in a throwaway worktree: reproduced all 13 files and 18 changed lines, applied this config, and re-ran -- zero source files written. `pnpm lint:fix` now exits 0 instead of 1.
Every other Tier 0 region on these pages signals a loading state, but the rail's details list did not. Its skeleton bars are aria-hidden, so a screen reader met two empty <dd>s with no cue that anything was still coming. aria-busy goes on the <dl> itself rather than inside the placeholder, because innerHTML and textContent both replace this element's children without touching the element — so unlike the usage section, whose aria-busy lives on markup the swap destroys, this one has to be cleared by hand on both the paint path and the details-unavailable path.
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (2)
🚫 Excluded labels (none allowed) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
uploads-web | e87289b | Commit Preview URL Branch Preview URL |
Jul 25 2026, 12:27 AM |
The DS shipped .ul-input/.ul-field/.ul-label but nothing for <select>, so every select in the web app styled itself independently or not at all — three rendered with raw browser chrome inside the dark, mono-token UI. Adds .ul-select / .ul-select--sm to packages/ui/src/styles.css (appearance: none with a drawn-in caret, matching .ul-input's box) plus a React Select export for parity with Input/Field/Label. Applies it to all six selects: .member-row__role-select, .plan-select, .limit-unit, .invite-role, #ws-select, #workspace-select — deleting the bespoke CSS each superseded. appearance: none also turns out to remove the <select>-vs-<button> height mismatch a previous fix (e8a9a8f) worked around with a pinned height: 27.5px on .member-row__role-select. Verified directly: select, button, and label now all compute to the same 28px box with no pin, and all four People-row variants (read-only/manageable/pending/skeleton) still measure identically before and after.
.ul-select's caret bakes in --muted's literal hex because a data-URI SVG renders in its own document scope and cannot resolve custom properties. The rule already explains that; this puts the back-reference where someone changing the token will actually see it.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
uploads-auth | e87289b | Commit Preview URL Branch Preview URL |
Jul 25 2026, 12:27 AM |
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.
Navigating
/account/*produced a run of layout shifts: explicit "Loading…" text, then the whole page flickering in at once, then the right rail swapping "Loading usage…" for a taller meter block.The cause was not asset delivery — Astro already serves hashed JS/CSS with long cache headers. Every shift came from the data waterfall, and from static chrome being gated behind it.
What was happening
Every workspace tab hid its entire card behind
<div id="ws-app" hidden>until two network round trips finished:Shift A was self-inflicted: the heading, explainer and CLI command need no data at all. And RT2 was redundant —
initWorkspacesNavalready fetched and cached that same list, and the rail separately fetched the same workspace.Approach
A three-tier loading model, applied across the rail and all three gated tabs.
#ws-appgate is gone.localStoragesnapshot store warm-paints last-known values at module-eval time.--panel, sized to the exact geometry of what replaces them.Errors are now section-scoped rather than page-replacing. The one exception stays page-level: no access to the workspace, where the rest of the page is meaningless.
The membership check moved from a
.find()over the workspace list to the/summaryendpoint the rail already called, sharing one in-flight promise — so the page and the rail now issue one request where there were three.Result
Measured in the browser, placeholder vs. real content:
The three People variants and the placeholder were measured together and all
four agree exactly; the absolute figure is omitted because it was captured in a
different measurement context from the two rows above it, and reconciling the
two bases would not change the result that matters — the delta is zero.
No
id="ws-app"remains, and no literal "Loading…" / "Loading usage…" ships in the HTML for these routes.Galleries tab, restructured
The hierarchy was inverted: the CLI command was the loudest element on the page but it is instructions, while the actual state — that the workspace has no galleries — was the quietest thing there, muted, at the bottom, under a paragraph carrying three more commands.
Now the table region comes first, the explainer is one sentence instead of two, the commands sit in a closed disclosure, and the empty state leads with "No galleries yet".
uploads gallery addis dropped from the page —uploads put --galleryalready does that job, and listing both invited a choice that does not matter.A design-system gap this turned up
Fixing the People-tab shift meant styling a
<select>by hand, which raised a fair question: why hand-roll a control at all?Because the DS had no select primitive — not as a React component, not as a CSS class.
@uploads/uishipsField/Input/Labeland.ul-field/.ul-label/.ul-input, but nothing for<select>. So all six selects in the web app styled themselves independently, and three of them (.invite-roleon admin,#ws-selecton device login,#workspace-selecton OAuth consent) had no CSS at all — raw browser chrome inside a dark mono-token UI.This adds
.ul-select(plus a compact.ul-select--sm) beside.ul-input, and a matching ReactSelectexport for parity, then adopts it at all six call sites and deletes the superseded bespoke CSS.The primitive sets
appearance: noneand draws its own caret as a data-URI SVG, which is also what retires the magic number: with full control of the box, a<select>and a<button>compute to the same 28px, so the member row's height converges on its own Remove button with no pinned height. That was measured, not assumed — an A/B against the previous commit put all four People-row variants at an identical height before and after.One tradeoff worth naming: the caret's stroke is
--muted's literal hex rather than a livevar(), because a data-URI SVG renders in its own document scope and cannot resolve the host page's custom properties. Both the rule and the token definition carry a note about the coupling.Also worth a reviewer's attention
Member rows are taller for everyone, by roughly the difference between the old read-only and manageable variants. A row's height used to depend on whether you could manage members, because the manageable variant carries a select and a Remove button. No placeholder can be right for both when permission isn't known at first paint, so the row now reserves one height for all three variants.
One commit here is unrelated to the rest.
fix(lint): disable three unicorn rules whose autofixes corrupt the tree—pnpm lint:fixrewrote 13 files across four packages, andapps/web/src/lib/console-mode.tsstopped compiling:prefer-set-hasconverts an array literal tonew Set(...)without touching thereadonly ConsoleMode[]annotation.no-array-sort/no-array-reversefire on this repo's[...arr].sort(...)idiom, which already copies, so their rewrite just adds a second copy. oxlint's--fixflags are global, so a rule's diagnostic cannot be kept while suppressing only its fix. The pre-commit hook was never affected — lint-staged runsoxlintwithout--fix.Known limits, accepted deliberately
data-staleis set on warm-painted sections as a diagnostic marker; it has no visual treatment, deliberately.Testing
pnpm test3170/3170 ·pnpm typesexit 0 · all changed files oxfmt-clean · no changeset (@uploads/webis changeset-ignored).New unit coverage for the cache (TTL, version mismatch, malformed entries, quota), the request de-dupe (including that a rejection clears the in-flight entry so retries work), and every placeholder builder. Astro page scripts and DOM boot code stay uncovered — the suite runs in node with no jsdom — so those were verified in the browser instead.
pnpm checkexits 1 on.claude/settings.local.json, an untracked gitignored local file unrelated to this branch.