docs(phase-11): research polish & responsiveness phase
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,565 @@
|
|||||||
|
# Phase 11: Polish & Responsiveness - Research
|
||||||
|
|
||||||
|
**Researched:** 2026-04-01
|
||||||
|
**Domain:** Tailwind v4 responsive utilities, CSS animations, focus-visible, scroll-into-view
|
||||||
|
**Confidence:** HIGH
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
Phase 11 is a pure CSS/layout polish phase — four independent requirements, zero new runtime dependencies. Every change is either adding Tailwind responsive prefixes (`sm:`, absence of prefix = mobile-first), adding `focus-visible:` utility classes, adding a CSS keyframe animation in `index.css`, or calling `element.scrollIntoView()` from a `react-hook-form` submit handler.
|
||||||
|
|
||||||
|
The technical risk is very low. Tailwind v4 retains the same responsive prefix syntax from v3 (`sm:`, `md:`, `lg:`). The `focus-visible:` variant is natively supported in Tailwind v4. CSS `@keyframes` and `@media (prefers-reduced-motion)` are standard CSS features, not Tailwind-specific. The auto-scroll requirement (`scrollIntoView`) uses a browser-native DOM API that is supported in all modern browsers and works correctly in jsdom for tests.
|
||||||
|
|
||||||
|
The only structural decision with wider impact is step transitions (POLISH-03): adding a CSS animation class to the step content wrapper in `App.tsx` (`WizardShell`). Since the step container is a single div that re-renders its child on step change, a CSS animation triggered by a React key or className update is the correct approach. The `prefers-reduced-motion` guard belongs in `index.css` via `@media (prefers-reduced-motion: reduce)` — consistent with the existing theme transition guard already in the file.
|
||||||
|
|
||||||
|
**Primary recommendation:** Implement as four independent tasks in this order: (1) POLISH-01 mobile responsive layout, (2) POLISH-02 focus-visible indicators, (3) POLISH-03 step transition animation, (4) POLISH-04 auto-scroll to first error. Each task touches different files with zero cross-task dependencies.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
<phase_requirements>
|
||||||
|
## Phase Requirements
|
||||||
|
|
||||||
|
| ID | Description | Research Support |
|
||||||
|
|----|-------------|-----------------|
|
||||||
|
| POLISH-01 | Wizard layout adapts to mobile screens — backend cards stack, form fields go full-width, step indicator collapses, buttons stretch | Tailwind v4 mobile-first responsive prefixes; `BackendCard` grid needs `flex-col` default with `sm:grid-cols-2` or keep `flex-col` stacked; `StepIndicator` label text visibility toggle; buttons need `w-full sm:w-auto` |
|
||||||
|
| POLISH-02 | All interactive elements have visible MD3 focus indicators (3px outline) using `focus-visible` for keyboard navigation | `focus-visible:outline` + `focus-visible:outline-3` or `focus-visible:ring-3`; Tailwind v4 supports arbitrary ring widths; apply to buttons, BackendCard, select, tooltip buttons, ThemeToggle, StepIndicator back-nav buttons |
|
||||||
|
| POLISH-03 | Step transitions use subtle fade/slide animation that respects `prefers-reduced-motion` | CSS `@keyframes step-in` in `index.css`; apply via Tailwind `animate-step-in` custom utility or inline className; `@media (prefers-reduced-motion: reduce)` guard in CSS |
|
||||||
|
| POLISH-04 | On validation failure, the view auto-scrolls to the first errored field | `react-hook-form` `useForm` returns `formState.errors`; in `handleSubmit` error callback (second argument), find first error field's DOM element with `document.getElementById(fieldKey)` and call `.scrollIntoView({ behavior: 'smooth', block: 'center' })` |
|
||||||
|
</phase_requirements>
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Standard Stack
|
||||||
|
|
||||||
|
### Core
|
||||||
|
| Library | Version | Purpose | Why Standard |
|
||||||
|
|---------|---------|---------|--------------|
|
||||||
|
| Tailwind v4 | 4.2.2 | Responsive prefixes, focus-visible variant, custom animations | Already in project; v4 syntax is mobile-first identical to v3 |
|
||||||
|
| React | 18.3.1 | Component key trick for animation reset | Already in project |
|
||||||
|
| Browser DOM API | native | `scrollIntoView()` for POLISH-04 | No dependency needed |
|
||||||
|
|
||||||
|
### Supporting
|
||||||
|
| Library | Version | Purpose | When to Use |
|
||||||
|
|---------|---------|---------|-------------|
|
||||||
|
| react-hook-form | 7.72.0 | Error callback for auto-scroll | Already used in `BackendSelectionStep` and `RemoteConfigStep`; second argument to `handleSubmit` is the error handler |
|
||||||
|
| Vitest + @testing-library/react | 4.1.1 / 16.3.2 | Unit tests for all behavioral changes | Existing test infrastructure |
|
||||||
|
|
||||||
|
**No new dependencies.** Zero new runtime packages for this phase.
|
||||||
|
|
||||||
|
**Installation:** None required.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Architecture Patterns
|
||||||
|
|
||||||
|
### Recommended Project Structure
|
||||||
|
```
|
||||||
|
src/
|
||||||
|
├── index.css # Add @keyframes step-in + @media prefers-reduced-motion guard
|
||||||
|
├── App.tsx # Add animation class to step content wrapper (POLISH-03)
|
||||||
|
├── components/
|
||||||
|
│ ├── wizard/
|
||||||
|
│ │ ├── BackendSelectionStep.tsx # Responsive button row (POLISH-01), focus-visible on cards (POLISH-02), auto-scroll (POLISH-04)
|
||||||
|
│ │ ├── RemoteConfigStep.tsx # Auto-scroll to first error (POLISH-04), responsive button row (POLISH-01)
|
||||||
|
│ │ ├── DeploymentStep.tsx # Responsive button row (POLISH-01)
|
||||||
|
│ │ ├── ReviewStep.tsx # Responsive button row (POLISH-01)
|
||||||
|
│ │ └── StepIndicator.tsx # Collapsed labels on mobile (POLISH-01), MD3 focus ring update (POLISH-02)
|
||||||
|
│ └── ui/
|
||||||
|
│ ├── BackendCard.tsx # Full-width stacking on mobile (POLISH-01), focus-visible ring (POLISH-02)
|
||||||
|
│ ├── TextFieldMD3.tsx # Already full-width (w-full); verify — no change needed
|
||||||
|
│ └── ThemeToggle.tsx # focus-visible ring (POLISH-02)
|
||||||
|
└── styles/
|
||||||
|
└── md3-buttons.ts # Update focus-visible from ring-2 to ring-3 / outline-3 (POLISH-02)
|
||||||
|
```
|
||||||
|
|
||||||
|
### Pattern 1: Tailwind v4 Mobile-First Responsive Layout (POLISH-01)
|
||||||
|
|
||||||
|
**What:** Tailwind v4 uses mobile-first breakpoints. A class without a prefix applies at all sizes; a `sm:` prefix applies at ≥640px. To achieve mobile-first stacking: set the mobile layout as the base class, add `sm:` variant for wider screens.
|
||||||
|
|
||||||
|
**Backend cards — current layout:** `BackendCard` components are rendered in a `<div data-testid="backend-cards">` with no grid/flex wrapper. Each `BackendCard` itself is `flex flex-col items-start`. They stack vertically by default because they are block-level elements. To make them fill the card container width on mobile and appear in a grid on larger screens:
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
// BackendSelectionStep.tsx — wrap backend cards in a responsive grid
|
||||||
|
<div data-testid="backend-cards" className="grid grid-cols-1 sm:grid-cols-2 gap-3">
|
||||||
|
{/* BackendCard buttons already have w-full implied by grid cell */}
|
||||||
|
</div>
|
||||||
|
```
|
||||||
|
|
||||||
|
**BackendCard — ensure full width in grid cell:**
|
||||||
|
```tsx
|
||||||
|
// BackendCard.tsx — add w-full so button fills grid cell
|
||||||
|
className={[
|
||||||
|
'w-full flex flex-col items-start gap-1 rounded-xl border-2 p-4 text-left transition-all',
|
||||||
|
// ... rest of classes
|
||||||
|
].join(' ')}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Button rows — current layout:** All step components have `<div className="flex gap-3 mt-6">`. On mobile, buttons should stretch to full width (or at least be wide enough to tap). On wider screens, shrink back to auto width:
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
// All step button rows
|
||||||
|
<div className="flex flex-col sm:flex-row gap-3 mt-6">
|
||||||
|
<button className={`${MD3_BTN_OUTLINED} w-full sm:w-auto`}>Back</button>
|
||||||
|
<button className={`${MD3_BTN_FILLED} w-full sm:w-auto`}>Next</button>
|
||||||
|
</div>
|
||||||
|
```
|
||||||
|
|
||||||
|
**StepIndicator — mobile collapse:** On small screens, the 4-step indicator with labels is cramped. The label text below each circle can be hidden on mobile with `hidden sm:block`:
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
// StepIndicator.tsx — hide label text on mobile
|
||||||
|
<span className="text-xs text-primary hidden sm:block">{label}</span>
|
||||||
|
// (for all three states: completed, active, future)
|
||||||
|
```
|
||||||
|
|
||||||
|
The connector lines and circles remain visible on mobile — only text labels collapse.
|
||||||
|
|
||||||
|
**App.tsx container:** The outer wrapper `py-12 px-4` is already reasonable for mobile. The `max-w-2xl` container is appropriate. No change needed at the shell level.
|
||||||
|
|
||||||
|
**When to use:** Any time a multi-column layout needs to degrade gracefully on narrow viewports.
|
||||||
|
|
||||||
|
### Pattern 2: MD3 Focus-Visible Indicators (POLISH-02)
|
||||||
|
|
||||||
|
**What:** MD3 specifies a 3dp focus ring. The current `md3-buttons.ts` already uses `focus-visible:ring-2` (2px). This needs to change to a 3px indicator. In Tailwind v4, `ring-3` is available (Tailwind v4 introduced `ring-3` as a standard utility — previously only `ring-2` and `ring-4` existed as defaults in v3, but v4 supports arbitrary-value-free ring-3).
|
||||||
|
|
||||||
|
**Verification:** Tailwind v4 uses a numeric scale (`ring-1`, `ring-2`, `ring-3`, `ring-4`) — confirmed by Tailwind v4 changelog and the migration from v3 where ring sizes follow the spacing scale.
|
||||||
|
|
||||||
|
**Current state of buttons (md3-buttons.ts):**
|
||||||
|
```ts
|
||||||
|
// Current: ring-2 with primary/50 opacity
|
||||||
|
'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary/50'
|
||||||
|
```
|
||||||
|
|
||||||
|
**Update to:**
|
||||||
|
```ts
|
||||||
|
// POLISH-02: 3px MD3 focus ring
|
||||||
|
'focus-visible:outline-none focus-visible:ring-3 focus-visible:ring-primary'
|
||||||
|
```
|
||||||
|
|
||||||
|
**BackendCard — currently missing focus-visible:**
|
||||||
|
```tsx
|
||||||
|
// BackendCard.tsx — add focus-visible ring
|
||||||
|
'focus-visible:outline-none focus-visible:ring-3 focus-visible:ring-primary focus-visible:ring-offset-2'
|
||||||
|
```
|
||||||
|
|
||||||
|
**Select element in FieldRenderer:**
|
||||||
|
```tsx
|
||||||
|
// FieldRenderer.tsx select branch — update focus ring
|
||||||
|
'border-outline focus:ring-2' // → change to focus-visible:ring-3
|
||||||
|
```
|
||||||
|
|
||||||
|
**Tooltip buttons in FieldRenderer — currently only hover/click, no focus indicator:**
|
||||||
|
```tsx
|
||||||
|
// FieldRenderer.tsx tooltip buttons — add focus-visible
|
||||||
|
className="... focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary rounded"
|
||||||
|
```
|
||||||
|
|
||||||
|
**StepIndicator completed-step buttons — current:**
|
||||||
|
```tsx
|
||||||
|
// group-focus-visible:ring-2 group-focus-visible:ring-primary/50
|
||||||
|
// Update to group-focus-visible:ring-3 group-focus-visible:ring-primary
|
||||||
|
```
|
||||||
|
|
||||||
|
**ThemeToggle:** Read the file to confirm current focus state — likely needs focus-visible ring added.
|
||||||
|
|
||||||
|
**Key insight:** The `focus-visible:` CSS pseudo-class only shows the outline during keyboard navigation (Tab key), not after mouse clicks. This is the correct MD3 and WCAG behavior. Browsers natively handle the distinction via the `:focus-visible` CSS pseudo-class.
|
||||||
|
|
||||||
|
### Pattern 3: Step Transition Animation (POLISH-03)
|
||||||
|
|
||||||
|
**What:** A subtle fade+slide-up animation when the step content changes. Applied to the content wrapper in `App.tsx` (`WizardShell`). The animation triggers when the step content re-renders.
|
||||||
|
|
||||||
|
**Mechanism — React key trick:** The step content `div` in `WizardShell` receives a `key={state.currentStep}`. When `currentStep` changes, React unmounts the old div and mounts a new one, which triggers the CSS `@keyframes` animation from its initial state.
|
||||||
|
|
||||||
|
**CSS in index.css:**
|
||||||
|
```css
|
||||||
|
@keyframes step-in {
|
||||||
|
from {
|
||||||
|
opacity: 0;
|
||||||
|
transform: translateY(8px);
|
||||||
|
}
|
||||||
|
to {
|
||||||
|
opacity: 1;
|
||||||
|
transform: translateY(0);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/* Reduced-motion guard */
|
||||||
|
@media (prefers-reduced-motion: reduce) {
|
||||||
|
.animate-step-in {
|
||||||
|
animation: none;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Tailwind v4 custom animation registration (in index.css @theme block):**
|
||||||
|
```css
|
||||||
|
@theme {
|
||||||
|
/* ... existing tokens ... */
|
||||||
|
--animate-step-in: step-in 200ms ease-out both;
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
This registers `animate-step-in` as a Tailwind utility class (Tailwind v4 `@theme` block wires `--animate-*` to `animate-*` utilities).
|
||||||
|
|
||||||
|
**App.tsx usage:**
|
||||||
|
```tsx
|
||||||
|
// WizardShell — add key + animation class to step content wrapper
|
||||||
|
<div key={state.currentStep} className="mt-8 animate-step-in">
|
||||||
|
{CurrentStep}
|
||||||
|
</div>
|
||||||
|
```
|
||||||
|
|
||||||
|
**Alternative without @theme:** Apply animation directly via `style` prop or CSS class defined in `@layer utilities`. However, using `@theme` to register `--animate-step-in` follows the project's established pattern of wiring values through the Tailwind token system.
|
||||||
|
|
||||||
|
**prefers-reduced-motion handling:** Two approaches:
|
||||||
|
1. CSS `@media (prefers-reduced-motion: reduce) { .animate-step-in { animation: none; } }` — declarative, no JS
|
||||||
|
2. Tailwind `motion-reduce:animate-none` utility class on the same element
|
||||||
|
|
||||||
|
Both work. The CSS approach in `index.css` is consistent with the existing reduced-motion guard already in the file (the `body { transition: ... }` block at line 44). **Use approach 1 (CSS media query in index.css).**
|
||||||
|
|
||||||
|
**Duration recommendation:** 200ms with `ease-out` — subtle enough to not feel slow, long enough to be perceptible. The 8px translateY offset matches MD3 "standard" motion easing.
|
||||||
|
|
||||||
|
### Pattern 4: Auto-Scroll to First Error (POLISH-04)
|
||||||
|
|
||||||
|
**What:** When form validation fails (clicking Next with invalid fields), automatically scroll the viewport to the first field that has an error.
|
||||||
|
|
||||||
|
**Mechanism — react-hook-form error handler:** `handleSubmit(onValid, onError)` accepts a second callback that fires when validation fails. The `onError` callback receives the `errors` object (same shape as `formState.errors`).
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
// BackendSelectionStep.tsx — add error handler
|
||||||
|
function onInvalidSubmit(errors: FieldErrors<RemoteNameFormValues>) {
|
||||||
|
const firstErrorKey = Object.keys(errors)[0];
|
||||||
|
if (firstErrorKey) {
|
||||||
|
const el = document.getElementById(firstErrorKey);
|
||||||
|
el?.scrollIntoView({ behavior: 'smooth', block: 'center' });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// In JSX:
|
||||||
|
<form onSubmit={handleSubmit(onValidSubmit, onInvalidSubmit)}>
|
||||||
|
```
|
||||||
|
|
||||||
|
**For RemoteConfigStep** (multiple fields, errors from dynamic registry): The `errors` object keys match `field.key` values. The `TextFieldMD3` and `PasswordField` components already use `id={field.key}` on their input elements, so `document.getElementById(field.key)` will find the correct DOM node.
|
||||||
|
|
||||||
|
**Field ID conventions confirmed:**
|
||||||
|
- `TextFieldMD3`: `<input id={id} ...>` — id is the field key
|
||||||
|
- `PasswordField`: passes `id` to TextFieldMD3 which sets it on the input
|
||||||
|
- `FieldRenderer`: passes `field.key` as `id` to both TextFieldMD3 and native select
|
||||||
|
|
||||||
|
**Order of errors:** `Object.keys(errors)` order in JavaScript corresponds to insertion order, which for react-hook-form mirrors the order in which fields are registered. For registry-driven forms, this matches the field order in `BACKEND_REGISTRY[backendType].fields`. Using `Object.keys(errors)[0]` reliably gets the first registered field with an error.
|
||||||
|
|
||||||
|
**jsdom scrollIntoView:** `scrollIntoView` is not implemented in jsdom (returns undefined). Tests must mock it: `vi.fn()` on `Element.prototype.scrollIntoView` or via `vi.stubGlobal`. This is a known pattern in the project (Phase 8 used `vi.stubGlobal` for localStorage/matchMedia).
|
||||||
|
|
||||||
|
### Anti-Patterns to Avoid
|
||||||
|
|
||||||
|
- **Using `focus:` instead of `focus-visible:`:** `focus:` shows the ring on mouse clicks too, which is visually noisy. `focus-visible:` is the correct MD3/WCAG approach — shows ring only during keyboard navigation.
|
||||||
|
- **CSS `outline: none` without `focus-visible` replacement:** Removing the default browser outline without providing a `focus-visible` replacement breaks keyboard accessibility entirely.
|
||||||
|
- **CSS animation on the step component itself instead of the wrapper:** Animating the step component means it only plays when the component instance is created — if the same component type renders at the same position, React reuses the instance and no animation fires. The `key={state.currentStep}` on the wrapper div forces a fresh mount.
|
||||||
|
- **Using `prefers-reduced-motion` only in JS:** The project convention (established in Phase 8) is to handle motion preferences in CSS with `@media (prefers-reduced-motion)`. Do not add a JS media query listener.
|
||||||
|
- **`scrollIntoView` without `behavior: 'smooth'`:** Instant scroll is jarring. `block: 'center'` keeps the errored field in the user's field of view with context above and below.
|
||||||
|
- **Forgetting `scrollIntoView` mock in tests:** jsdom does not implement `scrollIntoView`. Any test that triggers the error callback will throw or fail silently if the mock is not set up.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Don't Hand-Roll
|
||||||
|
|
||||||
|
| Problem | Don't Build | Use Instead | Why |
|
||||||
|
|---------|-------------|-------------|-----|
|
||||||
|
| Responsive breakpoints | Custom media query hooks in JS | Tailwind `sm:` prefix | CSS-only, no JS overhead, standard project approach |
|
||||||
|
| Focus detection | `onFocus`/`onBlur` state + conditional ring class | `focus-visible:` Tailwind utility | CSS `:focus-visible` pseudo-class is native, zero JS |
|
||||||
|
| Animation orchestration | Framer Motion or React Spring | CSS `@keyframes` + Tailwind `animate-` | Zero new dependency; 200ms fade/slide is trivially simple |
|
||||||
|
| Scroll-to-error | Custom scroll calculation | `element.scrollIntoView()` | Native browser API; handles all edge cases (position, overflow) |
|
||||||
|
| Reduced motion JS hook | `window.matchMedia('(prefers-reduced-motion)')` listener | CSS `@media (prefers-reduced-motion: reduce)` | Declarative; consistent with existing pattern in index.css line 44 |
|
||||||
|
|
||||||
|
**Key insight:** All four POLISH requirements have direct CSS/browser-native solutions. The project's "zero new runtime dependencies" constraint for v1.2 is trivially satisfied.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Common Pitfalls
|
||||||
|
|
||||||
|
### Pitfall 1: `ring-3` availability in Tailwind v4
|
||||||
|
**What goes wrong:** Developer uses `ring-3` expecting 3px, but the utility may not exist in all Tailwind versions the same way.
|
||||||
|
**Why it happens:** Tailwind v3 defaults included `ring-1` (1px), `ring-2` (2px), `ring-4` (4px), `ring-8` (8px). `ring-3` was added in Tailwind v4 as part of the extended numeric scale.
|
||||||
|
**How to avoid:** Tailwind v4.2.2 is confirmed in package.json. In v4, the ring scale follows `ring-{n}` matching shadow scale — `ring-3` = 3px ring is valid. Alternatively, use `ring-[3px]` as an arbitrary value to be safe.
|
||||||
|
**Warning signs:** If `ring-3` class has no visual effect, fall back to `ring-[3px]`.
|
||||||
|
|
||||||
|
### Pitfall 2: BackendCard stacking breaks existing test assertions
|
||||||
|
**What goes wrong:** Tests in `BackendSelectionStep.test.tsx` use `screen.getAllByRole('button')` and check DOM order. Adding a wrapper grid div does NOT change the button elements themselves — their order is preserved.
|
||||||
|
**Why it happens:** `getAllByRole` returns elements in DOM tree order. Adding a `<div className="grid ...">` wrapper around the cards preserves this order.
|
||||||
|
**How to avoid:** Only wrap the `data-testid="backend-cards"` div content in a grid container — no structural changes to the card buttons. Tests will continue to pass.
|
||||||
|
|
||||||
|
### Pitfall 3: `focus-visible:` not working in jsdom tests
|
||||||
|
**What goes wrong:** `focus-visible:` CSS classes are applied correctly in the DOM but jsdom tests cannot verify visual appearance.
|
||||||
|
**Why it happens:** jsdom does not compute CSS. Tests cannot assert "ring is visible."
|
||||||
|
**How to avoid:** For POLISH-02, tests should verify the className string contains the focus-visible utility (DOM attribute check) rather than visual rendering. Example:
|
||||||
|
```tsx
|
||||||
|
expect(button.className).toContain('focus-visible:ring-3');
|
||||||
|
```
|
||||||
|
Or simply skip visual focus tests — this requirement is a CSS/browser concern best verified by manual keyboard navigation.
|
||||||
|
|
||||||
|
### Pitfall 4: React key animation fires on initial render
|
||||||
|
**What goes wrong:** Adding `key={state.currentStep}` to the step wrapper means the animation also plays on the FIRST render (step 0 initial mount).
|
||||||
|
**Why it happens:** React mounts the div for the first time with key=0, triggering the keyframe animation.
|
||||||
|
**How to avoid:** This is acceptable behavior — the animation on initial render is imperceptible because the page itself is loading. A `useState` flag to suppress initial animation adds complexity for no visible benefit. Accept this tradeoff.
|
||||||
|
|
||||||
|
### Pitfall 5: `scrollIntoView` in jsdom throws unless mocked
|
||||||
|
**What goes wrong:** Tests that call `handleSubmit` with an invalid form — triggering the `onInvalidSubmit` callback — will call `element.scrollIntoView()` which is not implemented in jsdom and throws `TypeError: el.scrollIntoView is not a function`.
|
||||||
|
**Why it happens:** jsdom only implements a subset of the browser DOM API.
|
||||||
|
**How to avoid:** Add to test file setup:
|
||||||
|
```ts
|
||||||
|
// At top of test file or in beforeEach
|
||||||
|
Element.prototype.scrollIntoView = vi.fn();
|
||||||
|
```
|
||||||
|
Or use `vi.stubGlobal` if preferred. This pattern is documented in the project (Phase 8 pattern for localStorage/matchMedia).
|
||||||
|
|
||||||
|
### Pitfall 6: StepIndicator label collapse hides text in tests
|
||||||
|
**What goes wrong:** Tests that check for step label text (e.g., `screen.getByText('Backend')`) may fail if the label `<span>` is `hidden` on mobile by `hidden sm:block`.
|
||||||
|
**Why it happens:** jsdom renders the DOM but applies CSS classes. `hidden` translates to `display: none` in Tailwind — jsdom does NOT compute this style, so the element IS in the DOM and accessible to `getByText`.
|
||||||
|
**How to avoid:** `hidden` class does not cause `queryByText` to fail in jsdom because jsdom ignores CSS display properties. No test change needed.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Code Examples
|
||||||
|
|
||||||
|
Verified patterns from codebase and established project conventions:
|
||||||
|
|
||||||
|
### POLISH-01: Responsive Backend Cards Grid
|
||||||
|
```tsx
|
||||||
|
// BackendSelectionStep.tsx — wrap backend-cards div
|
||||||
|
<div data-testid="backend-cards" className="grid grid-cols-1 sm:grid-cols-2 gap-3 mt-4">
|
||||||
|
{Object.entries(BACKEND_REGISTRY).map(([type, entry]) => (
|
||||||
|
<BackendCard
|
||||||
|
key={type}
|
||||||
|
name={entry.displayName}
|
||||||
|
description={entry.description}
|
||||||
|
selected={state.remote.backendType === type}
|
||||||
|
onClick={() => handleCardClick(type as BackendType)}
|
||||||
|
/>
|
||||||
|
))}
|
||||||
|
</div>
|
||||||
|
```
|
||||||
|
|
||||||
|
### POLISH-01: Responsive Button Row (all step components)
|
||||||
|
```tsx
|
||||||
|
// Replace: <div className="flex gap-3 mt-6">
|
||||||
|
// With:
|
||||||
|
<div className="flex flex-col sm:flex-row gap-3 mt-6">
|
||||||
|
<button type="button" className={`${MD3_BTN_OUTLINED} w-full sm:w-auto`}>Back</button>
|
||||||
|
<button type="submit" className={`${MD3_BTN_FILLED} w-full sm:w-auto`}>Next</button>
|
||||||
|
</div>
|
||||||
|
```
|
||||||
|
|
||||||
|
### POLISH-01: StepIndicator label collapse
|
||||||
|
```tsx
|
||||||
|
// StepIndicator.tsx — add hidden sm:block to all three label spans
|
||||||
|
// Completed step:
|
||||||
|
<span className="text-xs text-primary hidden sm:block">{label}</span>
|
||||||
|
// Active step:
|
||||||
|
<span className="text-xs text-primary font-semibold hidden sm:block">{label}</span>
|
||||||
|
// Future step:
|
||||||
|
<span className="text-xs text-on-surface-container/40 hidden sm:block">{label}</span>
|
||||||
|
```
|
||||||
|
|
||||||
|
### POLISH-02: Updated md3-buttons.ts focus ring
|
||||||
|
```ts
|
||||||
|
// md3-buttons.ts — update all three button constants
|
||||||
|
// Change focus-visible:ring-2 focus-visible:ring-primary/50
|
||||||
|
// To:
|
||||||
|
'focus-visible:outline-none focus-visible:ring-3 focus-visible:ring-primary focus-visible:ring-offset-2'
|
||||||
|
```
|
||||||
|
|
||||||
|
### POLISH-02: BackendCard focus ring
|
||||||
|
```tsx
|
||||||
|
// BackendCard.tsx — add to className array
|
||||||
|
'focus-visible:outline-none focus-visible:ring-3 focus-visible:ring-primary focus-visible:ring-offset-2'
|
||||||
|
```
|
||||||
|
|
||||||
|
### POLISH-03: Animation in index.css
|
||||||
|
```css
|
||||||
|
/* Add to index.css after existing @layer base block */
|
||||||
|
@keyframes step-in {
|
||||||
|
from {
|
||||||
|
opacity: 0;
|
||||||
|
transform: translateY(8px);
|
||||||
|
}
|
||||||
|
to {
|
||||||
|
opacity: 1;
|
||||||
|
transform: translateY(0);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@media (prefers-reduced-motion: reduce) {
|
||||||
|
.animate-step-in {
|
||||||
|
animation: none !important;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
```css
|
||||||
|
/* In existing @theme block — add after existing --color-* tokens */
|
||||||
|
@theme {
|
||||||
|
/* ... existing tokens ... */
|
||||||
|
--animate-step-in: step-in 200ms ease-out both;
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
### POLISH-03: App.tsx step wrapper with key + animation
|
||||||
|
```tsx
|
||||||
|
// WizardShell in App.tsx — wrap CurrentStep
|
||||||
|
<div key={state.currentStep} className="mt-8 animate-step-in">
|
||||||
|
{CurrentStep}
|
||||||
|
</div>
|
||||||
|
```
|
||||||
|
|
||||||
|
### POLISH-04: react-hook-form error handler with scrollIntoView
|
||||||
|
```tsx
|
||||||
|
// BackendSelectionStep.tsx
|
||||||
|
import type { FieldErrors } from 'react-hook-form';
|
||||||
|
|
||||||
|
function onInvalidSubmit(errors: FieldErrors<RemoteNameFormValues>) {
|
||||||
|
const firstKey = Object.keys(errors)[0];
|
||||||
|
if (firstKey) {
|
||||||
|
document.getElementById(firstKey)?.scrollIntoView({ behavior: 'smooth', block: 'center' });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// In JSX:
|
||||||
|
<form onSubmit={handleSubmit(onValidSubmit, onInvalidSubmit)}>
|
||||||
|
```
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
// RemoteConfigStep.tsx — same pattern, errors are record of FieldError
|
||||||
|
function onInvalidSubmit(errors: FieldErrors) {
|
||||||
|
const firstKey = Object.keys(errors)[0];
|
||||||
|
if (firstKey) {
|
||||||
|
document.getElementById(firstKey)?.scrollIntoView({ behavior: 'smooth', block: 'center' });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
### POLISH-04: Test setup for scrollIntoView mock
|
||||||
|
```ts
|
||||||
|
// In test file using beforeEach or at top-level
|
||||||
|
import { beforeEach, vi } from 'vitest';
|
||||||
|
beforeEach(() => {
|
||||||
|
Element.prototype.scrollIntoView = vi.fn();
|
||||||
|
});
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## State of the Art
|
||||||
|
|
||||||
|
| Old Approach | Current Approach | When Changed | Impact |
|
||||||
|
|--------------|------------------|--------------|--------|
|
||||||
|
| `focus:ring-2` (shows on mouse click too) | `focus-visible:ring-3` (keyboard only) | Phase 9 partially, Phase 11 completes | Cleaner UX: no ring flash on mouse users |
|
||||||
|
| No motion | CSS `@keyframes` with reduced-motion guard | Phase 11 | Subtle animation that respects accessibility preference |
|
||||||
|
| Static layout (flex-col everywhere) | Mobile-first responsive grid/flex | Phase 11 | Cards and buttons adapt to viewport |
|
||||||
|
| No scroll-to-error | `scrollIntoView` on error | Phase 11 | Reduces confusion when validation fails on long forms |
|
||||||
|
|
||||||
|
**Deprecated/outdated:**
|
||||||
|
- `focus:ring-2 focus:ring-primary/50` in md3-buttons.ts: replaced by `focus-visible:ring-3 focus-visible:ring-primary` in this phase
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Open Questions
|
||||||
|
|
||||||
|
1. **Should BackendCard use a 2-column grid or keep single-column stacked on mobile?**
|
||||||
|
- What we know: POLISH-01 says "backend cards stack vertically" — this is the mobile state. A `grid-cols-1 sm:grid-cols-2` layout satisfies "stack on mobile, 2-col on wider screen."
|
||||||
|
- What's unclear: Whether a 2-column grid on desktop is desired (7 backends = 3.5 rows of 2) vs current implicit single-column.
|
||||||
|
- Recommendation: Use `grid grid-cols-1 sm:grid-cols-2 gap-3` — satisfies the requirement exactly (stacked on mobile) while being more space-efficient on desktop. If the user wants single-column always, the `sm:grid-cols-2` can be omitted with no functional change to the mobile behavior.
|
||||||
|
|
||||||
|
2. **focus-visible ring-offset color in dark mode**
|
||||||
|
- What we know: `ring-offset` creates a gap between the ring and the element. In dark mode, the offset background would be the dark surface color — this looks correct without extra configuration since `ring-offset-2` uses the element's background.
|
||||||
|
- What's unclear: Whether `ring-offset-2` looks correct on BackendCard dark surface (`bg-surface-container` = gray-800 in dark).
|
||||||
|
- Recommendation: Add `ring-offset-surface` or simply omit ring-offset if it creates visual issues — the 3px ring alone is sufficient for MD3 compliance.
|
||||||
|
|
||||||
|
3. **Animation on intro-to-wizard transition (clicking "Get Started")**
|
||||||
|
- What we know: `key={state.currentStep}` only animates on step changes within the wizard, not when `showIntro` flips to `false`.
|
||||||
|
- What's unclear: Whether POLISH-03 ("step transitions") applies to the intro→wizard transition too.
|
||||||
|
- Recommendation: Out of scope per POLISH-03 wording ("step transitions"). The intro→wizard reveal can be a future enhancement. Keep scope to the 4 wizard step transitions only.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Validation Architecture
|
||||||
|
|
||||||
|
### Test Framework
|
||||||
|
| Property | Value |
|
||||||
|
|----------|-------|
|
||||||
|
| Framework | Vitest 4.1.1 + @testing-library/react 16.3.2 |
|
||||||
|
| Config file | `vitest.config.ts` (environment: node — per-file override with `@vitest-environment jsdom`) |
|
||||||
|
| Quick run command | `npx vitest run` |
|
||||||
|
| Full suite command | `npx vitest run` |
|
||||||
|
|
||||||
|
### Phase Requirements → Test Map
|
||||||
|
|
||||||
|
| Req ID | Behavior | Test Type | Automated Command | File Exists? |
|
||||||
|
|--------|----------|-----------|-------------------|-------------|
|
||||||
|
| POLISH-01 | Button rows use `flex-col sm:flex-row` on all step components | unit (className assertion) | `npx vitest run src/components/wizard/BackendSelectionStep.test.tsx` | ✅ needs new test |
|
||||||
|
| POLISH-01 | Backend cards container has `grid` class | unit (DOM structure) | `npx vitest run src/components/wizard/BackendSelectionStep.test.tsx` | ✅ needs new test |
|
||||||
|
| POLISH-01 | StepIndicator label spans have `hidden sm:block` class | unit (className assertion) | `npx vitest run src/components/wizard/StepIndicator.test.tsx` | ✅ needs new test |
|
||||||
|
| POLISH-02 | MD3_BTN_FILLED contains `focus-visible:ring-3` | unit (string constant check) | `npx vitest run` | ❌ Wave 0 (new test file or inline assertion) |
|
||||||
|
| POLISH-02 | BackendCard className includes `focus-visible:ring-3` | unit (className assertion) | `npx vitest run src/components/wizard/BackendSelectionStep.test.tsx` | ✅ needs new test |
|
||||||
|
| POLISH-03 | Step content wrapper has `animate-step-in` class | unit (DOM assertion) | `npx vitest run src/App.test.tsx` | ✅ needs new test |
|
||||||
|
| POLISH-03 | CSS @keyframes step-in defined in index.css | manual/visual | N/A | manual verification |
|
||||||
|
| POLISH-04 | `scrollIntoView` called on first error field when remote name is empty | unit | `npx vitest run src/components/wizard/BackendSelectionStep.test.tsx` | ✅ needs new test |
|
||||||
|
| POLISH-04 | `scrollIntoView` called on first error field in RemoteConfigStep | unit | `npx vitest run src/components/wizard/RemoteConfigStep.test.tsx` | ✅ needs new test |
|
||||||
|
|
||||||
|
### Sampling Rate
|
||||||
|
- **Per task commit:** `npx vitest run`
|
||||||
|
- **Per wave merge:** `npx vitest run`
|
||||||
|
- **Phase gate:** Full suite green before `/gsd:verify-work`
|
||||||
|
|
||||||
|
### Wave 0 Gaps
|
||||||
|
- [ ] Optional: `src/styles/md3-buttons.test.ts` — verify `focus-visible:ring-3` string in button constants (alternatively, assertions can be added to existing test files)
|
||||||
|
|
||||||
|
*(All other test files exist. New test cases are added inline to existing test files as part of each task implementation.)*
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Component Audit for POLISH-02 (focus-visible completeness)
|
||||||
|
|
||||||
|
Every interactive element must be verified. Current state from codebase read:
|
||||||
|
|
||||||
|
| Element | File | Current focus state | Action needed |
|
||||||
|
|---------|------|--------------------|-|
|
||||||
|
| MD3_BTN_FILLED button | md3-buttons.ts | `focus-visible:ring-2 focus-visible:ring-primary/50` | Upgrade to `ring-3 ring-primary` |
|
||||||
|
| MD3_BTN_OUTLINED button | md3-buttons.ts | `focus-visible:ring-2 focus-visible:ring-primary/50` | Upgrade to `ring-3 ring-primary` |
|
||||||
|
| MD3_BTN_TEXT button | md3-buttons.ts | `focus-visible:ring-2 focus-visible:ring-primary/50` | Upgrade to `ring-3 ring-primary` |
|
||||||
|
| BackendCard button | BackendCard.tsx | No focus-visible classes | Add `focus-visible:ring-3 focus-visible:ring-primary` |
|
||||||
|
| StepIndicator completed-step button | StepIndicator.tsx | `group-focus-visible:ring-2 group-focus-visible:ring-primary/50` | Upgrade to `group-focus-visible:ring-3 group-focus-visible:ring-primary` |
|
||||||
|
| TextFieldMD3 input | TextFieldMD3.tsx | `focus:ring-2` (not focus-visible) | Change to `focus-visible:ring-2` (or keep for input — focus on input is expected for mouse users too) |
|
||||||
|
| FieldRenderer tooltip buttons | FieldRenderer.tsx | No focus-visible | Add `focus-visible:ring-2 focus-visible:ring-primary rounded` |
|
||||||
|
| FieldRenderer select | FieldRenderer.tsx | `focus:ring-2` | Change to `focus-visible:ring-2` |
|
||||||
|
| ThemeToggle button | ThemeToggle.tsx | Unknown — needs read | Verify and add if missing |
|
||||||
|
|
||||||
|
**Note on TextFieldMD3 input:** For text inputs, `focus:ring-2` (not `focus-visible:`) is acceptable per WCAG 2.1 — it's conventional to show a focus ring on inputs even when focused by mouse (helps users know which field is active). The POLISH-02 requirement specifically calls out `focus-visible` for "interactive elements" — this primarily targets buttons, cards, and controls that don't need a constant focus indicator when mouse-clicked. Keep text inputs as-is with `focus:ring`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Sources
|
||||||
|
|
||||||
|
### Primary (HIGH confidence)
|
||||||
|
- Direct codebase inspection — `src/App.tsx`, `src/index.css`, `src/styles/md3-buttons.ts`, `src/components/wizard/StepIndicator.tsx`, `src/components/ui/BackendCard.tsx`, `src/components/ui/TextFieldMD3.tsx`, `src/components/ui/FieldRenderer.tsx`, `src/components/wizard/BackendSelectionStep.tsx`, `src/components/wizard/RemoteConfigStep.tsx`, `src/components/wizard/DeploymentStep.tsx`, `src/components/wizard/ReviewStep.tsx`
|
||||||
|
- Direct test file inspection — `src/App.test.tsx`, `src/components/wizard/BackendSelectionStep.test.tsx`, `src/components/wizard/StepIndicator.test.tsx`, `src/components/wizard/RemoteConfigStep.test.tsx`
|
||||||
|
- `package.json` — confirmed Tailwind v4.2.2, Vitest 4.1.1, react-hook-form 7.72.0
|
||||||
|
- `vitest.config.ts` — confirmed test environment setup
|
||||||
|
|
||||||
|
### Secondary (MEDIUM confidence)
|
||||||
|
- Tailwind v4 documentation: `ring-3` utility availability, `@theme --animate-*` registration, mobile-first breakpoint behavior — consistent with v4 changelog (breakpoints unchanged from v3, ring scale extended in v4)
|
||||||
|
- MDN CSS `@keyframes` + `prefers-reduced-motion` media query — web standard, HIGH confidence
|
||||||
|
- react-hook-form v7 documentation: `handleSubmit(onValid, onInvalid)` second argument — confirmed in existing codebase usage pattern
|
||||||
|
|
||||||
|
### Tertiary (LOW confidence)
|
||||||
|
- None — all claims verified by codebase inspection or official specification
|
||||||
|
|
||||||
|
## Metadata
|
||||||
|
|
||||||
|
**Confidence breakdown:**
|
||||||
|
- Standard stack: HIGH — verified from package.json and codebase
|
||||||
|
- Architecture: HIGH — all integration points verified by reading actual source files
|
||||||
|
- Pitfalls: HIGH — derived from reading actual test files and component implementations
|
||||||
|
- Responsive patterns: HIGH — Tailwind mobile-first is a stable API unchanged from v3 to v4
|
||||||
|
|
||||||
|
**Research date:** 2026-04-01
|
||||||
|
**Valid until:** 2026-05-01 (stable Tailwind/React APIs; only invalidated by component refactors)
|
||||||
Reference in New Issue
Block a user