docs(12): research phase dark mode visibility fixes
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,328 @@
|
||||
# Phase 12: Dark Mode Visibility Fixes - Research
|
||||
|
||||
**Researched:** 2026-04-01
|
||||
**Domain:** CSS dark mode theming — Tailwind v4 CSS custom properties, native form control styling
|
||||
**Confidence:** HIGH (all findings based on direct source code audit of the live codebase)
|
||||
|
||||
## Summary
|
||||
|
||||
Phase 12 addresses a specific set of dark-mode visibility regressions introduced during the v1.2 UI overhaul. The project uses a robust two-layer CSS token system (raw `--r2b-*` custom properties overridden per `.dark` class, wired to Tailwind utility classes via `@theme`). The architecture is sound. The problem is that several components were incompletely migrated: their structural/container elements received semantic tokens but their text elements, headings, and native form controls were left unstyled, rendering them invisible or unreadable in dark mode.
|
||||
|
||||
The issues fall into three categories: (1) bare `<h2>` headings with no className in four wizard steps, (2) DeploymentStep using completely unstyled native form controls (fieldset, legend, labels, checkbox, radio), and (3) a `<select>` element in FieldRenderer with no background or text color class, causing browsers to apply a white system background in dark mode.
|
||||
|
||||
The fix approach is purely additive: apply existing semantic token classes (`text-on-surface`, `bg-surface-container`, etc.) to the affected elements. Zero new CSS variables, zero new design decisions, zero new dependencies. The existing `.dark` token overrides in `index.css` already define the correct dark-mode values — the work is connecting those tokens to the elements that currently bypass them.
|
||||
|
||||
**Primary recommendation:** Audit each component's rendered elements and add semantic token classes to every text and interactive element that currently has none. Do not introduce hardcoded colors.
|
||||
|
||||
## Standard Stack
|
||||
|
||||
### Core (already in project — no changes needed)
|
||||
|
||||
| Library | Version | Purpose | Notes |
|
||||
|---------|---------|---------|-------|
|
||||
| Tailwind v4 | ^4.2.2 | Utility classes via `@theme` + CSS custom properties | Already wired; all needed tokens exist |
|
||||
| `@tailwindcss/vite` | ^4.2.2 | Tailwind v4 Vite integration | No postcss config needed |
|
||||
|
||||
### CSS Token System (already complete in `src/index.css`)
|
||||
|
||||
All required tokens exist. Dark variants are already correct in `.dark` block:
|
||||
|
||||
| Token | Light value | Dark value | Use |
|
||||
|-------|------------|-----------|-----|
|
||||
| `--r2b-on-surface` | `#111827` (gray-900) | `#F9FAFB` (gray-50) | Primary text, headings |
|
||||
| `--r2b-on-surface-container` | `#374151` (gray-700) | `#D1D5DB` (gray-300) | Labels, secondary text |
|
||||
| `--r2b-surface-container` | `#FFFFFF` | `#1F2937` (gray-800) | Input/select backgrounds |
|
||||
| `--r2b-outline` | `#D1D5DB` | `#4B5563` (gray-600) | Borders |
|
||||
| `--r2b-warning` | `#D97706` | `#FCD34D` | Warning text |
|
||||
| `--r2b-success` | `#15803D` | `#86EFAC` | Success text |
|
||||
|
||||
No installation needed. No new dependencies.
|
||||
|
||||
## Architecture Patterns
|
||||
|
||||
### Two-Layer Token Pattern (established, Phase 08-01)
|
||||
|
||||
```css
|
||||
/* Layer 1: raw values, overridden per theme */
|
||||
@layer base {
|
||||
:root { --r2b-on-surface: #111827; }
|
||||
.dark { --r2b-on-surface: #F9FAFB; }
|
||||
}
|
||||
|
||||
/* Layer 2: wire to Tailwind utility classes */
|
||||
@theme {
|
||||
--color-on-surface: var(--r2b-on-surface);
|
||||
}
|
||||
```
|
||||
|
||||
This means `text-on-surface` already works correctly in both themes. The fix is always "add the right class", never "add a new CSS variable".
|
||||
|
||||
### Semantic Class Application Pattern
|
||||
|
||||
**What:** Apply semantic Tailwind utility classes to elements that currently have no className or only structural classes.
|
||||
|
||||
**Key classes for this phase:**
|
||||
- `text-on-surface` — primary text, headings (h1, h2, h3)
|
||||
- `text-on-surface-container` — labels, secondary text, legend elements
|
||||
- `bg-surface-container` — input/select backgrounds
|
||||
- `text-on-surface-container/60` — placeholder-like text, muted elements
|
||||
- `border-outline` — form control borders
|
||||
|
||||
### Anti-Patterns to Avoid
|
||||
|
||||
- **Hardcoded color classes** (`text-gray-900`, `text-black`, `bg-white`): These ignore the `.dark` cascade — they were the root cause of v1.1 tech debt that THEME-01 fixed.
|
||||
- **Adding `dark:` variants**: The project uses `.dark` class toggle (not `prefers-color-scheme`); the `@custom-variant dark (&:where(.dark, .dark *))` declaration in `index.css` means semantic tokens already handle dark automatically. Never add explicit `dark:` prefixes.
|
||||
- **Styling `<option>` elements**: Browser-native `<option>` elements cannot be reliably styled cross-browser via CSS. Accept system rendering for options; only style the `<select>` container itself.
|
||||
|
||||
## Don't Hand-Roll
|
||||
|
||||
| Problem | Don't Build | Use Instead | Why |
|
||||
|---------|-------------|-------------|-----|
|
||||
| Dark select styling | Custom `<select>` component | Add `bg-surface-container text-on-surface` classes to existing `<select>` | `<option>` can't be styled cross-browser; native select with container styling is sufficient |
|
||||
| Checkbox/radio dark mode | Custom checkboxes | Accept native rendering + add `accent-primary` Tailwind class | Already in project out-of-scope (REQUIREMENTS.md: "Custom checkbox/radio styling: High effort for few toggles") |
|
||||
|
||||
**Key insight:** All tokens already exist and resolve correctly in dark mode. The fix is application of existing classes, not new infrastructure.
|
||||
|
||||
## Specific Issues Found (Source Code Audit)
|
||||
|
||||
### Issue 1: Bare `<h2>` headings — CRITICAL (invisible in dark mode)
|
||||
|
||||
**Files affected:**
|
||||
- `src/components/wizard/BackendSelectionStep.tsx` line 65: `<h2>Step 1: Select Backend</h2>`
|
||||
- `src/components/wizard/RemoteConfigStep.tsx` line 65: `<h2>Step 2: Configure {backendLabel[backendType]}</h2>`
|
||||
- `src/components/wizard/DeploymentStep.tsx` line 24: `<h2>Step 3: Deployment Options</h2>`
|
||||
- `src/components/wizard/ReviewStep.tsx` line 75: `<h2>Step 4: Review & Download</h2>`
|
||||
|
||||
**Root cause:** No `className` at all — browser renders `<h2>` in its default black color, which is invisible on the dark `bg-surface` (#111827) background.
|
||||
|
||||
**Fix:** Add `className="text-2xl font-bold text-on-surface mb-2"` (matching the pattern used by IntroSection's `<h2>` in App.tsx line 19).
|
||||
|
||||
**Reference pattern (correct, from App.tsx line 19):**
|
||||
```tsx
|
||||
<h2 className="text-2xl font-bold text-on-surface mb-4">
|
||||
```
|
||||
|
||||
### Issue 2: DeploymentStep — completely unstyled (CRITICAL)
|
||||
|
||||
**File:** `src/components/wizard/DeploymentStep.tsx`
|
||||
|
||||
**Elements with no dark-mode styling:**
|
||||
- Container `<div>` for checkbox (line 31) — no text color, labels will be black
|
||||
- `<label>` wrappers — inherit black from browser default
|
||||
- `<fieldset>` elements (lines 45, 74) — no styling
|
||||
- `<legend>` elements (lines 46, 75: "Config deployment path", "Script targets") — browser default black
|
||||
- All `<label>` elements inside fieldsets — browser default black
|
||||
- `<input type="checkbox">` — browser default (white bg in dark mode)
|
||||
- `<input type="radio">` — browser default (white bg in dark mode)
|
||||
|
||||
**Fix strategy:**
|
||||
- Add `text-on-surface` to the containing `<div>` that wraps each section, so labels inherit
|
||||
- Add `text-on-surface-container font-medium mb-2 block` to `<legend>` elements
|
||||
- Add `accent-primary` to `<input type="checkbox">` and `<input type="radio">` (Tailwind utility for native control accent color)
|
||||
- Wrap each control group section in a styled container for visual separation
|
||||
|
||||
### Issue 3: FieldRenderer `<select>` — no background/text color (SIGNIFICANT)
|
||||
|
||||
**File:** `src/components/ui/FieldRenderer.tsx` lines 67–78
|
||||
|
||||
**Current classes:**
|
||||
```
|
||||
'w-full rounded-md border px-3 py-2 text-sm focus:outline-none focus-visible:ring-2'
|
||||
```
|
||||
|
||||
**Missing:** `bg-surface-container text-on-surface`
|
||||
|
||||
**Root cause:** Without explicit `bg-*` class, browsers apply system default background to `<select>` (typically white on Windows, even in dark mode). Text defaults to browser default (black).
|
||||
|
||||
**Fix:** Add `bg-surface-container text-on-surface` to the select's className array.
|
||||
|
||||
### Issue 4: ReviewStep — security acknowledge checkbox (MINOR)
|
||||
|
||||
**File:** `src/components/wizard/ReviewStep.tsx` line 93
|
||||
|
||||
The `<input type="checkbox">` inside the warning box has no styling. The label text uses `text-warning` (correct), but the checkbox renders with browser default.
|
||||
|
||||
**Fix:** Add `accent-primary` or `accent-warning` class to the checkbox input.
|
||||
|
||||
### Issue 5: Native `select` — `<option>` elements
|
||||
|
||||
**Cannot be fixed via CSS** — browser-native `<option>` elements do not respond to CSS color properties cross-browser (especially on Windows). This is an accepted limitation documented in REQUIREMENTS.md ("Custom checkbox/radio styling: High effort"). Accept native option rendering.
|
||||
|
||||
## Common Pitfalls
|
||||
|
||||
### Pitfall 1: Using `dark:` prefix instead of semantic tokens
|
||||
|
||||
**What goes wrong:** Developer adds `dark:text-white` instead of `text-on-surface`. This bypasses the token system and creates a second class of hardcoded colors.
|
||||
|
||||
**Why it happens:** Tailwind's built-in dark mode muscle memory.
|
||||
|
||||
**How to avoid:** The project uses `.dark` class-based dark variant declared in `index.css` via `@custom-variant dark (&:where(.dark, .dark *))`. Semantic token classes (`text-on-surface`) resolve correctly in both themes already. Never use `dark:` prefix.
|
||||
|
||||
### Pitfall 2: Styling `<option>` elements
|
||||
|
||||
**What goes wrong:** Adding `bg-surface-container` or `text-on-surface` to `<option>` elements. Ignored on Windows Chrome/Edge.
|
||||
|
||||
**How to avoid:** Only style the `<select>` container, not its `<option>` children.
|
||||
|
||||
### Pitfall 3: Breaking floating label behavior in TextFieldMD3
|
||||
|
||||
**What goes wrong:** Adding `bg-surface-container` to the `<input>` inside TextFieldMD3. The current `bg-transparent` is intentional — the input overlays the parent's background. Adding an explicit background breaks the floating label peer selector appearance.
|
||||
|
||||
**How to avoid:** Do not modify TextFieldMD3 input background. The component already renders correctly in dark mode because `bg-transparent` inherits from the parent container which uses `bg-surface-container`.
|
||||
|
||||
**Verification:** TextFieldMD3 uses `bg-transparent` on the input and the parent `<div>` has no background — the parent of FieldRenderer is expected to sit on a `bg-surface-container` surface (e.g., form sections). This is working correctly. Do not fix what is not broken.
|
||||
|
||||
### Pitfall 4: Test selector breaks from className additions
|
||||
|
||||
**What goes wrong:** Adding `className` to a bare `<h2>` can break test queries if tests use `getByRole('heading', { name: '...' })` with strict matching.
|
||||
|
||||
**How to avoid:** Tests using `getByRole` and `getByText` are unaffected by className additions. Watch out for `getByTestId` selectors that might break if structural wrappers are reorganized.
|
||||
|
||||
**From STATE.md:** "131 test selectors could break during component restyling — one-component-at-a-time discipline required"
|
||||
|
||||
## Code Examples
|
||||
|
||||
### Correct: heading with semantic token
|
||||
```tsx
|
||||
// Source: App.tsx line 19 (existing correct pattern in codebase)
|
||||
<h2 className="text-2xl font-bold text-on-surface mb-2">Step 1: Select Backend</h2>
|
||||
```
|
||||
|
||||
### Correct: styled select with dark mode support
|
||||
```tsx
|
||||
// Fix pattern for FieldRenderer.tsx
|
||||
<select
|
||||
id={field.key}
|
||||
className={[
|
||||
'w-full rounded-md border px-3 py-2 text-sm text-on-surface bg-surface-container',
|
||||
'focus:outline-none focus-visible:ring-2',
|
||||
error ? 'border-error focus:ring-error/50' : 'border-outline focus:ring-primary/50',
|
||||
].join(' ')}
|
||||
{...register(field.key)}
|
||||
>
|
||||
```
|
||||
|
||||
### Correct: native checkbox/radio with accent color
|
||||
```tsx
|
||||
// accent-primary uses the --color-primary token for native control accent color
|
||||
<input
|
||||
type="checkbox"
|
||||
className="accent-primary"
|
||||
checked={includeInstall}
|
||||
onChange={...}
|
||||
/>
|
||||
```
|
||||
|
||||
### Correct: legend/label styling
|
||||
```tsx
|
||||
// legend
|
||||
<legend className="text-sm font-medium text-on-surface-container mb-2">
|
||||
Config deployment path
|
||||
</legend>
|
||||
|
||||
// label wrapping a native control
|
||||
<label className="flex items-center gap-2 text-sm text-on-surface cursor-pointer">
|
||||
<input type="radio" className="accent-primary" ... />
|
||||
Machine-wide (C:\ProgramData\rclone\)
|
||||
</label>
|
||||
```
|
||||
|
||||
### Correct: DeploymentStep section container pattern
|
||||
```tsx
|
||||
// Wrap each section for consistent dark-mode spacing and text inheritance
|
||||
<div className="flex flex-col gap-2 mb-4">
|
||||
<span className="text-sm font-medium text-on-surface-container">Include Installation</span>
|
||||
<label className="flex items-center gap-2 text-sm text-on-surface cursor-pointer">
|
||||
<input type="checkbox" className="accent-primary" ... />
|
||||
Include rclone installation
|
||||
</label>
|
||||
</div>
|
||||
```
|
||||
|
||||
## State of the Art
|
||||
|
||||
| Old Approach | Current Approach | Status |
|
||||
|--------------|------------------|--------|
|
||||
| Hardcoded Tailwind color classes | CSS custom property tokens via `@theme` | Phase 8 complete |
|
||||
| `dark:` prefix variants | `.dark` class cascade via `@custom-variant` | Phase 8 complete |
|
||||
| Browser-default heading color | `text-on-surface` on all heading elements | THIS PHASE |
|
||||
| Unstyled native form controls | `accent-primary` + explicit text/bg tokens | THIS PHASE |
|
||||
| Unstyled `<select>` element | `bg-surface-container text-on-surface` on select | THIS PHASE |
|
||||
|
||||
## Open Questions
|
||||
|
||||
1. **DeploymentStep visual redesign scope**
|
||||
- What we know: The entire DeploymentStep uses bare native controls with no Tailwind classes — it was never styled beyond basic structure.
|
||||
- What's unclear: Phase 12 is "dark mode visibility fixes" — should DeploymentStep get a deeper visual polish (MD3-style sections) or only the minimum to fix dark mode legibility?
|
||||
- Recommendation: Minimum fix — add semantic tokens to existing structure. Full DeploymentStep MD3 redesign would be a separate phase. The phase name says "fixes" not "redesign".
|
||||
|
||||
2. **Accent color for native controls**
|
||||
- What we know: CSS `accent-color` property sets the color of native checkbox/radio/range controls. Tailwind v4 `accent-primary` should map to `--color-primary`.
|
||||
- What's unclear: Whether Tailwind v4's `accent-{color}` utility is generated from `@theme` color tokens.
|
||||
- Recommendation: Verify `accent-primary` renders at plan time. If not available, use `accent-[var(--r2b-primary)]` as fallback.
|
||||
|
||||
## Validation Architecture
|
||||
|
||||
### Test Framework
|
||||
|
||||
| Property | Value |
|
||||
|----------|-------|
|
||||
| Framework | Vitest ^4.1.1 (node environment) + @testing-library/react |
|
||||
| Config file | `vitest.config.ts` (environment: 'node') |
|
||||
| Quick run command | `npm test -- --reporter=verbose` |
|
||||
| Full suite command | `npm test` |
|
||||
|
||||
**Note:** Vitest is configured with `environment: 'node'` in `vitest.config.ts`. DOM component tests use jsdom via `@testing-library/react`. Visual dark mode correctness **cannot be automatically tested** with jsdom — CSS custom properties do not resolve in jsdom. Dark mode fixes require visual browser verification.
|
||||
|
||||
### Phase Requirements → Test Map
|
||||
|
||||
Phase 12 has no formal requirement IDs assigned (TBD per ROADMAP.md). The work is bug-fix in nature.
|
||||
|
||||
| Behavior | Test Type | Automated Command | Notes |
|
||||
|----------|-----------|-------------------|-------|
|
||||
| h2 headings have className (not empty) | Unit — DOM snapshot | Verify `getByRole('heading')` still resolves | Automated — className changes don't break role selectors |
|
||||
| Select element renders with bg/text classes | Unit — className assertion | Manual code review | jsdom doesn't resolve CSS; visual verify in browser |
|
||||
| DeploymentStep labels have text classes | Unit — DOM structure check | `npm test -- src/components/wizard/DeploymentStep` | Automated — existing tests cover structure |
|
||||
| Dark mode visual correctness | Visual | Browser manual test | Cannot be automated; jsdom CSS limitation |
|
||||
|
||||
### Sampling Rate
|
||||
|
||||
- **Per task commit:** `npm test` (full suite, ~159 tests, fast in node env)
|
||||
- **Per wave merge:** `npm test`
|
||||
- **Phase gate:** Full suite green + visual browser verification in dark mode before `/gsd:verify-work`
|
||||
|
||||
### Wave 0 Gaps
|
||||
|
||||
None — existing test infrastructure covers all automated assertions. Dark mode correctness is visual-only and requires browser review.
|
||||
|
||||
*(Existing tests will catch any regressions in component rendering; they do not need modification for className additions to headings or form controls.)*
|
||||
|
||||
## Sources
|
||||
|
||||
### Primary (HIGH confidence — direct source code audit)
|
||||
|
||||
- `src/index.css` — complete token inventory, `.dark` overrides confirmed correct
|
||||
- `src/components/wizard/BackendSelectionStep.tsx` — bare `<h2>` confirmed at line 65
|
||||
- `src/components/wizard/RemoteConfigStep.tsx` — bare `<h2>` confirmed at line 65
|
||||
- `src/components/wizard/DeploymentStep.tsx` — completely unstyled native controls confirmed
|
||||
- `src/components/wizard/ReviewStep.tsx` — bare `<h2>` at line 75, unstyled checkbox at line 93
|
||||
- `src/components/ui/FieldRenderer.tsx` — `<select>` missing `bg-*` and `text-*` at lines 67–78
|
||||
- `src/App.tsx` — correct `<h2>` pattern with `text-on-surface` at line 19 (reference)
|
||||
- `.planning/REQUIREMENTS.md` — "Custom checkbox/radio styling: Out of scope"
|
||||
- `.planning/STATE.md` — "131 test selectors could break during component restyling"
|
||||
|
||||
### Secondary (MEDIUM confidence)
|
||||
|
||||
- MDN CSS `accent-color` property — native control tinting via CSS; Tailwind `accent-{color}` utility class
|
||||
- Tailwind v4 `@theme` documentation — confirms `accent-primary` should be generated from `--color-primary`
|
||||
|
||||
## Metadata
|
||||
|
||||
**Confidence breakdown:**
|
||||
- Issue identification: HIGH — direct source code audit, not inference
|
||||
- Fix patterns: HIGH — all use existing project-established patterns from App.tsx and other components
|
||||
- Tailwind v4 `accent-primary` availability: MEDIUM — needs verification at plan time
|
||||
- Test impact: HIGH — className additions to existing elements don't break `getByRole`/`getByText` selectors
|
||||
|
||||
**Research date:** 2026-04-01
|
||||
**Valid until:** Indefinite (stable codebase, no external dependencies changing)
|
||||
Reference in New Issue
Block a user