docs(plans): loading skeletons — 2-task plan (#53)
This commit is contained in:
@@ -0,0 +1,247 @@
|
||||
# Standardize Loading States on Skeleton — Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Replace the "…" text and empty `role="status"` divs with shared `Skeleton`-based loading recipes that mirror the loaded layout and announce loading to screen readers.
|
||||
|
||||
**Architecture:** A new `ui/skeletons.tsx` exports `ListSkeleton`, `FormSkeleton`, `AppShellSkeleton` (each a `role="status" aria-label={t("common.loading")}` live region built on the existing `Skeleton`). Apply them at every inconsistent loading site; retrofit the two good list-like skeletons to `ListSkeleton`.
|
||||
|
||||
**Tech Stack:** React 19 + TS + pnpm, react-i18next, Vitest + RTL + Storybook. Test runner: `pnpm test` (single pass).
|
||||
|
||||
**Conventions:** pnpm; **no `any`/`eslint-disable`/`@ts-ignore`**; no codename; en/sv parity (one new key); **ui/ files = no-semicolon** (match `ui/skeleton.tsx`); app source = double-quote+semicolon; stories single-quote/no-semicolon; token classes only; never nest `<div>` inside `<ul>`.
|
||||
|
||||
**Spec:** `docs/superpowers/specs/2026-06-08-loading-skeletons-design.md`
|
||||
|
||||
**Key facts:**
|
||||
- `ui/skeleton.tsx`: `function Skeleton({ className, ...props }: React.ComponentProps<"div">) { return <div data-slot="skeleton" className={cn("animate-pulse rounded-md bg-muted", className)} {...props} /> }` (no-semicolon).
|
||||
- "…" sites render `<li>…</li>` inside a `<ul>` — `vocabulary-list.tsx` (`<ul className="flex-1 overflow-auto">`, loading `<li className="p-3 text-sm text-muted-foreground">…</li>`), `vocabulary-terms.tsx` (`<ul className="mb-4">`), `authorities-page.tsx` (`<ul className="mb-4">`).
|
||||
- empty status divs: `require-auth.tsx` `if (isLoading) return <div role="status" aria-label="loading" />;` (pre-shell); `object-edit-form.tsx` `if (isLoading) return <div className="p-4" role="status" aria-label="loading" />;`.
|
||||
- `app.tsx`: `function FormFallback() { return <div role="status" className="p-4 text-sm text-muted-foreground">Loading…</div> }` used in 3 Suspense fallbacks (ObjectNewPage, ObjectEditForm, FieldsPage).
|
||||
- retrofits: `field-list.tsx` (`space-y-2 p-3` + 6 × `<Skeleton className="h-9 w-full" />`), `search-panel.tsx` (`space-y-2 p-3` + 5 × `<Skeleton className="h-12 w-full" />`).
|
||||
- i18n `common` namespace exists in both locales: `{ "yes", "no", "close" }` — add `"loading"`.
|
||||
|
||||
---
|
||||
|
||||
# Task 1: Shared skeleton recipes + i18n + story
|
||||
|
||||
**Files:** `web/src/components/ui/skeletons.tsx` (new), `web/src/components/ui/skeletons.stories.tsx` (new), `web/src/i18n/en.json`, `web/src/i18n/sv.json`.
|
||||
|
||||
- [ ] **Step 1: i18n** — add `"loading"` to the `common` namespace in BOTH locales (keep parity):
|
||||
- en: `"common": { "yes": "Yes", "no": "No", "close": "Close", "loading": "Loading" },`
|
||||
- sv: `"common": { "yes": "Ja", "no": "Nej", "close": "Stäng", "loading": "Laddar" },`
|
||||
|
||||
- [ ] **Step 2: Implement `web/src/components/ui/skeletons.tsx`** (no-semicolon, ui/* style):
|
||||
```tsx
|
||||
import { useTranslation } from "react-i18next"
|
||||
|
||||
import { cn } from "@/lib/utils"
|
||||
import { Skeleton } from "@/components/ui/skeleton"
|
||||
|
||||
function ListSkeleton({
|
||||
rows = 6,
|
||||
rowClassName = "h-9 w-full",
|
||||
className,
|
||||
}: {
|
||||
rows?: number
|
||||
rowClassName?: string
|
||||
className?: string
|
||||
}) {
|
||||
const { t } = useTranslation()
|
||||
return (
|
||||
<div role="status" aria-busy="true" aria-label={t("common.loading")} className={cn("space-y-2 p-3", className)}>
|
||||
{Array.from({ length: rows }).map((_, i) => (
|
||||
<Skeleton key={i} className={rowClassName} />
|
||||
))}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
function FormSkeleton({ fields = 5, className }: { fields?: number; className?: string }) {
|
||||
const { t } = useTranslation()
|
||||
return (
|
||||
<div role="status" aria-busy="true" aria-label={t("common.loading")} className={cn("space-y-4 p-4", className)}>
|
||||
{Array.from({ length: fields }).map((_, i) => (
|
||||
<div key={i} className="space-y-1">
|
||||
<Skeleton className="h-3 w-24" />
|
||||
<Skeleton className="h-8 w-full" />
|
||||
</div>
|
||||
))}
|
||||
<Skeleton className="h-8 w-28" />
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
function AppShellSkeleton() {
|
||||
const { t } = useTranslation()
|
||||
return (
|
||||
<div role="status" aria-busy="true" aria-label={t("common.loading")} className="flex min-h-screen">
|
||||
<aside className="w-44 space-y-2 border-r p-3">
|
||||
{Array.from({ length: 5 }).map((_, i) => (
|
||||
<Skeleton key={i} className="h-8 w-full" />
|
||||
))}
|
||||
</aside>
|
||||
<div className="flex flex-1 flex-col">
|
||||
<div className="flex items-center border-b px-4 py-2">
|
||||
<Skeleton className="h-6 w-40" />
|
||||
</div>
|
||||
<div className="flex-1 space-y-2 p-3">
|
||||
{Array.from({ length: 6 }).map((_, i) => (
|
||||
<Skeleton key={i} className="h-9 w-full" />
|
||||
))}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
export { ListSkeleton, FormSkeleton, AppShellSkeleton }
|
||||
```
|
||||
(`AppShellSkeleton` inlines its content rows rather than nesting `ListSkeleton`, so there's ONE `role="status"` for the whole boot screen. Token classes only.)
|
||||
|
||||
- [ ] **Step 3: Story `web/src/components/ui/skeletons.stories.tsx`** (single-quote, no-semicolon; match an existing story, e.g. `menu.stories.tsx`/`page-title.stories.tsx`). Export three stories (`List`, `Form`, `AppShellLoading`) each rendering the respective recipe; one `play` test (on `List`) asserting a `role="status"` region is present:
|
||||
```tsx
|
||||
import type { Meta, StoryObj } from '@storybook/react-vite'
|
||||
import { expect } from 'storybook/test'
|
||||
|
||||
import { AppShellSkeleton, FormSkeleton, ListSkeleton } from './skeletons'
|
||||
|
||||
const meta = { title: 'ui/Skeletons', tags: ['ai-generated'] } satisfies Meta
|
||||
|
||||
export default meta
|
||||
type Story = StoryObj
|
||||
|
||||
export const List: Story = {
|
||||
render: () => <ListSkeleton rows={4} />,
|
||||
play: async ({ canvas }) => {
|
||||
await expect(canvas.getByRole('status')).toBeInTheDocument()
|
||||
},
|
||||
}
|
||||
|
||||
export const Form: Story = { render: () => <FormSkeleton /> }
|
||||
|
||||
export const AppShellLoading: Story = { render: () => <AppShellSkeleton /> }
|
||||
```
|
||||
(Adjust the `Meta`/`StoryObj` typing to the house pattern if `satisfies Meta` without a component arg complains — these are render-only stories; mirror how an existing component-less story is typed, or pass `component: ListSkeleton`.)
|
||||
|
||||
- [ ] **Step 4: Verify (vitest ONCE):**
|
||||
`cd web && pnpm vitest run src/components/ui/skeletons.stories.tsx && pnpm typecheck && pnpm lint`
|
||||
Expected: PASS, clean. (If a storybook cache flake appears — `Cannot read properties of null (reading 'useEffect')` — `rm -rf node_modules/.cache/storybook node_modules/.vite` and re-run ONCE.)
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
```bash
|
||||
git add web/src/components/ui/skeletons.tsx web/src/components/ui/skeletons.stories.tsx web/src/i18n/en.json web/src/i18n/sv.json
|
||||
git commit -m "feat(web): shared loading skeleton recipes (List/Form/AppShell) + common.loading (#53)"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
# Task 2: Apply skeletons across all loading sites + gate
|
||||
|
||||
**Files (modify):** `web/src/vocab/vocabulary-list.tsx`, `web/src/vocab/vocabulary-terms.tsx`, `web/src/authorities/authorities-page.tsx`, `web/src/objects/object-edit-form.tsx`, `web/src/auth/require-auth.tsx`, `web/src/app.tsx`, `web/src/fields/field-list.tsx`, `web/src/search/search-panel.tsx`.
|
||||
|
||||
Add `import { ListSkeleton } from "@/components/ui/skeletons";` (and `FormSkeleton`/`AppShellSkeleton` where used) to each. NEVER nest a `<div>` (the recipe) inside a `<ul>` — render the skeleton in place of the `<ul>`.
|
||||
|
||||
- [ ] **Step 1: vocabulary-list.tsx.** Read the loading region. The list is `<ul className="flex-1 overflow-auto">` with a loading `<li>…</li>`. Render the skeleton in place of the `<ul>` during load:
|
||||
```tsx
|
||||
{isLoading ? (
|
||||
<ListSkeleton className="flex-1 overflow-auto" />
|
||||
) : (
|
||||
<ul className="flex-1 overflow-auto">
|
||||
{isError && (<li className="p-3 text-sm text-destructive">{t("vocab.loadError")}</li>)}
|
||||
{data?.length === 0 && (/* keep existing empty state */)}
|
||||
{data?.map(/* keep existing rows */)}
|
||||
</ul>
|
||||
)}
|
||||
```
|
||||
Keep the `isError`/empty/rows branches exactly as they are now (just move them under the `!isLoading` `<ul>`; remove the loading `<li>`). Preserve the `flex-1 overflow-auto` layout via the skeleton's `className`.
|
||||
|
||||
- [ ] **Step 2: vocabulary-terms.tsx.** The `<ul className="mb-4">` has a loading `<li>`. Same approach:
|
||||
```tsx
|
||||
{isLoading ? (
|
||||
<ListSkeleton className="mb-4" rows={5} />
|
||||
) : (
|
||||
<ul className="mb-4">
|
||||
{isError && (/* keep */)}
|
||||
{terms?.length === 0 && (/* keep */)}
|
||||
{terms?.map(/* keep TermRow */)}
|
||||
</ul>
|
||||
)}
|
||||
```
|
||||
|
||||
- [ ] **Step 3: authorities-page.tsx.** The `<ul className="mb-4">` has a loading `<li>`. Same:
|
||||
```tsx
|
||||
{isLoading ? (
|
||||
<ListSkeleton className="mb-4" rows={5} />
|
||||
) : (
|
||||
<ul className="mb-4">
|
||||
{isError && (/* keep */)}
|
||||
{authorities?.length === 0 && (/* keep */)}
|
||||
{authorities?.map(/* keep AuthorityRow */)}
|
||||
</ul>
|
||||
)}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: object-edit-form.tsx.** Replace the outer loading branch:
|
||||
`if (isLoading) return <div className="p-4" role="status" aria-label="loading" />;`
|
||||
→
|
||||
`if (isLoading) return <FormSkeleton />;`
|
||||
(Import `FormSkeleton`. The not-found branch stays unchanged.)
|
||||
|
||||
- [ ] **Step 5: require-auth.tsx.** Replace:
|
||||
`if (isLoading) return <div role="status" aria-label="loading" />;`
|
||||
→
|
||||
`if (isLoading) return <AppShellSkeleton />;`
|
||||
(Import `AppShellSkeleton`. It uses only `useTranslation` — safe pre-shell.)
|
||||
|
||||
- [ ] **Step 6: app.tsx lazy fallbacks.** Remove the `FormFallback` function. Import `FormSkeleton` and `ListSkeleton` from `@/components/ui/skeletons`. Replace the three Suspense fallbacks:
|
||||
- ObjectNewPage: `fallback={<div className="mx-auto max-w-2xl"><FormSkeleton /></div>}`
|
||||
- ObjectEditForm: `fallback={<div className="mx-auto max-w-2xl"><FormSkeleton /></div>}`
|
||||
- FieldsPage: `fallback={<ListSkeleton />}`
|
||||
Keep the `<Suspense>` wrappers + lazy imports; only the `fallback` prop changes (and `FormFallback` is deleted).
|
||||
|
||||
- [ ] **Step 7: field-list.tsx (retrofit).** Replace the inline `isLoading` block:
|
||||
`return (<div className="space-y-2 p-3">{Array.from({length:6}).map(... <Skeleton h-9 w-full/>)}</div>)`
|
||||
→ `return <ListSkeleton rows={6} />;`
|
||||
Remove the now-unused `Skeleton` import if nothing else uses it (check).
|
||||
|
||||
- [ ] **Step 8: search-panel.tsx (retrofit).** Replace the `hasQuery && search.isLoading` block's inline skeleton:
|
||||
`<div className="space-y-2 p-3">{Array.from({length:5}).map(... <Skeleton h-12 w-full/>)}</div>`
|
||||
→ `<ListSkeleton rows={5} rowClassName="h-12 w-full" />`
|
||||
Remove the now-unused `Skeleton` import if nothing else uses it (check).
|
||||
|
||||
- [ ] **Step 9: FULL GATE (run tests EXACTLY ONCE):**
|
||||
```bash
|
||||
cd web && pnpm typecheck && pnpm lint && pnpm test && pnpm build && pnpm check:size && pnpm check:colors
|
||||
```
|
||||
All green. The existing suite must stay green (no test asserts the old "…"/empty-div markup; tests `findBy` content). If a test fails because it queried `getByRole("status")` and now finds a labelled region (or multiple), update it minimally without weakening. Report test totals, largest chunk (KB gz), check:colors line.
|
||||
|
||||
- [ ] **Step 10: Codename + status:**
|
||||
```bash
|
||||
cd /Users/olsson/Laboratory/biggus-dickus
|
||||
git grep -in 'biggus\|dickus' -- web/src; echo "codename-exit=$?"
|
||||
git status --short
|
||||
```
|
||||
|
||||
- [ ] **Step 11: Manual smoke (recommended).** `pnpm dev`: first load shows the app-shell skeleton (no blank flash); navigating to /vocabularies, /authorities, /search, /fields shows list skeletons (no "…"); opening /objects/:id/edit and /objects/new shows a form skeleton (no full-pane "Loading…"); all transition into content without an obvious jump.
|
||||
|
||||
- [ ] **Step 12: Commit**
|
||||
```bash
|
||||
git add web/src/vocab/vocabulary-list.tsx web/src/vocab/vocabulary-terms.tsx web/src/authorities/authorities-page.tsx web/src/objects/object-edit-form.tsx web/src/auth/require-auth.tsx web/src/app.tsx web/src/fields/field-list.tsx web/src/search/search-panel.tsx
|
||||
git commit -m "feat(web): standardize loading on shared skeleton recipes; retire '…' + empty status divs (#53)"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Self-Review (completed)
|
||||
|
||||
**Spec coverage:** recipes List/Form/AppShell as `role="status"` live regions + `common.loading` + story (T1); three "…" → ListSkeleton, object-edit-form → FormSkeleton, require-auth → AppShellSkeleton, per-route lazy fallbacks replacing FormFallback (T2 S1–S6); field-list + search-panel retrofit (T2 S7–S8); gate (T2 S9). Acceptance criteria 1–5 mapped. ✓
|
||||
|
||||
**Placeholder scan:** the per-site replacements show the exact before→after and say "keep the existing error/empty/row branches" with the files/lines named — concrete, not vague. The story typing has an explicit fallback note. No TODOs. ✓
|
||||
|
||||
**Type/consistency:** `ListSkeleton({rows,rowClassName,className})`, `FormSkeleton({fields,className})`, `AppShellSkeleton()` defined in T1, consumed with matching props in T2; import path `@/components/ui/skeletons` uniform. ✓
|
||||
|
||||
## Notes
|
||||
- No new dependency; one new i18n key (`common.loading`, en+sv).
|
||||
- HTML validity: list skeletons replace the `<ul>` during load (never `<div>`-in-`<ul>`).
|
||||
- `check:size` ≈ unchanged (small components) — report it.
|
||||
- Retrofitting field-list/search-panel may leave an unused `Skeleton` import — remove it (lint will catch).
|
||||
Reference in New Issue
Block a user