## Summary
Deflakes the `adjust user theme preference` Playwright test
(`site/e2e/tests/users/userSettings.spec.ts`), tracked in DEVEX-415.
The test selected the Light theme and then hard-navigated with
`page.goto`
before the optimistic appearance update was persisted. The navigation
could
cancel the in-flight `PUT /api/v2/users/me/appearance`, so the reloaded
document embedded the stale `dark` preference and the final assertion
flaked.
`toPass` retries could not help because retrying the reload only
re-reads the
still-stale persisted state.
## Fix
Wait for the appearance form's save spinner to clear before the hard
reload,
mirroring how other settings tests wait for a visible save confirmation.
Asserting the optimistic light class first guarantees the spinner is
already
showing if a save started; a repeat run that is already light never
shows it,
so the test is idempotent and there are no direct API calls.
To give the test a UI signal, `Spinner` gets an opt-in `label` prop that
exposes it as a `role="status"` live region with an `aria-label`
(decorative
otherwise). `Loader` moves its label onto its own status container so it
keeps
a single status region.
## Changes
- `site/src/components/Spinner/Spinner.tsx`: opt-in `label` prop.
- `site/src/components/Loader/Loader.tsx`: label on its own status
region.
- `site/src/pages/UserSettingsPage/AppearancePage/AppearanceForm.tsx`:
label
both appearance save spinners.
- `site/e2e/tests/users/userSettings.spec.ts`: wait for the save
spinner.
## Validation
- `biome check` and `tsc -p .` pass.
- Loader + AppearancePage unit tests and the affected storybook tests
pass.
- The e2e test passed 20/20 under
`pnpm playwright:test -g "adjust user theme preference" --repeat-each
20`.
<details>
<summary>Implementation plan & decision log</summary>
# DEVEX-415: Fix flake in "adjust user theme preference" e2e test
## Problem
Playwright test `site/e2e/tests/users/userSettings.spec.ts` → `adjust
user
theme preference` flakes. After selecting the Light theme and
hard-navigating
to `/`, the reloaded page sometimes stays `dark`, failing the final
assertion.
CI Flake Bot has recorded repeated recurrences on `main` (latest
2026-08-17,
runs `32004239187`, `31745261984`) even after PR #25183 added `toPass`
retries.
## Root cause (confirmed by reading the code)
The appearance update is optimistic and its persistence is not awaited
before
navigation:
- `updateAppearanceSettings` (`site/src/api/queries/users.ts`) has an
`onMutate` that optimistically writes the new theme into the React Query
cache. The `<html>` class flips to `light` immediately, before the
`PUT /api/v2/users/me/appearance` completes.
- `useQueuedAppearanceSubmit`
(`site/src/pages/UserSettingsPage/AppearancePage/AppearancePage.tsx`)
serializes submits: if a request is in flight, the next is queued and
only
fires after the first settles.
- A fresh `member` user has **empty** appearance settings.
`migrateLegacyPreference` (`site/src/theme/themeMode.ts`) maps empty
settings
to `{ mode: "single", theme: "dark" }` (`DEFAULT_THEME = "dark"`). So
the
"Theme mode" dropdown already starts on **Single theme** and `<html>`
starts
`dark`. Selecting "Single theme" in the test is therefore a **no-op**
(`onChangeMode` early-returns when `mode === draft.mode`) and fires
**no**
PUT. The only appearance PUT in the test is the one from clicking
"Light default" (`onSelectSingle("light")` → `theme_preference:
"light"`).
- The test's first `expectLightThemeClasses(page)` passes purely from
the
optimistic cache. It then calls `page.goto("/")` almost immediately
(~20 ms after the click). If PUT #2 (the light one) has not persisted
server-side, the new document loads the still-persisted `dark`
preference
from embedded metadata, and every retry of the post-navigation assertion
sees `dark` for the full 10 s window.
`toPass` cannot help post-navigation because it only re-reads stale,
already
persisted state; it cannot make an unfinished/queued PUT complete.
## Implemented fix: wait for the saving spinner (UI signal) before
navigation
> Iteration history: (1) An earlier attempt waited on the appearance
`PUT` via
> a `waitForApiCall` helper, but that couples the test to a state
transition
> and is non-idempotent (a repeat run that is already light fires no
PUT, so
> the wait times out). CI retries reuse the same ephemeral server, so
this is a
> real hazard. (2) A second attempt confirmed persistence with
> `page.request.get(...)`, but that calls the API directly and stops
being a
> site test. Both were rejected.
The repo's non-flaky settings tests wait for a **visible save
confirmation**
(e.g. "settings updated successfully" toasts) before trusting the
result. The
appearance theme form has no toast; its only save-in-progress feedback
is the
`<Spinner>`. So the fix mirrors that pattern using the spinner:
1. Make the theme section's spinner identifiable via a new opt-in
`label` prop
on `Spinner` (sets `role="status"` + `aria-label` only when provided;
decorative otherwise). `Loader` moves its label onto its own `status`
container so it keeps a single status region and its `getByLabelText`
queries keep working.
2. In the test, after clicking "Light default", assert the optimistic
light
class, then wait for that spinner to be hidden before `page.goto("/")`.
Why this is correct and idempotent:
- React Query sets `isPending` before `onMutate` applies the optimistic
cache
update, so by the time the optimistic light class is visible the spinner
is
already showing if a save started. Waiting for it to clear guarantees
the PUT
settled (and was not canceled by navigation) before the reload.
- A repeat run that is already light: clicking "Light default" is a
no-op radio
change, no PUT fires, the spinner never shows, and `toBeHidden` passes
immediately. The reload still shows light.
- No direct API calls: the test only observes site UI.
### Implemented changes
`Spinner` gains an opt-in `label` prop
(`site/src/components/Spinner/Spinner.tsx`):
```tsx
role={label ? "status" : undefined}
aria-label={label}
```
`Loader` carries the label on its own status container
(`site/src/components/Loader/Loader.tsx`), and the appearance save
spinners use
the new prop (`AppearanceForm.tsx`):
```tsx
<Spinner loading={isUpdating} size="sm" label="Saving theme preference" />
<Spinner loading={isUpdating} size="sm" label="Saving terminal font" />
```
`site/e2e/tests/users/userSettings.spec.ts`:
```ts
await expect(
page.getByRole("combobox", { name: /theme mode/i }),
).toContainText("Single theme"); // precondition: single mode
const singleThemeGroup = page.getByRole("group", { name: "Theme" });
await expect(singleThemeGroup).toBeVisible();
await singleThemeGroup.getByText("Light default", { exact: true }).click();
await expectLightThemeClasses(page); // optimistic DOM => spinner showing if saving
await expect(
page.getByRole("status", { name: "Saving theme preference" }),
).toBeHidden(); // save settled before the hard reload
await page.goto("/", { waitUntil: "domcontentloaded" });
await expectLightThemeClasses(page);
```
Validation: `biome check`, `tsc -p .` pass; Loader + AppearancePage unit
tests
and the affected storybook tests pass; the e2e test passed 20/20 under
`pnpm playwright:test -g "adjust user theme preference" --repeat-each
20`
(idempotent).
## Alternatives considered
- **Wait on the appearance `PUT` via a `waitForApiCall` helper**:
rejected. It
couples the test to a state transition and is non-idempotent, a repeat
run
that is already light fires no PUT so the wait times out (14/15 repeats
failed). CI retries reuse the same ephemeral server, so this is a real
hazard, not just a local-repeat artifact.
- **Confirm persistence with `page.request.get(...)`**: rejected. It
calls the
API directly and stops being a site test.
- **Default `role="status"` on the shared `Spinner`**: rejected.
`Loader` wraps
`Spinner` in its own `status` div, so a default would nest two status
regions and break `Loader.test.tsx`. The opt-in `label` prop avoids
this.
- **Add a networkidle wait or sleep before navigation**: rejected.
Violates
the repo guidance against `time.Sleep`-style timing hacks and is
inherently
racy.
- **Fix product behavior instead of the test**: out of scope. Related
bug
DEVEX-94 ("Light Theme setting not respected until Appearance page
opened")
tracks product-side persistence/embedding behavior; this task is scoped
to
stabilizing the e2e test.
## Files touched
- `site/src/components/Spinner/Spinner.tsx` (new opt-in `label` prop).
- `site/src/components/Loader/Loader.tsx` (label on its own status
region).
- `site/src/pages/UserSettingsPage/AppearancePage/AppearanceForm.tsx`
(use the
`label` prop on both appearance save spinners).
- `site/e2e/tests/users/userSettings.spec.ts` (wait for the spinner).
</details>
---
This PR was created by Coder Agents on behalf of @jeremyruppel.
---------
Co-authored-by: Samuel Volin <sam.volin@coder.com>
Abstracts the Spinner component to control the display of the CircularProgress component. This allows us to make it static during Chromatic tests, making loading tests easier to visualize.
This refactoring re-organizes the `site` folder to have a nested `src` folder.
Originally, [we wanted to keep the directory structure shallow](https://github.com/coder/coder/pull/8#issuecomment-1009578910) - but there were two points that motivated this change to introduce the `src` level.
1. We have several non-`src` folders now (`e2e`, `static`, `html_templates`, `.storybook`)
2. Having a `src` folder makes it easier to run XState Typegen
So given those two data points - I believe it makes sense to revisit that and introduce a `src` folder.
