peter/siftlode
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(e4): address code-review findings (a11y sweep + dedup guards)

Browse Source 
Resolves the 5 /code-review high findings on this branch:
- a11y sweep: extend aria-label to the remaining icon-only Plex card buttons the
  first pass missed — PlexPosterCard Info, SeasonCard watched-toggle + add-season
  (EpisodeCard already had them). "Fix a principle everywhere, in one sweep."
- new shared lib/parseId.ts `parsePositiveIntId`, used by BOTH readSelectedId
  (now guarding the stored ?pl fallback the same as the URL — was unguarded) and
  plexSubUrl's ?v=playlist decode; removes the hand-duplicated guard.
- PAGE_KEY exported from navigationInit and imported by NavigationProvider (was
  defined in both).
- hoist a `watchedLabel` const per card so title + aria-label can't drift.

+8 tests (parseId unit + guarded-stored ?pl); full FE gate green (tsc/eslint/
prettier/knip, 317 vitest).
This commit is contained in:
peter
2026-08-20 05:40:59 +02:00
parent 0b7180ce8f
commit 482dd28006
8 changed files with 68 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/components/NavigationProvider.tsx
+2 -4
View File
@@ -11,10 +11,10 @@ import {
import { useTranslation } from "react-i18next";
import { urlWithParams, type Page } from "../lib/urlState";
import { pageTitleKey } from "../lib/pageMeta";
import { LS, setAccountRaw } from "../lib/storage";
import { setAccountRaw } from "../lib/storage";
import { useAccountRehydrate } from "../lib/useAccountRehydrate";
import { setNavigator } from "../lib/nav";
import { channelViewFrom, loadInitialPage, type ChannelView } from "./navigationInit";
import { channelViewFrom, loadInitialPage, PAGE_KEY, type ChannelView } from "./navigationInit";
// Owns the app's navigation + browser-history state, lifted out of the App god component so pages
// consume it via a hook instead of prop-drilling `page`/`setPage`/`onOpenChannel` through the tree.
@@ -27,8 +27,6 @@ import { channelViewFrom, loadInitialPage, type ChannelView } from "./navigation
// re-render on a page/channel switch — preserving the memo that keeps feed appends from re-rendering
// every existing card.
const PAGE_KEY = LS.page;
export interface NavigationState {
page: Page;
/** The open channel page (null = none). Overlays `page`; rides in history.state._chan. */
frontend/src/components/navigationInit.ts
+1 -1
View File
@@ -5,7 +5,7 @@
import { hasFilterParams, isPage, readPage, type Page } from "../lib/urlState";
import { getAccountRaw, LS } from "../lib/storage";
const PAGE_KEY = LS.page;
export const PAGE_KEY = LS.page;
export interface ChannelView {
id: string;
frontend/src/components/playlistsInit.test.ts
+8
View File
@@ -48,6 +48,14 @@ describe("readSelectedId ?pl guard", () => {
expect(readSelectedId()).toBe(9);
});
it.each(["", "0", "-2", "1.5", "abc"])(
"rejects a non-positive-integer STORED value %s (guarded like the URL)",
(raw) => {
stored.v = raw;
expect(readSelectedId()).toBeNull();
},
);
it("returns null when neither ?pl nor a stored selection is present", () => {
expect(readSelectedId()).toBeNull();
});
frontend/src/components/playlistsInit.ts
+8 -8
View File
@@ -1,14 +1,14 @@
import { getAccountRaw, LS } from "../lib/storage";
import { parsePositiveIntId } from "../lib/parseId";
// The Playlists module's selected-id seed, split out of PlaylistsProvider so its ?pl guard is
// unit-testable and the provider stays react-refresh-clean (component-only exports).
export function readSelectedId(): number | null {
// A ?pl bookmark wins over the stored selection so a pasted link opens that playlist. Accept only a
// positive integer id — a bare "?pl=" gives Number("")===0 (finite!) and "?pl=1.5" a fractional id,
// both of which would select a non-existent playlist.
const fromUrl = new URLSearchParams(window.location.search).get("pl");
const n = fromUrl ? Number(fromUrl) : NaN;
if (Number.isInteger(n) && n > 0) return n;
const s = getAccountRaw(LS.playlist);
return s ? Number(s) : null;
// A ?pl bookmark wins over the stored selection so a pasted link opens that playlist. BOTH must be a
// positive-integer id (a bare "?pl=" gives Number("")===0, "?pl=1.5" a fractional id) — either would
// select a non-existent playlist, so the stored fallback is guarded exactly like the URL.
return (
parsePositiveIntId(new URLSearchParams(window.location.search).get("pl")) ??
parsePositiveIntId(getAccountRaw(LS.playlist))
);
}
frontend/src/components/plexCards.tsx
+13 -7
View File
@@ -50,6 +50,7 @@ export function PlexPosterCard({
const { t } = useTranslation();
const inProgress = (card.position_seconds ?? 0) > 0 && card.status !== "watched";
const isPlayable = card.type !== "show"; // movies/episodes play; shows open the drill-down
const watchedLabel = card.status === "watched" ? t("plex.markUnwatched") : t("plex.markWatched");
const pct =
inProgress && card.duration_seconds
? Math.min(100, Math.round(((card.position_seconds ?? 0) / card.duration_seconds) * 100))
@@ -99,8 +100,8 @@ export function PlexPosterCard({
e.stopPropagation();
onToggleWatched(card);
}}
title={card.status === "watched" ? t("plex.markUnwatched") : t("plex.markWatched")}
aria-label={card.status === "watched" ? t("plex.markUnwatched") : t("plex.markWatched")}
title={watchedLabel}
aria-label={watchedLabel}
className="absolute top-1.5 right-1.5 p-1 rounded-full bg-black/60 reveal hover:bg-black/80 transition"
>
<WatchedIcon watched={card.status === "watched"} />
@@ -114,6 +115,7 @@ export function PlexPosterCard({
onInfo();
}}
title={t("plex.info.title")}
aria-label={t("plex.info.title")}
className="absolute bottom-1.5 left-1.5 p-1 rounded-full bg-black/60 reveal hover:bg-black/80 transition"
>
<Info className="w-4 h-4 text-white" />
@@ -192,6 +194,9 @@ export function SeasonCard({
}) {
const { t } = useTranslation();
const watched = se.status === "watched";
const watchedLabel = watched
? t("plex.series.markSeasonUnwatched")
: t("plex.series.markSeasonWatched");
return (
<div className="group text-left">
<div
@@ -225,9 +230,8 @@ export function SeasonCard({
e.stopPropagation();
onToggleWatched();
}}
title={
watched ? t("plex.series.markSeasonUnwatched") : t("plex.series.markSeasonWatched")
}
title={watchedLabel}
aria-label={watchedLabel}
className="absolute top-1.5 right-1.5 p-1 rounded-full bg-black/60 reveal hover:bg-black/80 transition"
>
<WatchedIcon watched={watched} />
@@ -241,6 +245,7 @@ export function SeasonCard({
onAdd();
}}
title={t("plex.playlist.addSeason")}
aria-label={t("plex.playlist.addSeason")}
className="absolute bottom-1.5 right-1.5 p-1 rounded-full bg-black/60 reveal hover:bg-black/80 transition"
>
<ListPlus className="w-4 h-4 text-white" />
@@ -404,6 +409,7 @@ export function EpisodeCard({
}) {
const { t } = useTranslation();
const inProgress = (ep.position_seconds ?? 0) > 0 && ep.status !== "watched";
const watchedLabel = ep.status === "watched" ? t("plex.markUnwatched") : t("plex.markWatched");
const pct =
inProgress && ep.duration_seconds
? Math.min(100, Math.round(((ep.position_seconds ?? 0) / ep.duration_seconds) * 100))
@@ -438,8 +444,8 @@ export function EpisodeCard({
e.stopPropagation();
onToggleWatched();
}}
title={ep.status === "watched" ? t("plex.markUnwatched") : t("plex.markWatched")}
aria-label={ep.status === "watched" ? t("plex.markUnwatched") : t("plex.markWatched")}
title={watchedLabel}
aria-label={watchedLabel}
className="absolute top-1.5 right-1.5 p-1 rounded-full bg-black/60 reveal hover:bg-black/80 transition"
>
<WatchedIcon watched={ep.status === "watched"} />
frontend/src/components/plexSubUrl.ts
+4 -3
View File
@@ -1,4 +1,5 @@
import type { SubviewUrl } from "../lib/history";
import { parsePositiveIntId } from "../lib/parseId";
// The Plex module's sub-view union and its address-bar mirror, extracted from PlexBrowse so the
// encode/decode round-trip (and its guards) can be unit-tested without pulling the whole component
@@ -46,9 +47,9 @@ export const PLEX_SUB_URL: SubviewUrl<Sub> = {
return arg ? { kind: "person", name: arg } : null;
case "playlist": {
// Positive-integer id only — a fractional/zero/negative ?v=playlist:<arg> would open a
// sub-view for a non-existent id (matches the ?pl guard in PlaylistsProvider.readSelectedId).
const id = arg ? Number(arg) : NaN;
return Number.isInteger(id) && id > 0 ? { kind: "playlist", id } : null;
// sub-view for a non-existent id (shares parsePositiveIntId with readSelectedId's ?pl guard).
const id = parsePositiveIntId(arg);
return id != null ? { kind: "playlist", id } : null;
}
case "season": {
const [showId, seasonId] = arg.split(":");
frontend/src/lib/parseId.test.ts
+23
View File
@@ -0,0 +1,23 @@
import { describe, it, expect } from "vitest";
import { parsePositiveIntId } from "./parseId";
describe("parsePositiveIntId", () => {
it("accepts a positive integer", () => {
expect(parsePositiveIntId("1")).toBe(1);
expect(parsePositiveIntId("42")).toBe(42);
});
it("rejects empty / nullish", () => {
expect(parsePositiveIntId("")).toBeNull();
expect(parsePositiveIntId(null)).toBeNull();
expect(parsePositiveIntId(undefined)).toBeNull();
});
it("rejects zero, negative, fractional, and non-numeric", () => {
expect(parsePositiveIntId("0")).toBeNull();
expect(parsePositiveIntId("-1")).toBeNull();
expect(parsePositiveIntId("1.5")).toBeNull();
expect(parsePositiveIntId("abc")).toBeNull();
expect(parsePositiveIntId(" ")).toBeNull(); // Number(" ")===0
});
});
frontend/src/lib/parseId.ts
+9
View File
@@ -0,0 +1,9 @@
/** Parse a raw value to a positive-integer id, or null if it isn't one. A bare "" gives
* Number("")===0 (finite but not positive), "1.5" is fractional, "-1"/"abc" invalid — all rejected.
* The single guard behind the ?pl / ?v=playlist id params AND the stored playlist selection, so a bad
* value (a malformed link, a legacy/corrupted storage entry) never selects a non-existent playlist. */
export function parsePositiveIntId(raw: string | null | undefined): number | null {
if (raw == null || raw === "") return null;
const n = Number(raw);
return Number.isInteger(n) && n > 0 ? n : null;
}