fix(managers): two-way menu flip + extract useCardPager
- 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.
This commit is contained in:
@@ -6,7 +6,7 @@ import { api, type DiscoveredChannel } from "../lib/api";
|
||||
import { accountKey, LS, readAccount, writeAccount } from "../lib/storage";
|
||||
import { formatCountOrDash } from "../lib/format";
|
||||
import { parseSortState, sortRows, type SortState } from "../lib/columnSort";
|
||||
import { CARD_PAGE_SIZES, parseCardSize } from "../lib/channelLayout";
|
||||
import { useCardPager } from "../lib/useCardPager";
|
||||
import { subsColumn } from "./channelColumns";
|
||||
import { notify } from "../lib/notifications";
|
||||
import { notifyYouTubeActionError } from "../lib/youtubeErrors";
|
||||
@@ -49,11 +49,6 @@ export default function ChannelDiscovery({
|
||||
setSortState(s);
|
||||
writeAccount(LS.channelDiscoverySort, s);
|
||||
};
|
||||
// Card layout pagination (its own; the table keeps its rows-per-page inside DataTable).
|
||||
const [cardSize, setCardSize] = useState(() =>
|
||||
parseCardSize(readAccount(LS.channelDiscoveryCardSize, null)),
|
||||
);
|
||||
const [cardPage, setCardPage] = useState(0);
|
||||
|
||||
const query = useQuery({
|
||||
queryKey: ["discovered-channels"],
|
||||
@@ -98,13 +93,6 @@ export default function ChannelDiscovery({
|
||||
const rows = (query.data ?? []).filter(
|
||||
(c) => !q || `${c.title ?? ""} ${c.handle ?? ""}`.toLowerCase().includes(q)
|
||||
);
|
||||
// Changing what you're looking at starts over at the first page, as the table does on its own
|
||||
// filter/sort change. NOT keyed on the row count: subscribing drops a row from this tab, and being
|
||||
// thrown to page 1 for it would punish the very action you just took. A list that shrank past your
|
||||
// page is handled by committing the clamp instead (below).
|
||||
useEffect(() => {
|
||||
setCardPage(0);
|
||||
}, [search, sort]);
|
||||
|
||||
const columns: Column<DiscoveredChannel>[] = [
|
||||
// Actions first, mirroring the Subscriptions table (where it sits right after Prio) so the two
|
||||
@@ -201,16 +189,17 @@ export default function ChannelDiscovery({
|
||||
// copy+sort the whole list every render for a result nothing reads.
|
||||
const isCards = layout === "cards";
|
||||
const sortedRows = isCards ? sortRows(rows, columns, sort) : rows;
|
||||
const cardTotalPages = cardSize <= 0 ? 1 : Math.max(1, Math.ceil(sortedRows.length / cardSize));
|
||||
const cardSafePage = Math.min(cardPage, cardTotalPages - 1);
|
||||
// Commit the clamp when the list shrinks past your page, so it can't spring back later.
|
||||
const { paged: pagedRows, pagerProps, resetPage } = useCardPager(
|
||||
sortedRows,
|
||||
LS.channelDiscoveryCardSize,
|
||||
isCards,
|
||||
);
|
||||
// Changing what you're looking at starts over at the first page. NOT keyed on the row count:
|
||||
// subscribing drops a row from this tab, and being thrown to page 1 for it would punish the very
|
||||
// action you just took — a list that shrank past your page is handled by the hook's clamp.
|
||||
useEffect(() => {
|
||||
if (cardPage > cardTotalPages - 1) setCardPage(cardTotalPages - 1);
|
||||
}, [cardPage, cardTotalPages]);
|
||||
const pagedRows =
|
||||
!isCards || cardSize <= 0
|
||||
? sortedRows
|
||||
: sortedRows.slice(cardSafePage * cardSize, cardSafePage * cardSize + cardSize);
|
||||
resetPage();
|
||||
}, [search, sort, resetPage]);
|
||||
|
||||
return (
|
||||
<>
|
||||
@@ -221,19 +210,7 @@ export default function ChannelDiscovery({
|
||||
{/* Cards has no column headers, so a dropdown drives the sort + a pager (table uses its own). */}
|
||||
{layout === "cards" && (
|
||||
<div className="flex items-center gap-3 shrink-0">
|
||||
<Pager
|
||||
page={cardSafePage}
|
||||
size={cardSize}
|
||||
total={sortedRows.length}
|
||||
pageSizeOptions={CARD_PAGE_SIZES}
|
||||
onPageChange={setCardPage}
|
||||
onSizeChange={(s) => {
|
||||
setCardSize(s);
|
||||
writeAccount(LS.channelDiscoveryCardSize, s);
|
||||
setCardPage(0);
|
||||
}}
|
||||
sizeLabel={t("channels.cardsPerPage")}
|
||||
/>
|
||||
<Pager {...pagerProps} />
|
||||
<ColumnSortControl
|
||||
columns={columns}
|
||||
sort={sort}
|
||||
|
||||
@@ -19,7 +19,7 @@ import {
|
||||
} from "lucide-react";
|
||||
import { api, type ManagedChannel, type Tag } from "../lib/api";
|
||||
import { notifyYouTubeActionError } from "../lib/youtubeErrors";
|
||||
import { accountKey, LS, readAccount, writeAccount } from "../lib/storage";
|
||||
import { accountKey, LS } from "../lib/storage";
|
||||
import { useDismiss } from "../lib/useDismiss";
|
||||
import { useScrollFade } from "../lib/useScrollFade";
|
||||
import { formatEta, formatTotalHours, relativeTime } from "../lib/format";
|
||||
@@ -32,13 +32,9 @@ import Overlay from "./Overlay";
|
||||
import ViewSwitcher, { type ViewOption } from "./ViewSwitcher";
|
||||
import ColumnSortControl from "./ColumnSortControl";
|
||||
import Pager from "./Pager";
|
||||
import {
|
||||
CARD_PAGE_SIZES,
|
||||
CHANNEL_LAYOUTS,
|
||||
parseCardSize,
|
||||
type ChannelLayout,
|
||||
} from "../lib/channelLayout";
|
||||
import { CHANNEL_LAYOUTS, type ChannelLayout } from "../lib/channelLayout";
|
||||
import { sortRows } from "../lib/columnSort";
|
||||
import { useCardPager } from "../lib/useCardPager";
|
||||
import { PageToolbar } from "./PageShell";
|
||||
import ChannelLink from "./ChannelLink";
|
||||
import AboutCell from "./AboutCell";
|
||||
@@ -217,10 +213,6 @@ export default function Channels() {
|
||||
// Channel-name search (from the shared top SearchBar, via props) + tag-chip filtering, applied
|
||||
// client-side over the status-filtered list.
|
||||
const [tagFilter, setTagFilter] = useState<number[]>([]);
|
||||
// Card layout pagination (the table keeps its own inside DataTable). Size persists per account;
|
||||
// the page is transient.
|
||||
const [cardSize, setCardSize] = useState(() => parseCardSize(readAccount(LS.channelCardSize, null)));
|
||||
const [cardPage, setCardPage] = useState(0);
|
||||
// A focus-channel intent (header "without full history" / tag manager) seeds the search box.
|
||||
useEffect(() => {
|
||||
if (focusChannelName) setSearch(focusChannelName);
|
||||
@@ -253,14 +245,6 @@ export default function Channels() {
|
||||
if (tagFilter.length && !tagFilter.some((id) => c.tag_ids.includes(id))) return false;
|
||||
return true;
|
||||
});
|
||||
// Changing what you're looking at starts over at the first page, the way DataTable resets its own
|
||||
// page on a filter/sort change. Deliberately NOT keyed on the row count: the ["channels"] query
|
||||
// refetches on window focus and after every sync, so a catalogue that merely gained a channel would
|
||||
// otherwise yank you off page 7 for something you didn't do. A list that shrank under you is handled
|
||||
// by committing the clamp instead (see below).
|
||||
useEffect(() => {
|
||||
setCardPage(0);
|
||||
}, [statusFilter, search, tagFilter, sort]);
|
||||
const s = statusQuery.data;
|
||||
|
||||
const onView = (c: ManagedChannel) =>
|
||||
@@ -538,33 +522,20 @@ export default function Channels() {
|
||||
// too would copy+sort the whole ~300-row array on every render for a result nothing reads.
|
||||
const isCards = layout === "cards";
|
||||
const sortedChannels = isCards ? sortRows(visibleChannels, columns, sort) : visibleChannels;
|
||||
const cardTotalPages = cardSize <= 0 ? 1 : Math.max(1, Math.ceil(sortedChannels.length / cardSize));
|
||||
const cardSafePage = Math.min(cardPage, cardTotalPages - 1);
|
||||
// When the list shrinks under you the clamp above only HIDES the out-of-range page — commit it, or
|
||||
// it springs back the moment the list grows again. Keeps your place otherwise (unlike a blanket
|
||||
// reset), because it only fires when the page you're on no longer exists.
|
||||
useEffect(() => {
|
||||
if (cardPage > cardTotalPages - 1) setCardPage(cardTotalPages - 1);
|
||||
}, [cardPage, cardTotalPages]);
|
||||
const pagedCards =
|
||||
!isCards || cardSize <= 0
|
||||
? sortedChannels
|
||||
: sortedChannels.slice(cardSafePage * cardSize, cardSafePage * cardSize + cardSize);
|
||||
const cardPager = (
|
||||
<Pager
|
||||
page={cardSafePage}
|
||||
size={cardSize}
|
||||
total={sortedChannels.length}
|
||||
pageSizeOptions={CARD_PAGE_SIZES}
|
||||
onPageChange={setCardPage}
|
||||
onSizeChange={(s) => {
|
||||
setCardSize(s);
|
||||
writeAccount(LS.channelCardSize, s);
|
||||
setCardPage(0);
|
||||
}}
|
||||
sizeLabel={t("channels.cardsPerPage")}
|
||||
/>
|
||||
const { paged: pagedCards, pagerProps, resetPage } = useCardPager(
|
||||
sortedChannels,
|
||||
LS.channelCardSize,
|
||||
isCards,
|
||||
);
|
||||
// Changing what you're looking at starts over at the first page, the way DataTable resets its own
|
||||
// page on a filter/sort change. Deliberately NOT keyed on the row count: the ["channels"] query
|
||||
// refetches on window focus and after every sync, so a catalogue that merely gained a channel would
|
||||
// otherwise yank you off page 7 for something you didn't do — a list that shrank past your page is
|
||||
// handled by the hook's clamp instead.
|
||||
useEffect(() => {
|
||||
resetPage();
|
||||
}, [statusFilter, search, tagFilter, sort, resetPage]);
|
||||
const cardPager = <Pager {...pagerProps} />;
|
||||
|
||||
const tabs = (
|
||||
<div className="px-4 pt-4 max-w-[96rem] mx-auto flex items-center justify-between gap-4 flex-wrap">
|
||||
@@ -891,10 +862,13 @@ function TagsCell({
|
||||
const r = btnRef.current?.getBoundingClientRect();
|
||||
if (!open || !pop || !coords || !r) return;
|
||||
const h = pop.offsetHeight; // the width is read inside coordsFrom, off the same node
|
||||
const flip =
|
||||
coords.top !== undefined
|
||||
? coords.top + h > window.innerHeight - 8 && r.top - h - 4 > 8 // overflows below, fits above
|
||||
: true; // already flipped — keep it there
|
||||
const fitsBelow = r.bottom + 4 + h <= window.innerHeight - 8;
|
||||
const fitsAbove = r.top - 4 - h >= 8;
|
||||
// Decide BOTH ways, every time. Prefer the side it is already on (no flapping while scrolling),
|
||||
// but leave it as soon as that side stops fitting and the other one does — a one-way flip would
|
||||
// keep an above-anchored menu above a row scrolling toward the top and push it off the screen.
|
||||
const above = coords.bottom !== undefined;
|
||||
const flip = above ? fitsAbove || !fitsBelow : !fitsBelow && fitsAbove;
|
||||
const next = coordsFrom(r, flip);
|
||||
if (next.left !== coords.left || next.top !== coords.top || next.bottom !== coords.bottom) {
|
||||
setCoords(next);
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
import { useCallback, useEffect, useState } from "react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { CARD_PAGE_SIZES, parseCardSize } from "./channelLayout";
|
||||
import { readAccount, writeAccount } from "./storage";
|
||||
|
||||
// Client-side pagination for the channel manager's card layout, owned once instead of twice. Both
|
||||
// manager tabs page an in-memory list the same way, and keeping two copies meant every fix landed
|
||||
// twice (and the day one copy was missed, the tabs would page differently). The caller keeps only
|
||||
// what genuinely differs: which rows, which storage key, and when to jump back to page one.
|
||||
//
|
||||
// The page size persists per account; the page itself is transient. `enabled` is the card layout
|
||||
// flag — in table mode DataTable does its own paging, so this does no work and holds still.
|
||||
export function useCardPager<T>(rows: T[], sizeKey: string, enabled: boolean) {
|
||||
const { t } = useTranslation();
|
||||
const [size, setSize] = useState(() => parseCardSize(readAccount(sizeKey, null)));
|
||||
const [page, setPage] = useState(0);
|
||||
|
||||
const totalPages = size <= 0 ? 1 : Math.max(1, Math.ceil(rows.length / size));
|
||||
const safePage = Math.min(page, totalPages - 1);
|
||||
|
||||
// A list that shrank under you (an unsubscribe, a refetch) leaves `page` out of range: the clamp
|
||||
// above only HIDES that, so commit it or it springs back the moment the list grows again. Only
|
||||
// fires when the page you are on stopped existing, so it never costs you your place otherwise.
|
||||
useEffect(() => {
|
||||
if (enabled && page > totalPages - 1) setPage(totalPages - 1);
|
||||
}, [enabled, page, totalPages]);
|
||||
|
||||
const paged =
|
||||
!enabled || size <= 0 ? rows : rows.slice(safePage * size, safePage * size + size);
|
||||
|
||||
return {
|
||||
paged,
|
||||
/** Spread onto <Pager/>. */
|
||||
pagerProps: {
|
||||
page: safePage,
|
||||
size,
|
||||
total: rows.length,
|
||||
pageSizeOptions: CARD_PAGE_SIZES,
|
||||
onPageChange: setPage,
|
||||
onSizeChange: (s: number) => {
|
||||
setSize(s);
|
||||
writeAccount(sizeKey, s);
|
||||
setPage(0);
|
||||
},
|
||||
sizeLabel: t("channels.cardsPerPage"),
|
||||
},
|
||||
/** Call when the caller's own filters change — a different list starts at page one. */
|
||||
resetPage: useCallback(() => setPage(0), []),
|
||||
};
|
||||
}
|
||||
Reference in New Issue
Block a user