feat(web): note-array settings UI — presets, custom W×H, token, auto-detect [#1178]#1229
Conversation
…detect [#1178] Replace the per-board flagship/note pills with a grouped Radix Select (Devices: Flagship/Note; Note arrays: 5 presets + Custom…). Add Custom W×H inputs (validated 1..MAX_NOTES_PER_AXIS), a masked note_array_token field gated by isNoteArray, and an "Auto-detect from board" button that calls the detect-size endpoint and populates type/dims (matching presets by dimensions, not the matched_preset label). Adds note_array_token to BoardInstance, DetectBoardSizeResponse, and api.detectBoardSize(). New i18n keys mirrored across all 14 locales. New vitest suite + MSW detect-size handler; migrated the multi-board E2E to drive the Select. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@claude please review for correctness, UX, and a11y. Focus: (1) the synthetic Select value model ( |
|
Claude finished @jeffredodd's task in 4m 35s —— View job Code Review — Note Array Settings UITasks
1. Select value model ↔ updateBoard mapping ✅The synthetic value model is clean and correct.
Minor: When switching from note_array to "flagship" or "note", 2. Custom W×H validation — correctness issue
|
| # | Area | Severity | Status |
|---|---|---|---|
| 1 | Stale notes_wide/notes_tall sent on flagship/note switch |
Low | Advisory |
| 2 | Controlled number inputs snap back on invalid keystroke | Medium | Fix needed |
| 3 | Token <label> not associated with <input> (pre-existing) |
Low | Pre-existing, consistent with cloud_key |
| 4 | Stale dimError re-shows after Custom→Preset→Custom cycle |
Low | Fix recommended |
| 5 | <Loader2> missing aria-hidden |
Low | Fix recommended |
| 6 | Decorative × missing aria-hidden |
Low | Fix recommended |
The most impactful fix is #2 (controlled inputs). The value model, preset matching, masked round-trip, auto-detect error surfacing, and Select a11y structure are all well-implemented.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: small a11y fixes needed — two WCAG issues in the new note_array_token block; both are minor to fix and follow pre-existing patterns already in this file.
Finding 1 — Icon-only eye toggle has no accessible name (WCAG 4.1.2)
web/src/components/settings/display-settings.tsx:326
The new show/hide button for the Cloud API Token renders only <Eye> or <EyeOff> with no label text. Screen readers will announce it as an unnamed button.
<Button
type="button"
variant="outline"
size="sm"
onClick={() => setShowSecrets((prev) => ({ ...prev, note_array_token: !prev.note_array_token }))}
className="h-8 w-8 p-0"
disabled={board.note_array_token === "***"}
>
{showSecrets.note_array_token ? <EyeOff className="h-3.5 w-3.5" /> : <Eye className="h-3.5 w-3.5" />}
</Button>Suggested fix — add a sr-only span or an aria-label that tracks the toggle state:
<Button
type="button"
variant="outline"
size="sm"
onClick={() => setShowSecrets((prev) => ({ ...prev, note_array_token: !prev.note_array_token }))}
className="h-8 w-8 p-0"
disabled={board.note_array_token === "***"}
aria-label={showSecrets.note_array_token ? t("hideTokenAriaLabel") : t("showTokenAriaLabel")}
>
{showSecrets.note_array_token ? <EyeOff className="h-3.5 w-3.5" /> : <Eye className="h-3.5 w-3.5" />}
</Button>
(Requires adding hideTokenAriaLabel / showTokenAriaLabel keys to all locale files, or you could reuse the sr-only span approach if those keys are already present for the sibling local/cloud-key buttons.)
Note: The pre-existing eye buttons for
local_api_key(line 206) andcloud_key(line 294) have the same gap. This PR doesn't introduce the pattern, but it does copy it into new code.
Finding 2 — <label> not programmatically associated with the token <input> (WCAG 1.3.1 / 4.1.2)
web/src/components/settings/display-settings.tsx:304
The <label> at line 304 is a sibling of the <input> (inside a different <div>), with no htmlFor / id link. AT won't announce the label text when the user focuses the field, and clicking the label won't focus the input.
<label className="text-xs font-medium">{t("noteArrayTokenLabel")}</label>
<div className="flex gap-1.5">
<input ... /> {/* ← no id; label has no htmlFor */}Simplest fix — add a stable id to the input and a matching htmlFor on the label (or wrap the input inside the label, as is done for the custom W×H number inputs at lines 681/693):
<label htmlFor={`note-array-token-${board.id}`} className="text-xs font-medium">
{t("noteArrayTokenLabel")}
</label>
<div className="flex gap-1.5">
<input
id={`note-array-token-${board.id}`}
type={showSecrets.note_array_token ? "text" : "password"}
...
/>
Note: The same unassociated-label pattern exists for
boardHostLabel(line 136),localApiKeyLabel(line 182),enablementTokenLabel(line 226), andcloudKeyLabel(line 270) — also pre-existing.
Finding 3 — Dynamic error messages not in a live region (WCAG 4.1.3, minor)
web/src/components/settings/display-settings.tsx — dimError and detectError blocks
Both inline validation messages appear and disappear without being in an aria-live region. Screen readers won't announce them automatically.
dimError[board.id]renders into a plain<p>(~line 705 in the PR head).detectError[board.id]renders into a plain<div>(~line 719).
Adding role="alert" (or aria-live="polite") to these containers is sufficient.
For detectError, the <AlertCircle> icon inside the error block is decorative (the adjacent text carries the message) and should also get aria-hidden="true" to avoid a redundant announcement.
No issues found
- Select widget (device-type picker):
SelectTriggerhas a localizedaria-label={t("deviceTypeAriaLabel")}✓ - Color swatches:
aria-label+aria-pressedon both buttons ✓ - Custom W×H number inputs: wrapping
<label>pattern used correctly ✓ - Auto-detect button: visible text "Auto-detect from board" / "Detecting…" provides the accessible name ✓
- i18n coverage: all new user-facing strings (including
deviceTypeAriaLabel) are in every locale file ✓
1 participant
Closes #1178. Part of the Note Arrays epic #1167. Targets
next. Blocks #1180, #1182.What
The board-settings UI to configure a note array: pick a config, enter the Cloud API token, and auto-detect from the board.
<Select>(Devices: Flagship/Note · Note arrays: the 5 presets + "Custom…"). A synthetic value (flagship/note/preset:<id>/custom) maps toupdateBoard({ device_type, notes_wide?, notes_tall? }). Presets are matched by(notes_wide, notes_tall)— never thematched_presetlabel (which is localized text).1..MAX_NOTES_PER_AXIS); invalid values are blocked with an inline error and never persisted.note_array_tokeninput (clones thecloud_keyshow/hide + "(set)" + masked-round-trip pattern), shown only for note-array configs.POST /settings/board/{id}/detect-size(Note arrays: auto-detect board type & size (flagship/note/array) #1172); on success sets device type + dims (preset or Custom); 404/400/422detailshown inline. Works for any board type.note_array_tokenadded to the TSBoardInstance; newDetectBoardSizeResponse+api.detectBoardSize()wrapper. 15 newdisplaySettingsi18n keys across all 14 locales (parity test passes).Tests
web/src/__tests__/display-settings.test.tsx(new, 14): grouped options, preset save, flagship-from-array, Custom validation/blocking, token visibility + masked round-trip, auto-detect (array/flagship/custom success + 422/404/400 inline errors).web/tests/multi-board.spec.ts: migrated the existing "change device type via type pills" E2E to drive the new Select; added CI-only preset+token-persist-across-reload and route-mocked auto-detect→"2×2 grid". (Add-Board picker tests unchanged.)Verification (throwaway
node:26-alpine,npm ci --legacy-peer-deps)No backend changes (token field, detect endpoint, masking, presets all already merged).
🤖 Generated with Claude Code