#64 — Tech Debt: Selectable-Grid Picker Pattern Duplicated Across 3 Dialogs #64

New issue

Closed

opened 2026-08-18 13:13:24 +02:00 by lena · 1 comment

lena commented

2026-08-18 13:13:24 +02:00

(Migrated from git.butzei.de)

Tech Debt: Selectable-Grid Picker Pattern Duplicated Across 3 Dialogs

Reported by: Frontend Engineer, spotted during #24's code review, 2026-07-22

Symptom

The same "grid of selectable buttons" idiom — map over values, render a button with
cn(...)-composed classes, a ring-2 ring-foreground (or ring-offset) highlight when selected,
role="group" on the wrapper, aria-pressed per button — is now hand-implemented independently in:

ReactUi/src/components/ListAppearanceDialog.tsx (colour grid, icon grid — two copies already)
ReactUi/src/components/LabelPicker.tsx (colour grid)
ReactUi/src/components/CreateListDialog.tsx (template grid, added in #24)

Each copy has near-identical Tailwind class strings and identical interaction logic
(select-on-click, disabled-while-saving, selected-state styling), but no shared component. A future
style or accessibility fix (e.g. changing the selected-state ring colour, adding keyboard
arrow-key navigation) has to be found and applied in four places across three files; it's easy to
update some and miss others, producing inconsistent selection UI across dialogs.

Expected behaviour

A single shared component (e.g. ReactUi/src/components/ui/option-grid.tsx, name TBD by whoever
picks this up) parameterized by items, value, onChange, disabled, a renderLabel/renderItem
callback, and the grid layout classes — used by all four existing call sites (colour, icon, label
colour, template) with no visual or behavioral change.

Acceptance criteria

One shared component replaces all four hand-rolled picker grids
No visual regression — same classes/behaviour at each of the four call sites
Existing component tests for ListAppearanceDialog, LabelPicker, and CreateListDialog
continue to pass unmodified (or with only trivial selector updates)
Future picker grids (if any) use the shared component from day one

Notes for whoever picks this up

Purely a maintenance/consistency cleanup — no functional bug today. Low priority; bundle it with
other frontend tech-debt work rather than picking it up standalone unless a related story touches
one of these dialogs anyway.

Agents involved

Frontend Engineer — owns the extraction and the four call-site migrations

Blockers

None.

# Tech Debt: Selectable-Grid Picker Pattern Duplicated Across 3 Dialogs **Reported by:** Frontend Engineer, spotted during `#24`'s code review, 2026-07-22 ## Symptom The same "grid of selectable buttons" idiom — map over values, render a `button` with `cn(...)`-composed classes, a `ring-2 ring-foreground` (or `ring-offset`) highlight when selected, `role="group"` on the wrapper, `aria-pressed` per button — is now hand-implemented independently in: - `ReactUi/src/components/ListAppearanceDialog.tsx` (colour grid, icon grid — two copies already) - `ReactUi/src/components/LabelPicker.tsx` (colour grid) - `ReactUi/src/components/CreateListDialog.tsx` (template grid, added in `#24`) Each copy has near-identical Tailwind class strings and identical interaction logic (select-on-click, disabled-while-saving, selected-state styling), but no shared component. A future style or accessibility fix (e.g. changing the selected-state ring colour, adding keyboard arrow-key navigation) has to be found and applied in four places across three files; it's easy to update some and miss others, producing inconsistent selection UI across dialogs. ## Expected behaviour A single shared component (e.g. `ReactUi/src/components/ui/option-grid.tsx`, name TBD by whoever picks this up) parameterized by `items`, `value`, `onChange`, `disabled`, a `renderLabel`/`renderItem` callback, and the grid layout classes — used by all four existing call sites (colour, icon, label colour, template) with no visual or behavioral change. ## Acceptance criteria - [ ] One shared component replaces all four hand-rolled picker grids - [ ] No visual regression — same classes/behaviour at each of the four call sites - [ ] Existing component tests for `ListAppearanceDialog`, `LabelPicker`, and `CreateListDialog` continue to pass unmodified (or with only trivial selector updates) - [ ] Future picker grids (if any) use the shared component from day one ## Notes for whoever picks this up Purely a maintenance/consistency cleanup — no functional bug today. Low priority; bundle it with other frontend tech-debt work rather than picking it up standalone unless a related story touches one of these dialogs anyway. ## Agents involved - **Frontend Engineer** — owns the extraction and the four call-site migrations ## Blockers None.

lena commented

2026-08-18 13:13:25 +02:00

(Migrated from git.butzei.de)

design (64_shared_picker_grid_component_design.md)

`#64` — Design Note

Component shape

ReactUi/src/components/ui/option-grid.tsx's OptionGrid<T> factors out only the genuinely
shared, behavioral part of the four (now five, counting #28's EmailNotificationDialog, added
this same session) hand-rolled picker grids: the role="group" wrapper, one <button> per item,
aria-pressed/onClick/disabled wiring, and key. Per-item visual differences (circular
swatch vs. icon square vs. full-width text row) stay with each caller via itemClassName and
renderItem — folding those into the shared component too would have made it a much larger,
more special-cased abstraction for comparatively little further deduplication, since the actual
visual shapes genuinely differ across the five call sites.

item === value (reference/value equality) is the selection check — safe here because every
call site's items are plain string literal unions (TodoListColor, TodoListIcon, LabelColor,
TodoListTemplate, EmailNotificationMode), never objects.

Migrated call sites

ListAppearanceDialog.tsx (colour + icon grids), LabelPicker.tsx (colour grid),
CreateListDialog.tsx (template grid), EmailNotificationDialog.tsx (mode grid) — four files,
five grids total (matching the reuse-angle review finding from #28's own code review, which
flagged EmailNotificationDialog as a fifth undedup'd instance).

Behavioral alignment: `EmailNotificationDialog` conformed to the majority convention

Four of the five existing grids used toggle-button semantics (role="group" + aria-pressed),
not a true ARIA radiogroup/radio — even though all five are genuinely single-select,
mutually-exclusive choices, which radiogroup describes more precisely. EmailNotificationDialog
(written this same session, for #28) used radiogroup/radio instead, since it was designed in
isolation before this cleanup ticket. Rather than have OptionGrid support two different ARIA
patterns (or migrate the other four to radiogroup, a larger behavioral change this ticket's own
AC explicitly rules out — "No visual regression... same classes/behaviour at each of the four call
sites"), conformed EmailNotificationDialog to the established majority convention instead, since
it's the one component here safe to adjust (written this session, no accumulated external
dependents). Updated its own test file and one assertion in TodoListHeader.test.tsx accordingly
— both were the only two files needing a selector change to keep passing, matching AC's "trivial
selector updates" allowance.

Verification

All 458 pre-existing frontend tests continue to pass unmodified except the two files above.
ReactUi/src/components/ui/option-grid.test.tsx is new, testing the shared component directly
(group role, per-item aria-pressed, onChange wiring, disabled propagation, optional
getItemAriaLabel/getTitle).

**design** (`64_shared_picker_grid_component_design.md`) # `#64` — Design Note ## Component shape `ReactUi/src/components/ui/option-grid.tsx`'s `OptionGrid<T>` factors out only the genuinely shared, behavioral part of the four (now five, counting `#28`'s `EmailNotificationDialog`, added this same session) hand-rolled picker grids: the `role="group"` wrapper, one `<button>` per item, `aria-pressed`/`onClick`/`disabled` wiring, and `key`. Per-item visual differences (circular swatch vs. icon square vs. full-width text row) stay with each caller via `itemClassName` and `renderItem` — folding those into the shared component too would have made it a much larger, more special-cased abstraction for comparatively little further deduplication, since the actual visual shapes genuinely differ across the five call sites. `item === value` (reference/value equality) is the selection check — safe here because every call site's items are plain string literal unions (`TodoListColor`, `TodoListIcon`, `LabelColor`, `TodoListTemplate`, `EmailNotificationMode`), never objects. ## Migrated call sites `ListAppearanceDialog.tsx` (colour + icon grids), `LabelPicker.tsx` (colour grid), `CreateListDialog.tsx` (template grid), `EmailNotificationDialog.tsx` (mode grid) — four files, five grids total (matching the reuse-angle review finding from `#28`'s own code review, which flagged `EmailNotificationDialog` as a *fifth* undedup'd instance). ## Behavioral alignment: `EmailNotificationDialog` conformed to the majority convention Four of the five existing grids used toggle-button semantics (`role="group"` + `aria-pressed`), not a true ARIA `radiogroup`/`radio` — even though all five are genuinely single-select, mutually-exclusive choices, which `radiogroup` describes more precisely. `EmailNotificationDialog` (written this same session, for `#28`) used `radiogroup`/`radio` instead, since it was designed in isolation before this cleanup ticket. Rather than have `OptionGrid` support two different ARIA patterns (or migrate the other four to `radiogroup`, a larger behavioral change this ticket's own AC explicitly rules out — "No visual regression... same classes/behaviour at each of the four call sites"), conformed `EmailNotificationDialog` to the established majority convention instead, since it's the one component here safe to adjust (written this session, no accumulated external dependents). Updated its own test file and one assertion in `TodoListHeader.test.tsx` accordingly — both were the only two files needing a selector change to keep passing, matching AC's "trivial selector updates" allowance. ## Verification All 458 pre-existing frontend tests continue to pass unmodified except the two files above. `ReactUi/src/components/ui/option-grid.test.tsx` is new, testing the shared component directly (group role, per-item `aria-pressed`, `onChange` wiring, `disabled` propagation, optional `getItemAriaLabel`/`getTitle`).