The 'usage' query rendered both its loading and its error state as 'No usage' (the same false-empty
R3 targets), while the sibling 'status' query got a proper state. Split the fallback into
error(+retry) / loading / empty.
- notifyYouTubeActionError: on any non-403 error, show the server's own reason (err.detail, e.g.
"Not enough quota left today") instead of the caller's generic fallback, which was discarded
(U-3.2.8). 403 still offers the Connect affordance.
- App boot error now offers a Retry (StateMessage) instead of a dead "Something went wrong" (U-4.2).
- deletePlaylist prefers the server's reason over a blanket "couldn't delete on YouTube" that
misleads when the failure wasn't the YouTube side.
- Already covered in earlier sprints (verified): the live-search "Load more" already surfaces the
quota error inline via err.detail, and ChannelPage subscribe/unsubscribe already route through
notifyYouTubeActionError.
Replaces the plain "Loading…" text in every R3 loading branch with a single shared LoadingState
(a spinning lucide Loader2 in the accent colour + label, centered, role=status/aria-live) — one
component so all loads read the same and can be swapped for skeletons later in one place. Applied
across Feed, Channels, ChannelDiscovery, AuditLog, NotificationsPanel, Messages, Scheduler, Stats,
PlexBrowse (grid + subviews), PlexPlaylistView, ConfigPanel, DownloadCenter, AdminUsers, ChatThread.
Left the small "load more" footer and lazy Suspense fallbacks as-is.
- Blank-flash (showed an empty state on first load before data): DownloadCenter (queue + library
tabs), AdminUsers (roles + invites), ChatThread, PlaylistsRail now show a loading indicator
(and an error+retry) instead of a false-empty flash — guarded on `&& !data` so live data isn't
disturbed on refetch.
- Forever-loading (`isLoading || !data` loops forever on a failed fetch): ConfigPanel, the three
PlexBrowse subviews (playlist/show/season), and Stats' admin dashboard get an `isError && !data`
error+retry path before the loading branch.
- Playlists' detail pane already had an error path (isError+retry); its list lives in PlaylistsRail
(now covered), so no change needed there.
- The honest-state branch now guards on `isError && !data`, so a background/window-focus refetch
failure keeps already-visible content instead of replacing the list with a full error screen
(regression the bare `isError` introduced). Applied to AuditLog, Channels, ChannelDiscovery,
PlexBrowse, PlexPlaylistView, Messages(people), Feed — matching the guard already used in
Messages(conversations)/NotificationsPanel/Scheduler.
- Drop the unused DataTable loading/error/onRetry props (every table page uses the outer
StateMessage branch, not the props) — reverts DataTable to its simple empty block.
- StateMessage announces errors with role=alert/aria-live=assertive (was polite for all tones).
- Remove the now-unused common.empty string (en+hu), left over from the removed QueryState render-prop.
The error state didn't exist as a concept: a failed main query rendered as an "empty" list
("No channels" while the request actually died), so a backend restart read as "everything is
empty". Add the convention that an empty state may NEVER show while a query is erroring.
- New components/QueryState.tsx: shared StateMessage (loading / error+Retry, house style, with
role=status/aria-live — the app had zero live regions).
- DataTable gains loading/error/onRetry props (suppresses empty-text on error).
- isError branch (before the empty branch) added across the false-empty pages: Feed (retry),
Channels, ChannelDiscovery, AuditLog, NotificationsPanel, Messages (conversations + people),
Scheduler, Stats, PlexBrowse grid, PlexPlaylistView. ChannelPage's main content is Feed, now
covered. common.loadError/empty strings (en+hu).
Follow-up to user prod testing of 0.51.0:
- re-download now clears job.display_name so the card title + Content-Disposition filename are
re-derived from the freshly-cleaned title (previously an auto-name from an older un-cleaned
title — still carrying a "… views · … reactions |" prefix — survived the rebuild).
- on-disk AND served names are now a strict shell-safe slug: only [A-Za-z0-9._-] survive; every
other char (the "!" the user saw, plus ( ) [ ] & $ ' " ; | ? * …) collapses to "_". The id no
longer uses [brackets] (glob metacharacters). Shared _slug helper for sanitize + display_filename.
- GC gains a 5th pass: sweep on-disk files/dirs no ready asset owns (old accented channel dirs, a
poster from a superseded naming scheme left after a re-download), skipping the dot system trees
and anything newer than an hour so an in-flight download is never touched.
- Tests: slug strips !/$/()[]; rel_path is bracket-free; orphan-sweep keeps owned/system/fresh files.
- redownload now ALWAYS rebuilds into a fresh private asset instead of ever reusing a shared
cached file: the cache sig is salted with job_id + current asset_id, so the fork is distinct
from both the dedup pool and the job's own current asset. This resolves the co-held silent
no-op (finding #1) — a shared asset is left intact for its other holders while THIS job
re-fetches its own copy. Resolving the fork BEFORE releasing the old hold (and the unique
salted key) also removes the IntegrityError-rollback-after-file-delete window (finding #2).
- format_sig gains an optional `salt` for that fork identity.
- titles: don't consume the space after `|`, so a "stats | #hashtags" remainder keeps the
leading whitespace _TRAILING_HASHTAGS needs — and fall back to the prefix+hashtag-stripped
text, so neither the stats nor the SEO hashtags are resurrected when a title empties (#3).
- worker: early-fill asset.title falls back to the video id so a title that cleaned to empty
doesn't show a blank card while downloading (#5).
- Tests for the salt fork and the hashtag-remainder case.
(Finding #4 — quota on re-download — intentionally not changed: a re-download reuses the
existing job and re-fetches ~the same bytes, so calling check_enqueue would wrongly reject on
the job-count cap while footprint is unchanged.)
- redownload: never clobber a co-held (deduped) asset. Drop the force-reset-to-pending;
_release_asset already deletes+recreates a solo asset (genuine re-fetch), while a shared
ready asset is left intact and the re-queued job re-points to it via _finish_from_cache —
so one user's re-download can't delete another's file mid-serve (finding #1).
- redownload: reset queue_pos to the tail so a re-download doesn't front-run newer queued
jobs on the worker's `ORDER BY queue_pos` claim (finding #3). Made service.next_queue_pos public.
- formats: video-only mp4 selector prefers every video-only option before any progressive
`best`, so a "Video only" download stays audio-free whenever a video-only stream exists (#2).
- titles: a title that is only the engagement prefix no longer resurrects the stripped stats
(return the stripped text / empty, not the raw) (#4).
- worker: probe the file's duration when yt-dlp gave none, so a long re-encode shows real
progress instead of a frozen 0% (#5); tighten the ensure_browser_playable docstring to its
actual mp4-profile scope (#6).
Mini-epic "Downloads correctness & UX" (S1–S3), on top of the iOS-AV1 + 720p fixes.
S1 — naming & titles:
- ASCII-fold on-disk dir/file names AND the served (Content-Disposition) filename: no
spaces (underscore-joined), no accents (á→a, ő→o, ß→ss), pure ASCII — portable and free
of the `?` mojibake accented names show in non-UTF-8 tools. (Forward-only; existing files
clean up via re-download.)
- Strip a leading social engagement prefix ("1.6M views · 66K reactions | …") from titles.
S2 — universal browser playability (generalizes the iOS re-encode):
- ensure_browser_playable guarantees the stored file is H.264 + AAC/MP3 in mp4 — the only
combo every browser decodes. Re-encode just the offending stream(s) (video for AV1/VP9/HEVC,
audio for Opus/etc), or remux when only the container is wrong. Gated on the mp4 compat
profile (an explicit vcodec / non-mp4 custom profile opts out).
S3 — force re-download (preserves share links):
- POST /downloads/{job_id}/redownload keeps the job row so its public DownloadLink survives
and resolves to the freshly-downloaded file; deletes the old file and re-fetches under the
current rules. A failed re-download leaves the job in error (the intended broken-link case).
- Library "Re-download" action (icon + confirm), api method, hu/en strings.
Tests: naming/title-strip (test_naming), browser_transcode_flags matrix; existing
download_filename expectation updated for the underscore policy.
Shared /watch links showed a "broken play button" on iPad (any iOS
browser — all forced onto WebKit) while playing fine on desktop Chrome.
Root cause: the stored mp4's video codec was AV1 (confirmed via ffprobe),
which WebKit can't decode. The existing H.264 guard was only a
format_sort *preference*, so a non-YouTube source (Facebook) that ships
AV1 as its best video-only stream slipped through.
- B1: for the mp4 compat profile, prefer H.264 at the SELECTION level
with a progressive fallback (bestvideo[vcodec^=avc1]+bestaudio /
best[vcodec^=avc1] / anything) so an avc1 progressive is chosen over
an AV1-only video-only stream.
- B2: re-encode the rare AV1/VP9-only result to H.264+AAC (+faststart)
in the worker before storing, gated on the compat profile; corrects
the asset codec metadata to match.
- Bump _SIG_VERSION 2->3 so cached AV1 assets re-derive.
- Default download preset is now 720p (migration 0058 re-seeds builtins
720p-first; ProfileEditor BLANK 1080->720).
- Tests for the selector, compat opt-outs, and the transcode predicate.
/code-review (S1, finding 3): C-3.19 switched the banner from CURRENT_VERSION (always
RELEASE_NOTES[0]) to FRONTEND_VERSION (VITE_APP_VERSION), which is "dev" in the Vite
dev server and can drift from the notes — so the modal highlighted nothing. The modal
(which already imports RELEASE_NOTES, keeping App decoupled) now falls back to the
newest note when the passed version isn't listed.
The demo account is shared, so writing deep_requested onto its subscription polluted
shared state (harmless today — the scheduler's deep paths are human-only — but a
future scheduler path trusting the flag would leak quota). Guard the flag WRITE at the
source (not user.is_demo), which also makes deep_turned_on always False for demo, so
the redundant is_demo check on the immediate backfill is removed.
/code-review (S1, finding 1): sanitize truncated the title at 120 CHARACTERS, but a
path component's hard limit is 255 BYTES and rel_path packs channel + date + id + ext
into that same component — so a long accented/multi-byte title (Hungarian titles are
common here) could already overflow to ENAMETOOLONG, and the new _a{id} suffix shrank
the margin further. sanitize now truncates on a UTF-8 boundary by byte length, so the
title (≤120B) + channel (≤80B) + fixed parts stay comfortably under 255B. Two tests:
byte-cap on accented input, and every rel_path component ≤255B for a pathological
long-accented channel+title+asset_id.
/code-review caught that role="status" on the toast container implies
aria-atomic="true", so every new toast made a screen reader re-read the entire
stack instead of just the new one — the opposite of the U-F fix's intent. Use a bare
aria-live="polite" (defaults atomic=false, announcing only the added toast); error
toasts keep role="alert". Also simplified the conditional role to a plain ternary.
Review found my C-3.23 additions double-notified: the global error modal (api.ts req)
already surfaces 400/409/422/500 with the backend's specific message, so a caller
toast on those codes is a second, weaker surface. Only 403/404 are caller-handled and
silent — and only a YouTube WRITE action has a meaningful 403 (missing scope).
So the correct set is just the ChannelPage unsubscribe handler (a YouTube write, 403 →
"connect", matching subscribe). Reverted the redundant onErrors on block (a local DB
op, not YouTube), SettingsPanel Plex watch toggle/reimport, and the DownloadCenter job
action — the global modal already speaks for them. Removed the three now-unused keys.
Toaster: moved the polite live region onto the stable, always-mounted container (a
polite region must pre-exist to announce later insertions); error/fatal toasts keep
role="alert", which announces on insertion regardless.
House policy: user actions must report failure. Added onError to:
- ChannelPage block + unsubscribe → notifyYouTubeActionError (a 403 = missing YT
scope shows the connect message; else the translated fallback);
- SettingsPanel PlexWatchSync toggle + reimport → a plain error notify (these are
Plex, not YouTube, so notifyYouTubeActionError's "connect YouTube" would mislead);
- DownloadCenter job action (pause/resume/cancel/delete) → a plain error notify.
New keys (HU+EN): channels.notify.blockFailed, settings.plexSync.failed,
downloads.actionFailed.
The toast is the app's main feedback channel but had no live region, so a screen
reader never spoke it. Each toast now carries role + aria-live by severity:
error/fatal interrupt (alert/assertive), everything else waits its turn
(status/polite).
House policy is HU+EN for every user-facing string. Modal's shared close button was a
hardcoded title="Close" (in every modal) — now t("common.close") + an aria-label.
Welcome's lightbox close aria-label and Feed's "this video" notification fallback are
translated too (new feed.thisVideo, HU+EN). The other Close hardcodes the review
listed were already fixed since.
Review found two low-severity items:
- toast: armDismiss now always clears the prior timer (and arms only when duration is
truthy), so a coalesced repeat that upgrades a toast to requiresInteraction no longer
lets the original timer fire and dismiss it. Was a pre-existing gap the C-3.25 change
hadn't closed.
- overlay: documented the known limitation of useDismiss's blanket stopPropagation — a
PlexPlayer menu open during an auto-skip countdown swallows the Escape that would also
cancel the skip. Proper fix needs R8's shared layer-registry (topmost-only dispatch),
not a broad stopPropagation; accepted until then.
A coalesced repeat re-surfaced the toast and armed a new dismiss timer without
clearing the old one, so it vanished at the ORIGINAL deadline (6s not the expected
11s). Timers are now tracked in a Map<id> and cleared before re-arming (and on
manual dismiss).
Three player/overlay interaction fixes (PlexPlayer changes share a file, hence one
commit):
- U-C (Escape double-close): useDismiss's Escape now stopPropagation()s (its document
listener bubbles before a player's window listener, so one Escape closes only the
popover, not the popover AND the player). Modal exports modalCount(); both players'
Escape defers when a Modal is open above them, so Escape closes the dialog, not the
player under it and then the dialog.
- U-3.1.4: PlexPlayer's Space now defers to a focused BUTTON/A (e.g. "Skip intro")
instead of toggling playback — matching PlayerModal.
- U-3.3.12: the audio/subtitle menus used theme-adaptive glass-menu + forced
text-white (white-on-white in light theme); they now use the player's own explicit
dark panel (border-white/15 + bg-neutral-900/95), readable in both themes.
PanelGroups registered only a PointerSensor, so its focusable, "reorderable" group
grips did nothing from the keyboard. Added KeyboardSensor + sortableKeyboardCoordinates,
matching the other sortable lists (Playlists/PlexPlaylistView already had it).
App.tsx imported CURRENT_VERSION from releaseNotes, which pulled its ~900-line
changelog into the eager App chunk and defeated the lazy ReleaseNotes split. App now
uses FRONTEND_VERSION (the built VITE_APP_VERSION, equal to RELEASE_NOTES[0].version
by release convention) from lib/version, and CURRENT_VERSION is removed. Verified in
the built bundle: the changelog text now lives only in the ReleaseNotes lazy chunk.
A MediaAsset is unique on (source_kind, source_ref, format_sig), so the same video in
two formats is two assets — but rel_path keyed only on video_id, so they computed the
same path: the second download silently overwrote the first, and deleting either
removed the shared file. rel_path now takes an optional asset_id and appends "_a{id}"
(which Plex ignores — the .nfo sidecar carries the metadata); the worker passes the
asset id. Only affects NEW downloads (existing assets keep their stored rel_path).
Covered by a new test (two ids → two paths; omitted → the plain name, unchanged).
The demo account could spend real YouTube quota where the human-only guard was
missing (the channel_detail enrichment already had it):
- channels PATCH: an immediate run_recent_backfill when deep_requested is turned on;
- discovery: the channel-detail enrich on the Discover tab;
- feed get_video_detail: the off-catalog videos.list lookup for a video we don't
store (the DB-hit path stays free; demo now gets a 404 for unknown videos).
The asset-busy and claim-fail branches wrote status='queued' unconditionally, so a
pause/cancel the route had just written was clobbered back to queued and the job
silently revived. New _requeue_if_running does the requeue as a conditional UPDATE
(...WHERE status='running'), a no-op once the route has moved the job off 'running'.
C-3.5: /auth/request-access had no rate limiter (unlike register/login/reset/demo)
and answered "approved" for an allow-listed email — a public enumeration oracle for
the allow-list, and an unthrottled invite/admin-mail firehose. It now rate-limits per
client IP and returns a uniform "pending" for every outcome (allowed, pending, new,
throttled); an already-allowed user can still just sign in with Google.
C-3.11: _consume_token was a read-check-write, so two concurrent requests could both
consume the same verify/reset token. It's now a single conditional UPDATE
(...WHERE used_at IS NULL AND expires_at >= now RETURNING user_id) — only the request
that flips used_at gets a user back.
Internal-only epic (gates, tests, compiler strictness); no user-facing change, so
the release note is chores-only. Bundles: the siftlode check/publish gate
(tsc/eslint/prettier/knip/vitest/pytest) with a hard pre-publish gate + e2e
freshness guard, ESLint flat config + one-time Prettier pass, the backend pytest
harness (isolated test image), frontend vitest growth, the e2e scroll-restore fix,
and noUncheckedIndexedAccess (+ the zero-fallout compiler flags).
Enable noUncheckedIndexedAccess and fix all 98 call sites it surfaced across 18
files. Every fix is behaviour-preserving — the flag flagged reads TypeScript
couldn't prove in-bounds, all of which were already guarded by a length/index check,
a findIndex result, or a non-empty invariant:
- read-once so a ternary's narrowing sticks (Feed overrides, linkify mention);
- non-null assertion AFTER an existing guard (PlayerModal/VideoEditor queue+cut-list
indexing, modules step, useUndoable stacks) with a comment stating the invariant;
- nullish fallback where undefined is a real possibility (GlassTuner slider ?? def,
ConfigPanel active group ?? [], descriptionLinks id ?? null);
- optional chaining where the entry genuinely can be absent (PlexBrowse IO entry);
- a non-empty tuple type for the RELATIVE_UNITS constant (encodes "always has [0]").
The single highest-leverage fix: PlayerModal's `active` became `queue?.[index] ??
video` (always Video), clearing 34 of the 45 errors in that file at once. Verified:
tsc/eslint/prettier clean, 56 vitest + 34 pytest + 17/17 e2e green (the e2e suite
exercises the feed/player/channel components touched here).
Three strictness flags that the codebase already satisfies (0 errors each):
- noFallthroughCasesInSwitch on src + e2e — a missing `break` is now a type error.
- noUnusedLocals + noUnusedParameters on the e2e project, so it matches src and node
and dead spec scaffolding can't accumulate (it was the one project without them).
- allowJs + checkJs on the node project, pulling tailwind.config.js and
postcss.config.js under the type checker (the last hole in C-4.5) — a real type
error in either is now caught (verified: `content: 123` fails; Config stays
permissive on plugin keys by design).
noUncheckedIndexedAccess is NOT here — it has 98 call-site fixes and lands as its own
reviewable sub-sprint (S3b).
Review round 1: the two existing safe_abs_path escape tests both pass even against
the classic buggy `str(path).startswith(str(root))` containment check, so a
regression to that bug would have shipped green. Add the one input that
distinguishes the correct `root not in path.parents` guard: a sibling dir whose name
prefixes the root (/root vs /rootx). Mutation-verified — swapping the guard to
startswith now fails this test. Also corrected the requirements-dev header (installed
by Dockerfile.test, not a nonexistent `dev` stage).
The backend had zero tests. This adds 33, all pure (no DB, no network), plus the
harness to run them in the gate:
- backend/Dockerfile.test: the app's deps + pinned pytest/ruff, code bind-mounted at
run time so it always tests the working tree. Isolated from the prod Dockerfile, so
the published image never carries test tooling.
- requirements-dev.txt: pytest==8.4.2, ruff==0.15.21 (the version the gate already
runs) — the backend lane is now pinned, not just host-global.
- tests: normalize_title (de-shout, hashtag strip, emoji drop, trilingual letters
survive); storage sanitize/rel_path/download_filename and the safe_abs_path
traversal guard (../ and absolute-path escapes rejected — verified by mutation);
links HMAC grants (round-trip, wrong-token, tampered sig/exp, expiry) + is_expired;
a DB-free app-assembly smoke (every router wires up, OpenAPI generates).
DB-backed router smoke (auth/feed/downloads) needs a test Postgres + migrations —
deferred. useCardPager needs hook-test infra (jsdom/renderHook) — deferred.
22 vitest cases for two pure helpers that had none:
- columnSort: numeric-vs-locale comparison, asc/desc, no-mutation, the null/junk
coercion in parseSortState, and the unsorted→asc→desc→unsorted click cycle.
- linkify: http/www/bare-domain URLs, the version-number non-match, the
"<platform>: @handle" mention (handle linked, prefix kept as text), alias→canonical
base URL, bare @handle/#hashtag left plain, and multi-link ordering. Nodes are
inspected structurally, so the existing node-env vitest needs no DOM.
useCardPager (a hook) is deferred to when component/hook test infra lands with the
backend pytest sprint.
The "Back restores the feed scroll position" spec failed against the production
build while the app was in fact correct. Root cause, found by instrumenting
PageScroller and comparing the baked :8080 build to live :5173:
`getByTestId("video-card-channel").first()` grabs the first card in the DOM, which
after scrolling 3000px is in the virtualizer's overscan ABOVE the viewport.
Playwright scrolls it into view to click it, moving the feed from 2879 to ~1017–1253
BEFORE navigating; PageScroller's save-on-scroll records that moved position, so Back
faithfully restores the moved position — under the test's own tolerance. A real user
clicks a card they can see, which does not move the feed.
New helper clickVisibleChannelLink() clicks a card whose bounding box is inside the
viewport. Restore now lands at ~2804 (was ~1017). 17/17 pass; stable over 3 reruns.
No app change — the scroll-restore logic was already correct.
Lists the repo-wide format commit so `git blame` skips it. One-time local setup:
`git config blame.ignoreRevsFile .git-blame-ignore-revs` (Forgejo honours it too).
Prettier 3 (pinned), printWidth 100, double quotes — chosen to match the existing
hand-formatting and minimise reflow. This commit is ONLY the mechanical format pass
(`prettier --write`) plus the config/ignore/scripts, isolated so it never pollutes a
feature diff. Verified behaviour-neutral: typecheck, eslint (0 errors), and vitest
all green before and after. See .git-blame-ignore-revs — `git blame` skips this sha.
ESLint 9 flat config, pinned. Narrow by design: the load-bearing rule is
react-hooks (rules-of-hooks + exhaustive-deps) — 33 disable comments existed for a
rule nothing ran. tsc already owns unused vars / undefined / types, so those are
turned off here to report each finding once. no-explicit-any is off: all 19 live in
the api.ts god-module that R7 rewrites, and it flips the rule back on then.
reportUnusedDisableDirectives is an error, which immediately caught two inert
`eslint-disable` comments (App.tsx, AddToPlaylist.tsx) whose effects have stable
deps — removed. no-unused-expressions allows the side-effect ternary idiom the
codebase already uses (`v.paused ? v.play() : v.pause()`).
Errors: 0. Remaining 38 are warnings (28 react-refresh DX, 10 exhaustive-deps) —
pre-existing, visible in the lint output, tracked to their epics.
Review round 1 fallout, all of it verified against real builds:
- .dockerignore: `node_modules/` is anchored at the context root and has no
implicit `**/` (unlike gitignore), so it never matched frontend/node_modules.
`COPY frontend/ ./` was overlaying the Windows host's node_modules on top of
what `npm ci` had installed — the image carried @esbuild/win32-x64 and 31 .cmd
shims, and the new typecheck ran a tsc resolved from that merged tree. Build
context transfer drops 3.05 GB -> 19 kB. Also excludes e2e/.auth (a live
Playwright session), the report/result dirs and logs.
- build no longer type-checks the e2e project: a type error in a spec must not
make the app unbuildable for a self-hoster. The gate still checks it via
`npm run typecheck` (now three named lanes).
- tsconfig.json references tsconfig.node.json, so editors check vite.config.ts /
vitest.config.ts under the same config the gate uses instead of an inferred one.
- vitest include accepts .test.tsx: an uncollected test file does not fail, it
silently never runs.
`npm run build` now runs `typecheck` first, so the Docker image is built through the
same gate as local dev — until now the image was a bare `vite build`, and esbuild
strips types without checking them.
typecheck covers three projects: src, the Node-side build configs (new
tsconfig.node.json — vite.config.ts/vitest.config.ts were type-checked by nothing),
and e2e. knip moves from `npx knip@latest` to a pinned devDependency so the gate
stops depending on the network and on whichever version shipped that morning.
Closes E4. The playlist detail gains real sortable/filterable column headers
over the drag-reorderable list, and the crucial distinction: the sort and the
filters are a VIEW, while the stored order changes only on drag or an explicit
save. Picking a sort used to write straight through to the server.
Also extracts the column header out of DataTable (shared by both tables now,
which fixed the filter popover remounting on every keystroke), and closes eight
findings from the 2026-07-20 reviews that lived in these files.
Sorting a playlist whose stored order already matches the sort produced a
warning, no Save button and no visible change — which reads as a malfunction,
not as "there is nothing to save". And when the order DID change, three separate
texts piled up in the toolbar (a reset link, a save hint, a "reordering is off"
notice), none of which answered the question actually being asked: did I just
change my playlist?
One line now names which of the three situations you are in — the view matches
the saved order, the view differs and can be saved, or a filter is on and saving
is impossible — and says in each case that the saved order is untouched. The
disabled grip still explains itself on hover and focus, so that notice no longer
needs to stand in the toolbar.
- The dragged row trailed the cursor by 150ms. The row carried Tailwind's bare
`transition` utility (copied from DataTable's <tr>), which animates TRANSFORM
— exactly the property dnd-kit drives from the pointer. transition-colors now,
and the row being dragged drops dnd-kit's own transition too.
- The list looked sorted by Channel when it was really in its stored order that
happens to be channel-grouped, and nothing said so. Sorting by "#" ascending
IS the stored order, so it is now the neutral state: the header shows a real
arrow for it, cycling off any column lands back on it, and it stays the state
where drag is enabled and no save is offered.
- A choice dialog with two or more actions stacks vertically. Side by side they
wrapped raggedly, and the wrapped one read as a lesser second group when the
two are peers. Cancel goes last, actions run least to most destructive.
A truncate cell is white-space:nowrap, so its min-content width is the whole
title — under table-layout:auto the Title column grew until Length and the
remove button hung off the scroller (measured: table 1059px inside an 855px
wrapper, main.scrollWidth 1074 vs clientWidth 887). Fixed layout with a declared
width on every column but Title, which absorbs the rest.
Header (shared, so every table gets these):
- The filter funnel could not close itself: useDismiss only knew the panel, so
the trigger's mousedown closed it and the click reopened it. It now knows the
trigger, captured at open time so it survives the close.
- Escape stranded focus on <body>; it goes back to the funnel, which also gained
aria-expanded/aria-haspopup.
- A non-sortable header rendered a focusable no-op button — two unlabeled dead
stops per playlist table, whose grip and actions headers are empty. Plain span
unless the column sorts.
Playlist:
- The disabled drag grip explained itself to nobody: `disabled` drops a button
from the tab order and suppresses its tooltip. It is aria-disabled now, stays
focusable, and the reason is its accessible name.
- That reason also lied on a built-in list, where the block has nothing to do
with sorting or filtering. Third branch, own string.
- The open player followed a position in a derived list, so a refetch or a view
change could slide a different video under it or unmount it mid-playback. It
follows the video id, like PlayerModal's own queue already does.
- A failed reorder could not resync: the id set is unchanged by a reorder, so
the arriving detail was judged "same list" and the rejected order stayed on
screen as the base for the next drag.
- The same guard is now keyed by playlist. Two lists holding the same videos
looked identical to it, so the second rendered in the first's order and
inherited its undo stack — where Undo would write it back to the wrong list.
- A one-item list filtered to nothing had no Reset view: the whole control strip
was gated on the item count while the funnels rendered regardless.
- Both delete outcomes now read as actions ("Delete here only" / "Delete here
and on YouTube"); "Here only" scanned as the safe choice while also deleting.
- Keyboard dragging shipped with dnd-kit's hardcoded English screen-reader
instructions and announcements. Localized, with live positions.
The column sort and the filters are a VIEW; the playlist's stored order only
changes when the user presses "Save this order", which still goes through the
undoable path. Picking a sort used to write straight to the server, so merely
looking at the list by title rewrote the playlist and marked a YouTube-linked
mirror dirty.
The table:
- playlistColumns.tsx holds the column defs; the body renders them as table rows
and fills the grip cell itself, since the handle needs the row's sortable
listeners that a render closure can't reach.
- The # column shows the STORED position, so a view-sort never renumbers the
playlist under the reader.
- Saving is withheld while a filter is active (it would drop the hidden items),
and drag is disabled under any view, with the reason spelled out rather than
silently doing nothing.
- applyView/sameOrder live in lib/playlistView.ts with tests: the sort and the
channel grouping compose, and getting that wrong produces a plausible order
that the save button would then persist.
- dnd-kit KeyboardSensor: the grip was focusable and called itself "reorder",
but only a pointer could move it.
The guards:
- Delete is ONE dialog with every outcome. It was two chained confirms where the
second one's Cancel/Escape/backdrop meant "delete here only" — dismissing the
dialog deleted the playlist. ConfirmProvider gained useChoice() for it.
- ConfirmProvider focuses Cancel when an offered action is destructive; the
confirm button took autoFocus unconditionally, so a stray Enter on an open
dialog ran "Delete user" or "Clear audit log".
- Removing an item asks first, and rebases the undo history instead of throwing
it away: useUndoable.rebase maps every snapshot, so the reorder history stays
valid for the remaining items.
- A failed detail query showed "Loading…" forever; it now says so and offers a
retry. Rename and reorder failures are surfaced instead of swallowed (reorder
was a bare .then(), which also produced an unhandled rejection).
- Local reorder/remove no longer require YouTube write scope.
- playlistName() replaces the watch-later localizer copied into three files.
The sort cell, filter funnel/popover and filter predicate move to
components/tableHeader.tsx so a non-DataTable body can render under the same
headers — the playlist manager's drag-reorderable list needs the headers
without inheriting DataTable's internal ordering state.
FilterPopover reaches module scope in the process. It was declared inside
DataTable's body, so it was a new component type on every render and React
remounted it on each keystroke; verified against the pre-change build, where
the input's DOM node is replaced on all four keystrokes of a test word.
Also picks up two things that live in this markup: aria-sort on the sortable
headers, and a real hit target on the filter funnel (it was 14x14).
Fixes a bug live since 0.2.0 (ea317c0): every relative timestamp named the
tier below its own range, so minutes read "Ns ago" and hours "N min ago".
Brings the repo its first unit-test runner along with it.
The tier table kept a divisor and a translation key as two independently
maintained halves, and a mispairing there rendered minutes as "Ns ago" for 59
releases. The table now pairs a divisor with the Intl unit it produces and the
platform supplies the words, so the wrong half is at least self-evident on
sight — "minute" next to 3600 reads wrong in a way "time.minutesAgo" never
did. It also removes 12 hand-written strings and hands plural rules to CLDR
for any language added later.
Hungarian output is byte-identical to the strings it replaces. English gets
shorter: "59 min ago" to "59m ago", "1 wk ago" to "1w ago", "1 mo ago" to
"1mo ago", "1 yr ago" to "1y ago"; "2h ago" and "1d ago" are unchanged.
style narrow matches the app's compact timestamps and numeric always keeps it
counting, rather than "yesterday"/"last week". time.justNow stays because Intl
has no wording for the sub-minute case. Formatters are memoized per language,
since this runs once per rendered timestamp across a virtualized feed.
Review follow-ups on the tier-shift fix. The table moves to a module-level
RELATIVE_UNITS: it is constant, so rebuilding it per call was pure garbage on
a path that runs once per rendered timestamp across a virtualized feed.
Its ascending order is load-bearing and was unstated — sorting it largest-first
would make every timestamp under a year read "just now", silently, the same
shape as the bug this branch fixes. It is now documented and asserted, and the
suite derives its tier coverage and translation checks from the table itself
rather than from its own case list, so a new tier cannot slip in uncovered.
The audit log read the empty string for an unreadable date as a value and
rendered a blank cell; it now uses the `|| "—"` idiom the channel table
already uses, so missing and unreadable dates look alike.
The repo had no unit-test runner, so this adds the minimum: vitest (3.x, as
4.x needs Vite 6), a node-env config deliberately separate from vite.config.ts
so the production build never loads it, npm test / test:watch, and a knip
entry so the config and specs are not reported unused. Browser flows stay in
e2e/ under Playwright.
The suite samples every tier at its exact opening second as well as inside it,
and stubs i18n so each assertion lands on the key rather than the copy. An
interior-only suite still passes when the range comparison flips to <=, which
is how the tier shift went unnoticed for so long.
The units table paired each divisor with the NEXT row's label, so the count
came out right and the word wrong, all the way up the scale: minutes rendered
as "Ns ago", hours as "N min ago", days as "N hours ago", months as "N weeks
ago". Most visible on the audit log, where scheduler rows three minutes apart
read "1s / 4s / 7s ago".
Each row now carries its own label and is read whole, so a divisor can no
longer drift from its word. Also guard a non-finite age, which rendered
"NaN mo ago", and drop time.secondsAgo now that nothing emits it.
Present since ea317c0 swapped the hardcoded English strings for i18n keys;
shipped in every release from 0.2.0 on.
- The tag menu's flip was one-way: once anchored above it stayed there, so a row
scrolling toward the top dragged the menu off the screen. It now re-decides both
ways each pass — prefer the side it is on (no flapping), leave it as soon as that
side stops fitting and the other does. Verified: trigger at y=229 (no room above)
flips back below, menuTop 253 = btnBottom 249 + 4, nothing off-screen.
- Extract the card pagination into lib/useCardPager: size (persisted, validated),
page, the out-of-range clamp-commit, the slice and the Pager props lived twice,
and every review round had to fix both copies in lockstep. The two tabs now keep
only what differs — which rows, which storage key, and when to reset.
The clamp is gated on the card layout too, so it no longer rewrites card state
while the table is the one on screen.
- The tag menu's "trigger is gone" test now measures against the SCROLLER, not the
window. The page scroller starts ~230px down (measured), so a row that slid under
the fixed toolbar was still "on screen" by window coords and the menu floated over
the chrome, anchored to something invisible.
- Scroll-repositioning keeps whichever side the menu is anchored to instead of
resetting it to "below" every frame — that reset made the layout effect flip it
back, costing two renders and two forced layouts per frame near the bottom edge.
- One placement rule (`coordsFrom`) now serves the initial open, the layout
correction and every reposition, so those can no longer drift apart.
- Card page: reset on the filter/sort INPUTS only, and separately COMMIT the clamp
when the page falls out of range. Keying the reset on the row count (round 2) was
too blunt — a background refetch that merely added a channel threw the reader off
page 7 for something they didn't do.
- Pager derives `pageCount()` once and `hasPagerContent` reuses it, instead of the
same expression twice in one file.
- Re-indent the pagerNode JSX left dangling by round 2's ternary edit.
- The portalled tag picker now FOLLOWS its trigger on scroll (rAF-coalesced)
instead of closing. Closing on every wheel tick was a regression for the table,
where the menu used to travel with its row; it still closes once the trigger
leaves the viewport (nothing left to anchor to) or on resize.
- Place the menu by MEASUREMENT, not a px estimate: it opens below, then the
layout effect flips it above only if the real box overflows. The old
`rows * 30px` guess read short at font-scale 1.3 (the rows are rem), which
could leave a fixed menu hanging off the bottom with no way to reach it.
- `toggle` only opens once a position exists, so the `open && coords` gate can
never show a stale anchor.
- One owner for "is there anything to page": `hasPagerContent()` lives in Pager
and DataTable calls it, instead of both re-deriving the rule — that drift is
what left empty control bands behind in the first place.
- Reset the card page on the row COUNT too, not just the filter inputs, so an
unsubscribe or a shrinking refetch can't leave a stale page to spring back.
- `parseCardSize` clamps to the sizes the pager actually offers (like clampView /
clampStatus), so the select can't display one value while the grid pages by another.
- VirtualGrid's docblock now carries the stacking-context constraint: its rows are
transformed, so anything floating an item renders must portal out.
In the card layout every VirtualGrid row carries a `transform`, which makes it a
stacking context — so the tag menu's z-index was scoped to its own row and the
NEXT card (a later DOM sibling) painted over it. Same hazard that buried the
players in E3, so the same blessed fix: render the menu through <Overlay> (i.e.
at <body>) with fixed coords instead of `absolute` inside the cell.
The flip-up case anchors the menu's BOTTOM to just above the button so it lands
exactly whatever its real height is, and the horizontal clamp measures the
rendered width rather than assuming px — `w-44` grows with the text-size slider.
Closes on outside click (dismiss now watches the cell AND the portalled menu),
and on page scroll/resize, where a fixed menu would otherwise strand at a stale
anchor. Verified in both layouts by hit-testing the menu's own pixels.
- DataTable: keep `pagerNode` NULLABLE. Extracting Pager made it an always-truthy
element, so the `controls` / `bottomControls` guards always fired and a table with
nothing to page rendered empty `my-3` bands (plus an empty in-band PageToolbar).
- ChannelsProvider: `setView` bails when the tab is unchanged (re-selecting the
active tab, or goToFullHistory forcing "subscribed", stacked dead Back entries),
and pushes a FRESH state like setPage does instead of spreading `_chan`/`_yt`
markers into the tab entry.
- Lift the layout into its own narrow context (`useChannelLayout`): App read the
whole ChannelsState for the fade, so every keystroke in the manager's search box
re-rendered the app root.
- Reset the card page when the row set or sort changes (both tabs) — the clamp only
hid a stale page, which came back when the filter was cleared.
- Sort + slice the card rows only in cards mode; table mode was copy-sorting ~300
rows per render for a result nothing read.
- Validate persisted `cardSize` / `sort` (`parseCardSize`, `parseSortState`), matching
the clamp-on-read convention the module's other persisted values already follow;
a corrupt size made the page count NaN and emptied the grid.
- Pager: page-size changes scroll to the top too (they re-slice from a new first item).
- ColumnSortControl: a stored sort naming a column this table no longer offers is
treated as no sort, instead of showing a direction arrow for a sort that isn't applied.
- Drop the stale "rows" layout comments (that variant was retired this sprint).
UAT round 4:
- Discovery table: move Actions to the first column, mirroring the Subscriptions
table (where it sits right after Prio) so both tabs' actions line up.
- The manager's tabs now ride in history.state (`_chTab`), the same way the channel
page (`_chan`) and the live search (`_yt`) already do — so Back steps back through
the tabs you visited before it leaves the module. Arriving from elsewhere stamps
the entry setPage pushed (one navigation never costs two Back presses).
- Turning a page now scrolls the list back to its top (both the tables and the card
grid, since they share Pager) — landing mid-list in an unseen page is disorienting.
UAT round 3 feedback on the card layout:
- Add a pager + "Cards per page" to the card view (it infinitely-scrolled before).
Extracted the pager cluster out of DataTable into a reusable Pager component
(the table now renders it too — no behaviour change); the card view slices the
sorted rows and drives its own per-account page size (subscribed + discovery).
- Restore the top edge-fade in the card layout. The channels page registers
fadeTop:false for its sticky-header table; the card layout has no sticky header,
so App now re-enables the top fade when the channel layout is cards.
Cards have no column headers to sort by, so lift the sort out of DataTable into
one shared state that BOTH views drive and reflect. New lib/columnSort.ts holds
the comparator + click-cycle (one source of truth); DataTable gains an optional
controlled sort (sort + onSortChange) and falls back to its internal persistKey
sort when they're omitted, so every other table is untouched. The subscribed
sort lives in ChannelsProvider, discovery keeps its own (its columns differ);
a new ColumnSortControl dropdown (options derived from the columns' sortable
metadata, feed-style + asc/desc toggle) drives it in cards mode. Per-account,
persisted, per-tab.
UAT round 1 feedback:
- Retire the `rows` layout — it barely differed from the table; keep Table + Cards.
- The card now shows the About cell (+ its hover overlay) and the priority stepper
in every layout, not just the table: ChannelLayoutGrid gains named slots
(lead / about / actions) and stops hiding the `hideInCard` columns, since the
roomy desktop card has space the cramped mobile fallback did not.
- Fix the table losing its centering with "Rows per page: All": a long-titled
channel stretched the uncapped Channel column (~289→429px) and overflowed the
centered table. Cap it with a max-w element inside the cell (table-layout:auto
ignores a cell max-width), the same trick AboutCell already uses. Gaps now 163/162.
Self-review follow-ups: the `rows` layout mapped the primary column without a
React key (single-element today, but a warning waiting to happen); wrap it in a
keyed Fragment. Also replace the guessed card/row rowEst with browser-measured
values (176 / 62) so the scrollbar doesn't jump on first scroll.
A generic ViewSwitcher (reused from the feed) drives a per-account layout
shared by the Subscriptions + Discovery tabs. `table` keeps the DataTable
(numbered pager); `cards` and `rows` render a virtualized grid over the same
client-side list via the new ChannelLayoutGrid, which composes cells straight
from each table's Column<T> render closures (cardPrimary/hideInCard/cardLabel),
so both tabs — with their different columns — get the views for free. Status
chips surface in the fixed band when the table's own controls row is gone.
VirtualFeed becomes a thin wrapper that maps a FeedView to VirtualGrid's
layout spec and renders a VideoCard per item; all measure/chunk/place/
infinite-scroll mechanics move to VirtualGrid so the channel manager (E4 S3)
can reuse them. Behaviour-preserving: e2e 17/17, all render families smoked.
Platform-prefixed handles (x/twitter/instagram/facebook/tiktok/youtube/twitch/threads/
telegram/soundcloud) now link to the platform's profile URL — only the @handle is
linked, the '<platform>:' label stays as text. A required colon/arrow separator + the
known-platform list keep false positives out; a bare @handle with no platform stays plain.
The description linkifier only caught http(s):// URLs; extend it to www.* hosts and
bare domain.tld/path URLs (both get an https:// href prefix), so links like
www.amelielens.com and facebook.com/amelielensmusic become clickable too. A required
path + letters-only TLD keep false positives (version numbers, file names) out; bare
@handles and #hashtags stay plain (the platform is ambiguous).
- linkify: bare http(s) URLs inside channel descriptions become clickable links
(new tab, rel=noopener noreferrer) — new lib/linkify.tsx, used in the shared
ChannelAboutContent so both the About overlay and the channel detail page get it
- Country flag: the flag EMOJI never renders on Windows/Chrome (showed the small-caps
'hu' fallback), so swap it for a real self-hosted SVG flag via the flag-icons package;
applies to the About overlay + the channel detail page
- The About overlay now shows the FULL channel About (description, external links,
country, language, topics, keywords) — the same content as the detail page's About
tab, via a new shared ChannelAboutContent component (reused by ChannelPage + AboutCell)
- AboutCell fetches the channel detail ON DEMAND on hover (shared [channel,id] react-query
cache with the channel page), so the list stays light; the overlay is interactive
(scrollable + clickable links) via a small hover-bridge, and closes on page scroll/resize
- Center the content: max-w-7xl -> max-w-[96rem] on the channel tables + header, so the
wider About table stays symmetric with the header (mx-auto handles the responsive centering)
- Topic chips are display-only for now; the future search epic makes them clickable
- Backend: add channel description to the shared _channel_summary projection, so both
the subscribed list and the discovery list carry it (no migration; description is
already stored from the channels.list call)
- New AboutCell: one truncated line in the table + full text in a portalled, always-on
glass hover/focus overlay (wider than Tooltip, flips above the anchor near the bottom,
closes on scroll/resize)
- About column (after Channel) on both the subscribed and discovery tables; hidden in the
mobile card fallback
peter