From 666b4802a912fe4ca718ce87ec10b60df4db72c7 Mon Sep 17 00:00:00 2001 From: Oier Bravo Urtasun Date: Mon, 18 May 2026 14:06:36 +0200 Subject: [PATCH] =?UTF-8?q?docs(fase-16):=20history=20record=20=E2=80=94?= =?UTF-8?q?=20loading=20spinner?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fase 16.Z. Documents the component decisions, the five integration sites, the unit + e2e test surface (5 + 3 specs), the vitest browser- condition tweak, the Supabase storage-key derivation gotcha used by SP-E-03, and the cumulative MVP2 test count (497 → 505). Co-Authored-By: Claude Opus 4.7 --- docs/history/fase-16-spinner.md | 78 +++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 docs/history/fase-16-spinner.md diff --git a/docs/history/fase-16-spinner.md b/docs/history/fase-16-spinner.md new file mode 100644 index 0000000..b7de439 --- /dev/null +++ b/docs/history/fase-16-spinner.md @@ -0,0 +1,78 @@ +# Fase 16 — Loading spinner + +**MVP2 · 8/8.** Reopened after Fase 15 to cover the missing visual feedback during async operations. + +**Status: ✅ Shipped 2026-05-18.** + +A small UX gap had survived through every previous fase: every async-loading site rendered the plain text `Loading…` and nothing else. Operations under ~100 ms feel instant; anything longer just sat there with stale UI. This fase adds a reusable spinner component and integrates it at the five existing loading sites listed in the plan, with no new loading semantics. + +Plan: `plan/fase-16-spinner.md`. No DB migration, no new Paraglide string. + +## What changed + +### 16.1 — `Spinner.svelte` + +`apps/web/src/lib/components/Spinner.svelte`: + +- SVG circle with `stroke-dasharray="42 14"` driven by Tailwind's `animate-spin`. `motion-reduce:animate-none` honours `prefers-reduced-motion`. +- Three sizes (`sm`/`md`/`lg`) map to 16 / 24 / 40 px. The `px` value is `$derived` from `size` to keep the Svelte 5 reactivity correct without warnings. +- `class` prop is forwarded to the wrapper so callers can colour-shift it (the SVG uses `currentColor`, so it integrates with the Fase 9 theme tokens out of the box). +- `role="status" aria-live="polite"` wrapper around the SVG, plus an `sr-only` `m.loading()` label. No new Paraglide strings — the existing `loading` message is reused. +- `data-testid="spinner"` on the SVG itself so e2e and unit tests can address it without scraping classes. + +### 16.2 — Integration at five existing loading sites + +The plan listed exactly five sites that already had a visible loading indicator. Each one keeps the `m.loading()` text as a visual + a11y fallback and gains a spinner alongside; no new flag was introduced anywhere. + +- `apps/web/src/routes/(app)/+layout.svelte` — auth splash gets `size="lg"` centred above the loading label. +- `apps/web/src/routes/(app)/search/+page.svelte` — inline `size="sm"` next to the search-in-flight copy. +- `apps/web/src/routes/(app)/notes/archive/+page.svelte` — inline `size="sm"` next to the archive-load copy. +- `apps/web/src/lib/components/CreateCollectiveModal.svelte` — inline `size="sm"` inside the disabled Save button while the `creating` flag is true. +- `apps/web/src/routes/(app)/collective/manage/+page.svelte` — three sites: members-list load (`loading`), invitation-generate button (`generating`), and add-common-item Save button (`addSaving`). The Common Items list itself uses the existing `$commonItems` store which already short-circuits before the table is rendered, so it didn't need a dedicated spinner. + +### 16.3 — Tests + +#### Unit (`apps/web/src/lib/components/Spinner.test.ts`) + +Five specs covering the public surface of the component, using Svelte 5's `mount()` + `unmount()` directly (no `@testing-library/svelte` dependency — it isn't in the repo). + +- **SP-U-01** default size renders a 24x24 SVG. +- **SP-U-02** `size="sm"` → 16x16, `size="lg"` → 40x40. +- **SP-U-03** wrapper exposes `role="status"` + `aria-live="polite"`. +- **SP-U-04** sr-only label content equals `m.loading()` (compared against the Paraglide message, not the literal string). +- **SP-U-05** `class` prop is forwarded onto the wrapper element. + +`apps/web/vitest.config.ts` had to grow `resolve.conditions: ['browser']` + an inline of `svelte` so `mount()` resolves to `index-client.js`. Without it, vitest picked Svelte's server entry and `mount()` threw `lifecycle_function_unavailable`. The other seven unit suites still pass under the new config — no behavioural drift. + +#### E2E (`apps/web/tests/e2e/spinner.test.ts`) + +Three Playwright specs, all using `page.route` to add deterministic latency so the spinner assertion window is wide enough for the auto-wait. + +- **SP-E-01** `/search` — throttles the `search_in_collective` RPC by 500 ms, fills the input, asserts the spinner is visible inside the loading paragraph and then disappears once the RPC resolves. +- **SP-E-02** `/collective/manage` (Ana, admin) — throttles `set_item_frequency_weight`, opens the Add common item modal, types a unique name, clicks Save, and asserts the spinner is visible scoped inside the Save button. +- **SP-E-03** auth splash — the hardest one. The splash is gated on `$authLoading`, which only flips to false once `onAuthStateChange` fires `INITIAL_SESSION`. On loopback that is microseconds, too fast for Playwright. We seed an *expired* `sb-${hostname-first-label}-auth-token` (the storage key Supabase derives from `PUBLIC_SUPABASE_URL`'s hostname) so GoTrue's `_recoverAndRefresh` is forced to call `/auth/v1/token?grant_type=refresh_token` before emitting `INITIAL_SESSION`, then we hang that endpoint indefinitely with a Promise that's only resolved in the test's `finally` block. `authLoading` stays true for the assertion window. Both `sb-192-auth-token` (LAN-IP dev) and `sb-localhost-auth-token` (loopback fallback) are seeded so the test is resilient to a `PUBLIC_SUPABASE_URL` swap. + +## Test deltas (Fase 16 only) + +| Suite | Before | After | Δ | +|---|---:|---:|---:| +| Vitest unit (apps/web) | 47 | 52 | +5 | +| Playwright e2e | 93 | 96 | +3 | +| **MVP2 cumulative (all categories)** | 497 | 505 | +8 | + +(pgTAP, Vitest integration, gated rate-limit unchanged.) + +## Gotchas / things found while wiring it + +- **Svelte 5 `mount()` requires the browser export condition.** Vitest's default conditions resolve `svelte` to `index-server.js`, which doesn't export `mount`. Adding `resolve.conditions: ['browser']` + inlining `svelte` in `server.deps` fixes it without touching any other test file. +- **`state_referenced_locally` warning** on `const px = size === ...`. The component's props are conceptually fixed for a given mount, but Svelte 5's strict mode still flags reading a prop in non-reactive position. Switching to `$derived` removes the warning and keeps the value tracked if the parent ever wires `size` to a store. +- **Supabase storage key derivation.** `sb-${hostname.split('.')[0]}-auth-token` was the surprise — for `http://192.168.1.167:8001` the key is `sb-192-auth-token`, not the project ref or the hostname itself. Knowing this is necessary to seed a session from a Playwright `addInitScript` and force a refresh path. Documented inline in `tests/e2e/spinner.test.ts` so the next person doesn't have to re-derive it from the supabase-js source. +- **Throttle pattern in Playwright.** `page.route` + `await new Promise((r) => setTimeout(r, 500)); await route.continue()` is enough to keep Playwright's 30 ms auto-wait cadence from racing past a brief loading state. For SP-E-03 the variant is an indefinitely-hung route (Promise resolved only in the test's `finally`) because a fixed delay still let `INITIAL_SESSION` fire too quickly under network jitter. + +## Out of scope (re-affirmed) + +- Skeleton placeholders. Spinner only. +- Determinate progress bars. +- Toast or notification-style "loading" indicators. +- Full-screen overlays. +- Wiring up stores that don't already expose a loading flag (`listsLoading`, `tasksLoading`, etc.).