fix(settings): stop offering a column the backend rejects #232

2026-08-30T08:42:41Z

logan commented

2026-08-30 08:42:41 +00:00

The issue

Settings → Track List Columns listed two rows both called "Track
Name", one enabled and one not, with no way to tell them apart — and
the accessible names collided too, so a screen reader heard "Show the
Track Name column" twice in one list.

What the open question turned out to be

#197's Direction offered two answers of different sizes: a second
string (configLabel) so the sort dropdown keeps "Track Name" while
the configurator says something else, or filtering the row out because
titleArtist is the phone's column and the user cannot select it.

The code settles it, and the honest answer is forced rather than
preferred. titleArtist is not in tracklist.AllColumnIDs, so:

validate = unknown track-list column ID: "titleArtist"
albumArt valid = true
titleArtist valid = false

Ticking that row sends a column set the backend rejects, config-page
swallows the rejection into console.error('Failed to update columns:', err), and the tick reverts on the next render. A configLabel would
have given a distinct name to a control that cannot work.

What changed

ColumnDef gains configurable?: boolean (default true), and
titleArtist is the one false — a definition is not the same
thing as a choice, and the fact lives next to the column it
describes.
ALL_COLUMN_IDS (Object.keys(COLUMN_DEFS)) becomes
CONFIGURABLE_COLUMN_IDS, which is what config-page lists. One
export, one consumer, an accurate name.
titleArtist's own comment claimed its label was user-visible in the
sort dropdown. It is not: that list is built from the configured
columns, which this one can never be. Corrected rather than left.
CLAUDE.md gains the pair that drifted, beside the
DefaultColumns / DEFAULT_COLUMN_IDS paragraph it belongs with.

Verification

tier	result
`make ui-test`	104 files, 1134 tests, green
`npx tsc --noEmit`	clean
`make skill-check`	47 targets, all present
screenshot, real app	18 rows, one "Track Name"
`make lint` / `make test` / `make generate` / `make bindings`	not run — no Go, no `.sql`, no `.templ`, no bound signature
`make e2e`	not run — nothing in `e2e/specs` counts these rows, and the tier that renders this component is the one above

frontend/test/components/settings-column-list.test.ts is new and
asserts four things: the rows are named once each, the checkboxes'
aria-labels are unique, every offered id is one Go's AllColumnIDs
lists, and the drawing table still has the phone's column. The Go list
is read out of backend/tracklist/config.go as text, because a
third copy of it is the fault one step earlier.

Planted, not read. With the filter removed, all four fail with the
defect's own numbers:

AssertionError: expected [ 'Track Name', 'Track Name' ] to have a length of 1 but got 2
AssertionError: expected 18 to be 19
AssertionError: expected [ 'titleArtist' ] to deeply equal []
AssertionError: expected [ 'albumArt', 'titleArtist', …(17) ] to not include 'titleArtist'

Both queries are scoped to [for^="column-"]: Settings' view-visibility
list (#25) draws with the same two classes, so a bare .column-label
sweeps 29 rows across two sections — and "Albums" the destination
sitting beside "Album" the column is not this fault.

Deliberately not done

Filed #231 instead: SetTrackListColumns assigns
c.TrackList.Columns = columns before validating, so a rejected list
stays in memory and Save() validates the whole config — after one
tick of the phantom row, no setting saves for the rest of the
session, silently:

later, unrelated SetThemeAccentColor = could not save config: invalid config: ... unknown track-list column ID: "titleArtist"

That is reachable from any invalid input — a duplicate id, a stale
frontend — rather than from this row, and every setter in
backend/config has the same read-modify-write-then-Save() shape, so
it wants one pass over the file rather than one fix here.

Closes #197

## The issue Settings → Track List Columns listed **two rows both called "Track Name"**, one enabled and one not, with no way to tell them apart — and the accessible names collided too, so a screen reader heard "Show the Track Name column" twice in one list. ## What the open question turned out to be #197's Direction offered two answers of different sizes: a second string (`configLabel`) so the sort dropdown keeps "Track Name" while the configurator says something else, or filtering the row out because `titleArtist` is the phone's column and the user cannot select it. **The code settles it, and the honest answer is forced rather than preferred.** `titleArtist` is not in `tracklist.AllColumnIDs`, so: ``` validate = unknown track-list column ID: "titleArtist" albumArt valid = true titleArtist valid = false ``` Ticking that row sends a column set the backend rejects, `config-page` swallows the rejection into `console.error('Failed to update columns:', err)`, and the tick reverts on the next render. A `configLabel` would have given a distinct name to a control that cannot work. ## What changed - `ColumnDef` gains `configurable?: boolean` (default true), and `titleArtist` is the one `false` — a *definition* is not the same thing as a *choice*, and the fact lives next to the column it describes. - `ALL_COLUMN_IDS` (`Object.keys(COLUMN_DEFS)`) becomes `CONFIGURABLE_COLUMN_IDS`, which is what `config-page` lists. One export, one consumer, an accurate name. - `titleArtist`'s own comment claimed its label was user-visible in the sort dropdown. It is not: that list is built from the *configured* columns, which this one can never be. Corrected rather than left. - `CLAUDE.md` gains the pair that drifted, beside the `DefaultColumns` / `DEFAULT_COLUMN_IDS` paragraph it belongs with. ## Verification | tier | result | |---|---| | `make ui-test` | **104 files, 1134 tests, green** | | `npx tsc --noEmit` | clean | | `make skill-check` | 47 targets, all present | | screenshot, real app | 18 rows, one "Track Name" | | `make lint` / `make test` / `make generate` / `make bindings` | not run — no Go, no `.sql`, no `.templ`, no bound signature | | `make e2e` | not run — nothing in `e2e/specs` counts these rows, and the tier that renders this component is the one above | `frontend/test/components/settings-column-list.test.ts` is new and asserts four things: the rows are named once each, the checkboxes' `aria-label`s are unique, every offered id is one Go's `AllColumnIDs` lists, and the drawing table still has the phone's column. The Go list is **read out of `backend/tracklist/config.go` as text**, because a third copy of it is the fault one step earlier. **Planted, not read.** With the filter removed, all four fail with the defect's own numbers: ``` AssertionError: expected [ 'Track Name', 'Track Name' ] to have a length of 1 but got 2 AssertionError: expected 18 to be 19 AssertionError: expected [ 'titleArtist' ] to deeply equal [] AssertionError: expected [ 'albumArt', 'titleArtist', …(17) ] to not include 'titleArtist' ``` Both queries are scoped to `[for^="column-"]`: Settings' view-visibility list (#25) draws with the same two classes, so a bare `.column-label` sweeps 29 rows across two sections — and "Albums" the destination sitting beside "Album" the column is not this fault. Looked at it on the running app (`make dev-headless`, port 34115 checked free first): `Track Name | Artist | Album | Duration | Art | Genre | Year | …`, eighteen rows, one "Track Name". Before the fix the duplicate sat sixth, between Art and Genre. ## Deliberately not done Filed **#231** instead: `SetTrackListColumns` assigns `c.TrackList.Columns = columns` *before* validating, so a rejected list stays in memory and `Save()` validates the whole config — after one tick of the phantom row, **no setting saves for the rest of the session**, silently: ``` later, unrelated SetThemeAccentColor = could not save config: invalid config: ... unknown track-list column ID: "titleArtist" ``` That is reachable from any invalid input — a duplicate id, a stale frontend — rather than from this row, and every setter in `backend/config` has the same read-modify-write-then-`Save()` shape, so it wants one pass over the file rather than one fix here. Closes #197

logan added 1 commit 2026-08-30 08:42:42 +00:00

fix(settings): stop offering a column the backend rejects

CI / check (push) Skipped

CI / e2e (push) Skipped

CI / check (pull_request) Successful in 2m50s

Details

CI / e2e (pull_request) Successful in 10m11s

Details

d225f922fb

Settings -> Track List Columns listed two rows both called "Track
Name", one of which could not be ticked, and a screen reader heard
"Show the Track Name column" twice with nothing to tell them apart.

They are `titleArtist` and `trackName`, and the issue left open which
way to fix it: a second label so the sort dropdown and the configurator
can say different words, or drop the row because the user cannot select
it. The code settles that. `titleArtist` is not in
`tracklist.AllColumnIDs`, so `Config.Validate()` returns `unknown
track-list column ID: "titleArtist"` -- ticking the row sends a column
set the backend rejects, `config-page` swallows the rejection into a
`console.error`, and the tick reverts. A second label would have named
a control that cannot work.

So a definition says whether it is a *choice*, and the configurator
reads `CONFIGURABLE_COLUMN_IDS` rather than `Object.keys(COLUMN_DEFS)`.

The new test reads Go's own list out of `backend/tracklist/config.go`
rather than writing it down a third time, since a third copy is the
fault one step earlier. Verified by planting: with the filter removed
all four assertions fail with the defect's own numbers.

Not fixed here, and filed as #231: `SetTrackListColumns` assigns before
it validates, so a rejected list stays in memory and `Save()` validates
the whole config -- one tick and no setting saves for the rest of the
session. That is reachable from any invalid input, not from this row.

Closes #197

logan commented

2026-08-30 09:02:08 +00:00

CI is green on both jobs, first attempt.

check — run 22713, success (make lint, make test x3 configurations, tsc --noEmit, make ui-test, make bindings-check, make skill-check, make commit-check)
e2e — run 22715, success (Chromium and WebKit)

Not merging; leaving it for review.

CI is green on both jobs, first attempt. - `check` — run 22713, success (`make lint`, `make test` x3 configurations, `tsc --noEmit`, `make ui-test`, `make bindings-check`, `make skill-check`, `make commit-check`) - `e2e` — run 22715, success (Chromium and WebKit) Not merging; leaving it for review.

logan referenced this pull request

2026-08-30 09:02:08 +00:00

Settings lists two columns both called "Track Name", and one of them may be unselectable #197

logan referenced this pull request

2026-08-30 09:41:41 +00:00

fix(config): put the old value back when a setter is rejected #233

All checks were successful

CI / check (push) Skipped

Required

CI / e2e (push) Skipped

Required

CI / check (pull_request) Successful in 2m50s