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:
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.
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 = columnsbefore 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.
## 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
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
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.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
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" whilethe configurator says something else, or filtering the row out because
titleArtistis the phone's column and the user cannot select it.The code settles it, and the honest answer is forced rather than
preferred.
titleArtistis not intracklist.AllColumnIDs, so:Ticking that row sends a column set the backend rejects,
config-pageswallows the rejection into
console.error('Failed to update columns:', err), and the tick reverts on the next render. AconfigLabelwouldhave given a distinct name to a control that cannot work.
What changed
ColumnDefgainsconfigurable?: boolean(default true), andtitleArtistis the onefalse— a definition is not the samething as a choice, and the fact lives next to the column it
describes.
ALL_COLUMN_IDS(Object.keys(COLUMN_DEFS)) becomesCONFIGURABLE_COLUMN_IDS, which is whatconfig-pagelists. Oneexport, one consumer, an accurate name.
titleArtist's own comment claimed its label was user-visible in thesort dropdown. It is not: that list is built from the configured
columns, which this one can never be. Corrected rather than left.
CLAUDE.mdgains the pair that drifted, beside theDefaultColumns/DEFAULT_COLUMN_IDSparagraph it belongs with.Verification
make ui-testnpx tsc --noEmitmake skill-checkmake lint/make test/make generate/make bindings.sql, no.templ, no bound signaturemake e2ee2e/specscounts these rows, and the tier that renders this component is the one abovefrontend/test/components/settings-column-list.test.tsis new andasserts four things: the rows are named once each, the checkboxes'
aria-labels are unique, every offered id is one Go'sAllColumnIDslists, and the drawing table still has the phone's column. The Go list
is read out of
backend/tracklist/config.goas text, because athird 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:
Both queries are scoped to
[for^="column-"]: Settings' view-visibilitylist (#25) draws with the same two classes, so a bare
.column-labelsweeps 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 34115checked free first):
Track Name | Artist | Album | Duration | Art | Genre | Year | …, eighteen rows, one "Track Name". Before the fix theduplicate sat sixth, between Art and Genre.
Deliberately not done
Filed #231 instead:
SetTrackListColumnsassignsc.TrackList.Columns = columnsbefore validating, so a rejected liststays in memory and
Save()validates the whole config — after onetick of the phantom row, no setting saves for the rest of the
session, silently:
That is reachable from any invalid input — a duplicate id, a stale
frontend — rather than from this row, and every setter in
backend/confighas the same read-modify-write-then-Save()shape, soit wants one pass over the file rather than one fix here.
Closes #197
CI is green on both jobs, first attempt.
check— run 22713, success (make lint,make testx3 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.