fix(651): review round 1 — never clear an unnamed id, complete the Lucene escaping, keyboard-operable picker
Cold cross-family review of 57aefcdf. Six findings, all web-only. F1 (HIGH, data-loss shaped): RerunCollectionsController.ProjectToResponseModel derives BOTH selectedId and selectedName from the same eager-loaded navigation, and GetRerunCollectionByIdHandler loads media metadata only for Show/Season/Artist/Movie while MediaCollections/Mapper maps RemoteStream through `_ => null`. So opening a RemoteStream rerun collection returned HTTP 200 with a null selection and the edit-load refresh CLEARED a stored id, leaving Save permanently disabled. The refresh now merges instead of replacing, so no path can clear an id it merely failed to name; the label degrades to `#id`. Covered per affected type — RemoteStream, Episode, MusicVideo, Song, OtherVideo, Image — plus a re-save assertion. The read-model gaps themselves are server-side and are NOT touched here. F2: `&` and `|` were missing from the escaped set, so `Rock && Roll` compiled with the boolean operator live. Pre-existing in Auto-Tune's original helper, but propagated to three more pickers — and now fixed for Auto-Tune too, since the helper is shared. The test that claimed to cover "every Lucene special" carried its own hand-copied sample and could not see its own omissions; it is now driven per-character off an exported LIBRARY_PICKER_LUCENE_SPECIALS. F3: a slow edit-load name resolution could relabel a newer selection. The label is now keyed to the id it was resolved for AND refuses to overwrite a label naming a different id — keying the render alone stops the mislabelling but discards the correct new label. F4: searchLibraryPickerOptions clamps pageSize instead of merely defaulting it. A bound a caller can exceed is not a bound. F6: replacing a native <select> with an input+listbox dropped keyboard operability. Full ARIA combobox pattern added — role/aria-expanded/aria-controls/aria-autocomplete, Arrow/Home/End over aria-activedescendant, Enter to commit, Escape to dismiss, options as non-tab-stops, cursor reset on each new result set. F7: both is-mounted tests were unsound. React 19 no longer warns on setState-after-unmount and an unmounted tree renders nothing either way, so the DOM assertion could not fail; the hook re-arm test used rerender rather than an effect cleanup. Now: a hook-module mock proving SearchPicker actually reads the guard and sees false, and a StrictMode double-invoke for the re-arm. Both verified by removing the mechanism and watching them fail. Same for the LCG divisor, which now has a direct boundary test. F5 (FillerPresets collection-family names) is filed as #670, not fixed here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1612,7 +1612,10 @@ body {
|
||||
font-size: 13px;
|
||||
}
|
||||
|
||||
/* `-active` is the keyboard virtual cursor (aria-activedescendant); it deliberately shares the
|
||||
hover treatment so pointer and keyboard users get the same affordance. */
|
||||
.ctv-picker-result:hover,
|
||||
.ctv-picker-result-active,
|
||||
.ctv-picker-result[aria-selected='true'] {
|
||||
background: var(--surface-3, rgba(255, 255, 255, 0.08));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user