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

fix(nav): the address keeps what the app rewrites around it (review round 3)

Browse Source 
- The Feed press over a live search ends only the search: the feed's own view stays named.
- Leaving a search steps back only off an entry the app pushed (_ytOwn); one a link or a reload
  opened is cleared in place, also where the Navigation API is missing.
- useUrlMirror rewrites a changed view through replaceView, so a reload's overlay marks go; its
  first write and its landing writes keep them for an overlay that reopens.
- Files and Playlists: a box emptied and filled again settles at once, never naming the ended search.
- Playlists: a linked player is scoped to the playlist the store started with, and only the rail's
  own move to the first playlist is no pick.
- Contract: writer objects are scanned to their matching brace, a computed or shorthand key is
  refused, and every useHistorySubview call hands a named definition.
- Lint: playwright/no-skipped-test also refuses fixme (gate snapshot regenerated).
- FEED_DEFAULT_SORT at the three remaining sites; linkedNotListed says to search by name.
This commit is contained in:
peter
2026-10-27 23:33:22 +01:00
parent 66e427dfa4
commit 02906d0acb
18 changed files with 282 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/modules/feed/Feed.tsx
+3 -2
View File
@@ -24,6 +24,7 @@ import {
import { YouTubeIcon } from "../../lib/brandIcons";
import {
api,
FEED_DEFAULT_SORT,
HttpError,
refusalReason,
type FeedFilters,
@@ -318,7 +319,7 @@ export default function Feed({
const qEmpty = !filters.q.trim();
useEffect(() => {
if (qEmpty && parseSort(filters.sort).key === "relevance") {
setFilters({ ...filters, sort: buildSort("date", "desc") });
setFilters({ ...filters, sort: FEED_DEFAULT_SORT });
}
// Naming `filters` is safe precisely because the write below falsifies the condition above: the
// sort it sets is "date", so the one re-run it causes does nothing. The old `[qEmpty]` was
@@ -624,7 +625,7 @@ export default function Feed({
.catch((e: unknown) =>
notifyActionError(e, i18n.t("errors.writeFailed.clearSearch")),
);
setFilters({ ...filters, librarySource: "organic", sort: "newest" });
setFilters({ ...filters, librarySource: "organic", sort: FEED_DEFAULT_SORT });
onExitYtSearch();
dropFeedCaches();
qc.removeQueries({ queryKey: qk.ytSearch() });
frontend/src/modules/feed/FeedHeader.tsx
+3 -2
View File
@@ -1,4 +1,5 @@
import { useTranslation } from "react-i18next";
import { FEED_DEFAULT_SORT } from "../../lib/api";
import { YouTubeIcon } from "../../lib/brandIcons";
import { useMe } from "../../lib/useMe";
import { PageHeader } from "../../components/ui/PageShell";
@@ -25,9 +26,9 @@ export default function FeedHeader() {
placeholder={t("header.searchPlaceholder")}
onChange={(q) => {
// When a search first appears, rank the feed by relevance — set atomically with the
// query (race-free vs. per-keystroke updates). Only overrides the default "newest"
// query (race-free vs. per-keystroke updates). Only overrides the default
// sort; a custom sort the user chose is left alone. Cleared in Feed.
const startSearch = !filters.q.trim() && !!q.trim() && filters.sort === "newest";
const startSearch = !filters.q.trim() && !!q.trim() && filters.sort === FEED_DEFAULT_SORT;
setFilters({ ...filters, q, ...(startSearch ? { sort: "relevance" } : {}) });
}}
// Turned off on this instance, the box only filters: no Go that could only be refused.
frontend/src/modules/files/FilesProvider.tsx
+4 -4
View File
@@ -102,10 +102,10 @@ export function FilesProvider({ children }: { children: ReactNode }) {
const shown = urlPage(nav) === "files";
// Written once the typing pauses (one address-bar write per keystroke is churn a browser throttles);
// an emptied box at once — an arrival's, or the page left: back on it within the pause, the address
// would name the ended search again.
const debounced = useDebounced(search.trim(), 300, focusRaw?.token);
const typed = search.trim() ? debounced : "";
// a box emptied — an arrival's, or the page left — and filled again, each at once: the pause would
// name the ended search again, on its way out or before the new text settled.
const q = search.trim();
const typed = useDebounced(q, 300, `${focusRaw?.token ?? ""}:${q === ""}`);
useUrlMirror(
filesListParams(
shown ? { search: typed, folder: viewParam(view), item: focus ? focus.id : null } : null,
frontend/src/modules/playlists/Playlists.dom.test.tsx
+49 -1
View File
@@ -12,6 +12,8 @@ const h = vi.hoisted(() => ({
notify: vi.fn(),
confirm: vi.fn(async () => true),
selectedId: 1 as number | null,
// The selection the store started with (the address's, on a reload or a link).
linkedId: 1 as number | null,
// Who reads the selection, so a test's change re-renders them the way the real store does.
selectionReaders: new Set<() => void>(),
mounted: false,
@@ -49,6 +51,7 @@ vi.mock("./playlistsContext", async () => {
return {
usePlaylists: () => ({
selectedId: useSyncExternalStore(subscribe, () => h.selectedId),
linkedId: h.linkedId,
search: "",
}),
usePlaylistsActions: () => ({ setSearch: () => {} }),
@@ -136,8 +139,17 @@ function mount(
{
strictOverCache = false,
detail,
}: { strictOverCache?: boolean; detail?: (id: number) => Promise<unknown> } = {},
linked = h.selectedId,
listIn = false,
}: {
strictOverCache?: boolean;
detail?: (id: number) => Promise<unknown>;
linked?: number | null;
/** The account's list already in, as the rail that fetched it leaves it. */
listIn?: boolean;
} = {},
) {
h.linkedId = linked;
vi.spyOn(api, "playlists").mockResolvedValue(
Object.entries(lists).map(([id, items]) => list(Number(id), items)) as never,
);
@@ -146,6 +158,11 @@ function mount(
);
qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
qc.setQueryData(qk.me(), { id: 1, can_write: true, prefs: {}, preferences: prefs });
if (listIn)
qc.setQueryData(
qk.playlists(),
Object.entries(lists).map(([id, items]) => list(Number(id), items)),
);
// Coming back to the page: the selected list is already cached, and StrictMode (dev) runs its
// effects twice on that mount.
if (strictOverCache && h.selectedId != null)
@@ -248,6 +265,37 @@ describe("Playlists — a player the address names", () => {
expect(new URLSearchParams(window.location.search).get("watch")).toBe("Z");
});
it("plays alone over a playlist this account does not hold, the rail moved on before the page came", async () => {
window.history.replaceState(null, "", "/?page=playlists&pl=1&watch=Z");
// The rail's keep-valid step ran before the page mounted: the selection is already 1, and 1 even
// holds the video — it is still not the playlist the link named.
h.selectedId = 1;
vi.spyOn(api, "videoRow").mockResolvedValue(vid("Z", 0));
mount({ 1: [vid("A", 0), vid("Z", 0)] }, {}, { linked: 7, listIn: true });
await waitFor(() => expect(h.player?.video.id).toBe("Z"));
expect(h.player!.queue?.map((v) => v.id)).toEqual(["Z"]);
});
it("gives up over a playlist this account does not hold once another one is picked", async () => {
window.history.replaceState(null, "", "/?page=playlists&pl=7&watch=Z");
h.selectedId = 7;
let answer: (v: Video) => void = () => {};
vi.spyOn(api, "videoRow").mockReturnValue(new Promise((r) => (answer = r)) as never);
const { rerender } = mount(
{ 1: [vid("A", 0)], 2: [vid("B", 0)] },
{},
{ detail: async (id) => (id === 7 ? new Promise<never>(() => {}) : list(id, [])) },
);
await waitFor(() => expect(qc.getQueryData(qk.playlists())).toBeDefined());
h.selectedId = 1; // the rail's own move
rerender();
h.selectedId = 2; // a pick, before the video's row came
rerender();
act(() => answer(vid("Z", 0)));
await new Promise((r) => setTimeout(r, 20));
expect(h.player).toBeNull();
});
it("plays alone when the linked playlist's read is refused", async () => {
window.history.replaceState(null, "", "/?page=playlists&pl=1&watch=Z");
vi.spyOn(api, "videoRow").mockResolvedValue(vid("Z", 0));
frontend/src/modules/playlists/Playlists.tsx
+8 -5
View File
@@ -192,7 +192,7 @@ export default function Playlists() {
function PlaylistDetail() {
// The selected playlist lives in PlaylistsProvider, shared with the PlaylistsRail (which owns the
// list + its rail controls). This detail pane reacts to the selection.
const { selectedId } = usePlaylists();
const { selectedId, linkedId } = usePlaylists();
const { t, i18n } = useTranslation();
const lang = i18n.language;
const qc = useQueryClient();
@@ -586,11 +586,14 @@ function PlaylistDetail() {
// (filtered out, gone from the list, the list emptied or out of reach) plays alone, and nothing
// removes it from a list. In: the order seeded from it (an empty one has none to wait for), or its
// read refused. Scoped to the playlist it was linked over: another one picked first gives it up.
// One the account's list does not hold (deleted since, another account's link) is out of reach
// as soon as that list is in: the rail then moves to another one by itself, which is no pick.
const [linkedOver] = useState(selectedId);
// Linked over the playlist the store started with, not the selection this page mounted on: the rail
// may have moved that already. One the account's list does not hold (deleted since, another
// account's link) is out of reach as soon as that list is in: the rail then moves to the first one
// by itself, which is no pick — any other is.
const linkedOver = linkedId;
const linkedOutOfReach =
listQuery.data != null && !listQuery.data.some((p) => p.id === linkedOver);
const railMovedTo = listQuery.data?.[0]?.id ?? null;
const linkedListIn =
linkedOutOfReach ||
(detail != null && (detail.items.length === 0 || items.length > 0)) ||
@@ -605,7 +608,7 @@ function PlaylistDetail() {
setPlaying({ video, queue: [video], index: 0, listId, editable: false });
}
},
linkedOutOfReach ? linkedOver : selectedId,
linkedOutOfReach && selectedId === railMovedTo ? linkedOver : selectedId,
);
async function moveItemTo(video: Video, target: Playlist) {
frontend/src/modules/playlists/PlaylistsProvider.tsx
+9 -4
View File
@@ -31,6 +31,7 @@ export function PlaylistsProvider({ children }: { children: ReactNode }) {
() => new URLSearchParams(window.location.search).get("plq") ?? "",
);
const [selectedId, setSelectedIdState] = useState<number | null>(readSelectedId);
const [linkedId] = useState<number | null>(readSelectedId);
const setSearch = useCallback((q: string) => setSearchState(q), []);
const setSelectedId = useCallback((id: number | null) => {
@@ -46,12 +47,16 @@ export function PlaylistsProvider({ children }: { children: ReactNode }) {
const shown = urlPage(nav) === "playlists";
// Leaving the page ends the search: adjusted during the render that leaves it.
if (nav.page !== "playlists" && search) setSearchState("");
// An emptied box at once: back on the page within the pause, the address would name it again.
const debounced = useDebounced(search.trim(), 300);
const typed = search.trim() ? debounced : "";
// A box emptied and filled again, each at once: the pause would name the ended search again, on
// its way out or before the new text settled.
const q = search.trim();
const typed = useDebounced(q, 300, q === "");
useUrlMirror(shown ? playlistsParams(selectedId, typed) : playlistsParams(null, ""));
const state = useMemo<PlaylistsState>(() => ({ search, selectedId }), [search, selectedId]);
const state = useMemo<PlaylistsState>(
() => ({ search, selectedId, linkedId }),
[search, selectedId, linkedId],
);
const actions = useMemo<PlaylistsActions>(
() => ({ setSearch, setSelectedId }),
[setSearch, setSelectedId],
frontend/src/modules/playlists/playlistsContext.ts
+4
View File
@@ -6,6 +6,9 @@ import { createContext, useContext } from "react";
export type PlaylistsState = {
search: string;
selectedId: number | null;
/** The selection the store started with — the playlist a reload or a link named. A player the
* address names opens over that one: the rail may have moved the selection before the page came. */
linkedId: number | null;
};
export interface PlaylistsActions {
@@ -16,6 +19,7 @@ export interface PlaylistsActions {
export const PlaylistsStateContext = createContext<PlaylistsState>({
search: "",
selectedId: null,
linkedId: null,
});
export const PlaylistsActionsContext = createContext<PlaylistsActions>({
setSearch: () => {},