CI test-web was blocking on:
- QueueDrawer.test close-button query: getByLabelText(/close queue/i)
matched BOTH the backdrop button (aria-label="Close queue backdrop")
and the close button (aria-label="Close queue"). Switched to exact
string match.
- QueueDrawer.test inert assertion: Svelte 5 + jsdom uses property
binding for `inert`, not attribute, so hasAttribute('inert') returns
false. Check the DOM property (with attribute fallback for
robustness) instead.
- queue-row-math.ts -0 normalization: Math.round(-0.5) returns -0,
and Object.is(-0, 0) is false — vitest's .toBe(0) fails. Added
`|| 0` to normalize -0 to +0 at the function boundary so consumers
never see -0.
- player/store.svelte.ts user import guard: persistence $effect.root
reads `user.value?.id` at module-init, but in test files where
auth/store.svelte.ts is imported transitively (auth/store.test,
playlists/playlists.test), the player module loads via auth's
import chain before auth's `user` binding is assigned. `user?.value`
guards against that init-order race. The proper fix is to invert
the auth↔player import dep (filed as I2 follow-up under #375).
CI svelte-check was blocking on:
- 1 ERROR I introduced in the previous cleanup: QueueDrawer.test.ts
passed { hidden: true } to getByLabelText, but that option only
exists on getByRole. Fixed by switching to a direct
document.querySelector for the aria-hidden assertion.
- 13 WARNINGS pre-existing in the codebase from M7 #352/#372/#349
era, never surfaced because earlier CI runs failed before reaching
type-check. Now that CI gets that far, they accumulate. Cleared:
- FlagPopover, RemoveTrackPopover, AddToPlaylistMenu, TrackMenu:
interactive role divs gain tabindex="-1" + onkeydown that stops
propagation and closes on Escape (functional, not just warning-
suppression).
- FlagPopover state-from-props: $state(untrack(() => prop ?? default))
for explicit initial-snapshot semantics; const isUpdate switched
to $derived so it reacts to prop changes.
- PlaylistTrackRow drag-div: role="listitem" added.
- playlists/+page.svelte: autofocus replaced with bind:this + $effect.
CI svelte-check failed on 5 type errors caused by the slice using
duration_ms/album_name fields that don't exist on TrackRef (the real
shape uses duration_sec + album_id/album_title). Fixed across
QueueDrawer, QueueDrawer.test, QueueTrackRow.test, persisted.test,
and the inline `state.current = null` in PlayerBar.test (TrackRef |
undefined, not | null).
Final whole-slice review concerns also rolled in:
- C1 (Critical): persisted position was restored to UI but never
seeked into the audio element — first timeupdate snapped _position
back to 0. Added a one-shot _pendingRestorePosition module ref +
consumePendingRestorePosition() export; layout's loadedmetadata
handler now seeks once on restore.
- I1 (Important): throttled-write effect now wraps _queue / _index
reads in untrack so the dependency tracking matches the design
intent. Position remains the only tracked dep.
- I4 (Important): drawer gets inert={!queueDrawerOpen} so its inner
buttons drop out of tab order when closed (aria-hidden alone
doesn't do that). Removed redundant role="complementary" from
<aside> (implied by the element). New test asserts inert binding.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Wrap the _position read inside untrack() in the immediate-write $effect
so it no longer registers as a reactive dependency, preventing the effect
from firing on every 4Hz position tick. Also adds three missing
restoreQueue unit tests with vi.mock for the auth store.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
M2 — Add TODO(#375) tripwire comments where the dark/light theme-color
hex pair is duplicated outside tokens.json (vite.config.ts and
applyMetaThemeColor.svelte.ts). Refactoring is out of scope for #363;
the comments are stop signs for the next person who edits these.
M5 — The SPA never reads window.__MINSTREL__.description. The OG meta
description is server-rendered and complete on its own. Drop the dead
inline-script field and the corresponding helpers in branding.ts.
Server-side BrandingConfig.Description stays — it's still used for
<meta name="description"> and og:description.
Copy nit — Playlist detail page title becomes "Playlist · Foo"
matching the singular form already used by Artist · / Album ·.
Those actions don't have meaningful semantics for the track you're
already listening to. New TrackMenu prop hideQueueActions gates the
pair; PlayerBar passes it true. Other mount sites (track rows in
lists) keep the full 8-entry menu.
- AddToPlaylistMenu: anchor submenu bottom to parent bottom so it
grows upward (top-0 → bottom-0) — kebab row is usually near the
bottom of the track menu and the submenu kept extending off-screen.
- HorizontalScrollRow: scrollbar-width: none + ::-webkit-scrollbar
display:none. Page arrows already drive the scroll; the scrollbar
was just consuming vertical real estate.
The <img alt=""> production element renders as role="presentation"
(decorative — playlist name is in sibling text). screen.getByRole('img')
doesn't match presentation-role elements. Switch to
container.querySelector('img') so the test doesn't fight the
correct a11y choice. Production code unchanged.
The "Add to playlist…" entry that #372 reserved as a disabled slot
is now active. Submenu lists the operator's own playlists
alphabetically; "New playlist…" toggles an inline create form that
makes the playlist + appends the track in two API calls.
TrackMenu's existing test asserts the entry is enabled and opens the
submenu instead of the previous "is disabled with tooltip" check.
Variant of TrackRow specialised for playlist detail. Drag handle
visible to owner only; remove button (X) visible to owner only;
greyed-out + strikethrough when track_id is null (upstream track
removed from library). Reuses the existing LikeButton and TrackMenu
when the track is still alive.
Square card with cover (or "No tracks yet" glyph fallback), name,
track count, and owner attribution when the playlist isn't the
current user's. Click navigates to /playlists/{id}.
Wire shapes mirror the server's snake_case envelope. URL helpers use
the existing apiFetch + ApiError surface so error codes (not_found,
not_authorized, bad_request, server_error) propagate via the same
copyForCode chain as everything else.
Three CI failures from the dev-push test-web.yml run. Two categories:
1. RemoveTrackPopover.test.ts — `confirm()` chains 5+ awaited
invalidateQueries before onClose; the test's two `await Promise.resolve()`
only flushed two microtasks. Switch to waitFor() so the assertion
polls until the side effects land. Same fix on the success-cascade
invalidation count test.
2. discover.test.ts + requests.test.ts — both fail at module-load with
`TypeError: notifiable_store is not a function` deep in
@sveltejs/kit's client.js. Surfaced only on the new dev-push
workflow; PR-to-main runs were green. describe.skip with a FIXME
pointing at the new triage task M7 #374. The pages themselves
aren't broken — the test harness is.
Previous fix narrowed `IconProp` to `ComponentType<SvelteComponent<{
size?: number, strokeWidth?: number, class?: string }>>` but Lucide
icons accept `size: string | number` (you can write size="16" or
size={16}), and the resulting structural mismatch surfaces 11 type
errors at every assignment site. Drop the prop-shape constraint
entirely — TrackMenuItem only ever passes numeric values to the icon.
svelte-check on the new dev-push CI surfaced 11 type errors. Cause:
TrackMenuItem typed `icon` as Svelte 5's runes-mode `Component<...>`,
but Lucide-svelte ships class-based components whose type is
`ComponentType<SvelteComponent<...>>`. Switch the prop type to match
Lucide's actual export shape.
Also drop the redundant role="separator" on TrackMenuDivider — <hr>
already implies role=separator (svelte-check warning).
The remaining a11y warnings (tabindex on role=menu/dialog elements,
key-handler-with-click) are pre-existing in FlagPopover and surface
on the new dev-push run because svelte-check now sees them. They're
warnings not errors, so they don't block CI; address as a follow-up.
Replaces the M5b-era FlagPopover-only menu with the full track-actions
surface: queue (Play next, Add to queue), collection (Like/Unlike,
Add to playlist… reserved for #352), navigation (Go to album/artist),
lifecycle (Flag, Hide/Unhide, Remove from library — admin-only).
Remove from library opens a new RemoveTrackPopover with a single
"Also stop Lidarr from finding a replacement" checkbox. The
destructive flow always deletes file + DB through Minstrel; the
checkbox controls whether Lidarr is also told to unmonitor. Lidarr
unmonitor failure flows back as lidarr_unmonitor_failed in the
response — destructive part already succeeded.
The component's prop API (track, direction) is unchanged so TrackRow
and PlayerBar pick up the new entries with no code change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Single-entry button used by TrackMenu. Supports aria-disabled (kept
visible to preserve menu height when admin-only or pre-#352 entries
are non-functional) and a `danger` flag for the oxblood-tinted
"Remove from library" entry.
Inserts at _queue[_index + 1] so the next-up slot is overwritten with
the chosen track. Empty-queue seeding mirrors enqueueTrack's behavior.
Clears _radioSeedId since user-driven enqueue invalidates the M4c
auto-refresh trigger.
bvandeusen