fix(feed): deep-review remediations for the filter-bar overhaul

Correctness / behaviour:
- Move the Personal Views modal from inside the collapsible filter rail to the
  feed page (moduleRegistry FeedPage), so opening it from the always-visible
  toolbar view chip works while the rail is unpinned (its default) instead of
  no-op'ing and latching modalOpen into a later stale pop-open.
- Migrate a persisted legacy global `tagMode` into per-category `tagModes` on the
  localStorage seed paths too (loadAccountFilters), matching the saved-view and
  URL paths — an upgrading user's topic any/all no longer silently reverts to 'or'.
- DateFilterSelect: reset the custom reveal on an external clear while the popover
  is closed (guard on !open, not maxAgeDays), so the trigger reverts to "Any time"
  instead of lingering on "Custom range" with empty fields.
- PersonalViewsModal delete: only advance the detail selection when the delete
  actually happened (remove() now resolves a boolean) — cancelling the confirm no
  longer switches the selected view.
- SavedViewsProvider: prefer the explicitly-applied (sticky) view among several
  that capture identical filters, so apply(B) isn't re-attributed to an earlier
  duplicate A (wrong "modified" chip name).

A11y:
- Give the clickable panel title a descriptive accessible name (titleActionLabel,
  wired to "Personal views") instead of announcing only "Filters".
- Give the "+N" overflow pill an aria-label ("N more filters").
- Use t("views.reorder") for the saved-view drag handle instead of a hardcoded
  English "reorder" literal.

Also extract the chip-fit decision into a pure, testable lib/chipOverflow.ts and
measure the inter-chip gap live (rem-safe) instead of a frozen 6px constant.
This commit is contained in:
2026-08-22 06:33:48 +02:00
parent dd76e9dcf7
commit e90603d4db
12 changed files with 156 additions and 66 deletions
+29 -33
View File
@@ -2,6 +2,7 @@ import { useLayoutEffect, useRef, useState } from "react";
import { useTranslation } from "react-i18next";
import { Filter, X } from "lucide-react";
import type { ActiveFilter } from "../lib/useActiveFilters";
import { decideChipLayout } from "../lib/chipOverflow";
import { PopoverItem, PopoverSurface, useAnchoredPopover } from "./AnchoredPopover";
// The applied-filter row above the feed: every narrowing currently in force, each removable on its
@@ -18,7 +19,6 @@ import { PopoverItem, PopoverSurface, useAnchoredPopover } from "./AnchoredPopov
// Below this lane width an overflow collapses all the way to a single count pill (mobile), instead of
// the "fit some + N more" treatment used on wider lanes.
const NARROW_PX = 420;
const GAP_PX = 6; // matches gap-1.5 (px)
const chipCls =
"chip group inline-flex items-center gap-1 pl-2.5 pr-1.5 py-1 text-sm whitespace-nowrap hover:border-accent hover:bg-accent/20 transition";
@@ -43,11 +43,15 @@ function ChipButton({ f, testId }: { f: ActiveFilter; testId?: string }) {
// The "+N" / "N filters" pill and its popover listing the (overflowed / all) chips, each removable.
function OverflowPopover({
label,
ariaLabel,
chips,
onClearAll,
icon,
}: {
label: string;
/** Accessible name for the trigger + popover — describes what's inside (e.g. "3 more filters"),
* since the visible label can be a bare "+3". Defaults to `label`. */
ariaLabel?: string;
chips: ActiveFilter[];
onClearAll: () => void;
icon?: boolean;
@@ -60,12 +64,14 @@ function OverflowPopover({
mode: "menu",
placement: "bottom-end",
});
const name = ariaLabel ?? label;
return (
<>
<button
type="button"
ref={pop.refs.setReference}
{...pop.getReferenceProps()}
aria-label={name}
className={`chip inline-flex shrink-0 items-center gap-1 px-2.5 py-1 text-sm whitespace-nowrap transition ${
open ? "border-accent text-accent" : "hover:border-accent"
}`}
@@ -75,10 +81,10 @@ function OverflowPopover({
</button>
<PopoverSurface
popover={pop}
ariaLabel={label}
ariaLabel={name}
className="glass-menu min-w-48 max-w-72 rounded-xl p-1.5 shadow-lg animate-[popIn_0.16s_ease]"
>
<div role="group" aria-label={label} className="max-h-72 overflow-y-auto no-scrollbar">
<div role="group" aria-label={name} className="max-h-72 overflow-y-auto no-scrollbar">
{chips.map((f) => (
<PopoverItem
key={f.key}
@@ -141,40 +147,29 @@ export default function ActiveFilterChips({
if (!lane || !labelEl || n === 0) return;
const measure = () => {
const avail = lane.clientWidth;
const rightOf = (el: HTMLElement | null | undefined) =>
el ? el.offsetLeft + el.offsetWidth : 0;
const labelRight = rightOf(labelEl);
const chips = chipRefs.current.slice(0, n);
const plusW = plusRef.current?.offsetWidth ?? 0;
// "all" mode also renders a Clear all button after the chips (when n > 1), so reserve its width
// in the fit-all check — otherwise it spills past the lane into the Sort/View controls.
const clearW = n > 1 ? GAP_PX + (clearRef.current?.offsetWidth ?? 0) : 0;
const plusEl = plusRef.current;
// Measure the real rendered gap off the box between the last chip and the "+N" sample, so it
// scales with the text-size slider (gap-1.5 is rem) rather than being frozen in px.
const lastChip = chips[n - 1];
const gap = plusEl && lastChip ? Math.max(0, plusEl.offsetLeft - rightOf(lastChip)) : 6;
// "all" mode also renders a Clear all button after the chips (when n > 1), so reserve its width.
const clearW = n > 1 ? gap + (clearRef.current?.offsetWidth ?? 0) : 0;
// Do all chips (plus the Clear all button) fit? (last chip's right edge within the lane.)
const allRight = rightOf(chips[n - 1]);
if (allRight + clearW <= avail) {
setLayout({ mode: "all", visible: n });
return;
}
// Overflow. On a narrow lane, collapse everything into one count pill.
if (avail < NARROW_PX) {
setLayout({ mode: "count", visible: 0 });
return;
}
// Otherwise fit as many chips as leave room for the "+N" pill after them.
let k = 0;
for (let i = 0; i < n; i++) {
const right = rightOf(chips[i]);
if (right + GAP_PX + plusW <= avail) k = i + 1;
else break;
}
// If not even one chip fits beside the pill, fall back to the count pill.
if (k === 0 && labelRight + GAP_PX + plusW > avail) {
setLayout({ mode: "count", visible: 0 });
return;
}
setLayout({ mode: "some", visible: k });
setLayout(
decideChipLayout({
n,
labelRight: rightOf(labelEl),
chipRights: chips.map((c) => rightOf(c)),
plusW: plusEl?.offsetWidth ?? 0,
clearW,
avail: lane.clientWidth,
gap,
narrowPx: NARROW_PX,
}),
);
};
measure();
@@ -243,6 +238,7 @@ export default function ActiveFilterChips({
{layout.mode === "some" && hiddenCount > 0 && (
<OverflowPopover
label={`+${hiddenCount}`}
ariaLabel={t("feed.chips.moreCount", { count: hiddenCount })}
chips={overflowChips}
onClearAll={onClearAll}
/>
+6 -5
View File
@@ -32,12 +32,13 @@ export default function DateFilterSelect({
const [open, setOpen] = useState(false);
const [customOpen, setCustomOpen] = useState<boolean>(!!(filters.dateFrom || filters.dateTo));
// If a range is cleared elsewhere (the chip-row Clear all) while nothing was ever typed, drop the
// "custom" reveal so the trigger doesn't linger on "Custom range" with empty fields.
// If the range is cleared elsewhere (chip-row Clear all / removing the date chip) while this popover
// is closed, drop the custom reveal so the trigger reverts to "Any time" instead of lingering on
// "Custom range" with empty fields. Guarded on !open so it never closes a freshly-picked Custom the
// user hasn't filled in yet — that stays open until they close it (handled by onOpenChange below).
useEffect(() => {
if (!filters.dateFrom && !filters.dateTo && filters.maxAgeDays !== undefined)
setCustomOpen(false);
}, [filters.dateFrom, filters.dateTo, filters.maxAgeDays]);
if (!open && customOpen && !filters.dateFrom && !filters.dateTo) setCustomOpen(false);
}, [open, customOpen, filters.dateFrom, filters.dateTo]);
const key = deriveDateKey(filters, customOpen, presets);
@@ -9,7 +9,13 @@ import {
} from "react";
import { qk } from "../lib/queryKeys";
import { useQuery } from "@tanstack/react-query";
import { api, getActiveAccount, DEFAULT_TAG_MODES, type FeedFilters } from "../lib/api";
import {
api,
getActiveAccount,
coerceTagModes,
DEFAULT_TAG_MODES,
type FeedFilters,
} from "../lib/api";
import { filterParamUpdates, hasFilterParams, paramsToFilters } from "../lib/urlState";
import { useUrlMirror } from "../lib/useUrlMirror";
import { accountKey, LS, readJSON, readMerged, writeJSON } from "../lib/storage";
@@ -62,10 +68,19 @@ function loadAccountFilters(id: number | null): FeedFilters {
} catch {
/* localStorage may be unavailable */
}
if (hasStored && fKey) return readMerged(fKey, DEFAULT_FILTERS);
// Migrate a persisted legacy global `tagMode` into the per-category `tagModes` on every seed path
// (readMerged is a shallow merge that leaves the legacy scalar as ignored junk), matching what the
// saved-view (asFilters) and URL (paramsToFilters) paths already do.
if (hasStored && fKey) {
const merged = readMerged(fKey, DEFAULT_FILTERS);
return { ...merged, tagModes: coerceTagModes(merged) };
}
const dvKey = accountKey(LS.defaultViewFilters, id);
const def = dvKey ? readJSON<FeedFilters | null>(dvKey, null) : null;
if (def) return { ...DEFAULT_FILTERS, ...def };
if (def) {
const merged = { ...DEFAULT_FILTERS, ...def };
return { ...merged, tagModes: coerceTagModes(merged) };
}
return { ...DEFAULT_FILTERS };
}
+3
View File
@@ -25,6 +25,7 @@ export default function FilterRail({
collapsible = true,
iconExpanded,
onTitleClick,
titleActionLabel,
children,
}: {
kind: PanelId;
@@ -41,6 +42,7 @@ export default function FilterRail({
collapsible?: boolean;
iconExpanded?: ReactNode;
onTitleClick?: () => void;
titleActionLabel?: string;
children?: ReactNode;
}) {
const { t } = useTranslation();
@@ -59,6 +61,7 @@ export default function FilterRail({
actions={actions}
iconExpanded={iconExpanded}
onTitleClick={onTitleClick}
titleActionLabel={titleActionLabel}
>
<PanelGroups
items={items}
@@ -36,6 +36,7 @@ function ViewRow({
active: boolean;
onSelect: () => void;
}) {
const { t } = useTranslation();
const { attributes, listeners, setNodeRef, transform, transition, isDragging } = useSortable({
id: view.id,
});
@@ -50,7 +51,7 @@ function ViewRow({
<button
{...attributes}
{...listeners}
aria-label="reorder"
aria-label={t("views.reorder")}
className="cursor-grab p-1.5 text-muted hover:text-fg"
>
<GripVertical className="h-3.5 w-3.5" />
@@ -196,10 +197,11 @@ export default function PersonalViewsModal() {
{t("views.default")}
</button>
<button
onClick={() => {
onClick={async () => {
const next = views.find((v) => v.id !== selected.id);
void remove(selected);
setSelectedId(next?.id ?? null);
// Only move the selection if the delete actually happened (remove awaits a confirm
// and resolves false on cancel) — otherwise cancelling would still switch the pane.
if (await remove(selected)) setSelectedId(next?.id ?? null);
}}
title={t("views.delete")}
className="ml-auto inline-flex items-center gap-1.5 rounded-lg border border-border px-3 py-1.5 text-sm text-danger transition hover:border-danger"
+10 -7
View File
@@ -59,7 +59,8 @@ export interface SavedViewsContext {
create: (name: string) => Promise<boolean>;
rename: (id: number, name: string) => Promise<boolean>;
toggleDefault: (v: SavedView) => Promise<void>;
remove: (v: SavedView) => Promise<void>;
/** Delete a view after confirming; resolves true only when it was actually deleted (false on cancel/fail). */
remove: (v: SavedView) => Promise<boolean>;
reorder: (nextIds: number[]) => Promise<void>;
}
@@ -84,10 +85,12 @@ export function SavedViewsProvider({ children }: { children: ReactNode }) {
}, [viewsQuery.data]);
const currentKey = keyOf(filters);
const activeView = useMemo(
() => views.find((v) => keyOf(v.filters) === currentKey) ?? null,
[views, currentKey],
);
const activeView = useMemo(() => {
const matches = views.filter((v) => keyOf(v.filters) === currentKey);
// When several views capture identical filters, prefer the one the user explicitly applied
// (sticky) so apply(B) isn't silently re-attributed to an earlier duplicate A; else the first.
return matches.find((v) => v.id === stickyId) ?? matches[0] ?? null;
}, [views, currentKey, stickyId]);
// An exact match becomes the sticky selection, so filters that happen to equal a view (deep-link,
// manual, or an applied view) name that view in the chip and keep naming it after a later tweak.
useEffect(() => {
@@ -151,9 +154,9 @@ export function SavedViewsProvider({ children }: { children: ReactNode }) {
confirmLabel: t("views.delete"),
danger: true,
});
if (!ok) return;
if (!ok) return false;
if (stickyId === v.id) setStickyId(null);
await runViewMutation(() => api.deleteView(v.id), t("views.actionFailed"));
return runViewMutation(() => api.deleteView(v.id), t("views.actionFailed"));
},
reorder: async (nextIds) => {
// Optimistic reorder so the drop sticks visually before the round-trip; the refetch reverts to
+5
View File
@@ -22,6 +22,7 @@ export default function SidePanel({
actions,
iconExpanded,
onTitleClick,
titleActionLabel,
children,
}: {
title: string;
@@ -41,6 +42,8 @@ export default function SidePanel({
iconExpanded?: ReactNode;
// When set, the expanded-header title becomes a button (the feed opens its Personal Views modal).
onTitleClick?: () => void;
// Accessible name / tooltip describing what onTitleClick does (the plain title text alone doesn't).
titleActionLabel?: string;
children: ReactNode;
}) {
const { t } = useTranslation();
@@ -110,6 +113,8 @@ export default function SidePanel({
{onTitleClick ? (
<button
onClick={onTitleClick}
title={titleActionLabel}
aria-label={titleActionLabel}
className="flex-1 min-w-0 truncate text-left text-sm font-bold uppercase tracking-[0.12em] transition hover:text-accent"
>
{title}
+2 -3
View File
@@ -9,7 +9,6 @@ import { useDebounced } from "../lib/useDebounced";
import { useActiveFilters } from "../lib/useActiveFilters";
import TagManager from "./TagManager";
import SavedViewsPopover from "./SavedViewsPopover";
import PersonalViewsModal from "./PersonalViewsModal";
import { useSavedViews } from "./SavedViewsProvider";
import FilterRail from "./FilterRail";
import { type PanelItem } from "./PanelGroups";
@@ -47,7 +46,7 @@ export default function Sidebar({
}) {
const { t } = useTranslation();
const isDemo = useMe().is_demo;
const { modalOpen, openModal } = useSavedViews();
const { openModal } = useSavedViews();
const filters = useFeedFilters();
const { setFilters } = useFeedFiltersActions();
// "Focus this channel in the manager" (from the tag manager here) lives in ChannelsProvider.
@@ -206,11 +205,11 @@ export default function Sidebar({
) : undefined
}
onTitleClick={!isDemo && source === "feed" ? openModal : undefined}
titleActionLabel={!isDemo && source === "feed" ? t("views.manageTitle") : undefined}
>
{tagManagerOpen && (
<TagManager onClose={() => setTagManagerOpen(false)} onFocusChannel={onFocusChannel} />
)}
{modalOpen && source === "feed" && <PersonalViewsModal />}
</FilterRail>
);
}
+20 -11
View File
@@ -7,6 +7,8 @@ import { useFeedView, useFeedViewActions } from "./FeedViewProvider";
import { useNavigation, useNavigationActions } from "./NavigationProvider";
import Sidebar from "./Sidebar";
import PlaylistsRail from "./PlaylistsRail";
import PersonalViewsModal from "./PersonalViewsModal";
import { useSavedViews } from "./SavedViewsProvider";
// The RENDER half of the module registry: which component (and side rail) each page renders. The
// metadata half — nav order, label, icon, gate — lives in lib/modules.ts (imported by NavSidebar /
@@ -45,18 +47,25 @@ function FeedPage() {
const { setView } = useFeedViewActions();
const { ytSearch } = useNavigation();
const { enterYtSearch, exitYtSearch } = useNavigationActions();
// The Personal Views modal lives at the feed-page level (always mounted while on the feed, unmounts
// on navigation) rather than inside the collapsible filter rail — otherwise opening it from the
// always-visible toolbar view chip while the rail is unpinned would render nothing and latch open.
const { modalOpen } = useSavedViews();
return (
<Feed
filters={filters}
setFilters={setFilters}
view={view}
setView={setView}
canRead={me.can_read}
isDemo={me.is_demo}
ytSearch={ytSearch}
onYtSearch={enterYtSearch}
onExitYtSearch={exitYtSearch}
/>
<>
<Feed
filters={filters}
setFilters={setFilters}
view={view}
setView={setView}
canRead={me.can_read}
isDemo={me.is_demo}
ytSearch={ytSearch}
onYtSearch={enterYtSearch}
onExitYtSearch={exitYtSearch}
/>
{modalOpen && !me.is_demo && <PersonalViewsModal />}
</>
);
}
+2
View File
@@ -5,6 +5,8 @@
"contentNone": "No content types",
"count_one": "{{count}} filter",
"count_other": "{{count}} filters",
"moreCount_one": "{{count}} more filter",
"moreCount_other": "{{count}} more filters",
"maxAge_one": "Last {{count}} day",
"maxAge_other": "Last {{count}} days"
},
+2
View File
@@ -5,6 +5,8 @@
"contentNone": "Nincs tartalomtípus",
"count_one": "{{count}} szűrő",
"count_other": "{{count}} szűrő",
"moreCount_one": "még {{count}} szűrő",
"moreCount_other": "még {{count}} szűrő",
"maxAge_one": "Utolsó {{count}} nap",
"maxAge_other": "Utolsó {{count}} nap"
},
+53
View File
@@ -0,0 +1,53 @@
// Pure fit decision for a single-line chip lane that must share its width with fixed trailing
// controls (the feed's applied-filter row + Sort/View). Lifted out of ActiveFilterChips so the
// all/some/count math is unit-testable without a layout engine (jsdom reports 0-width boxes), the
// same split as lib/dateFilter and lib/columnSort. The caller measures real widths off a hidden
// mirror (including the live rem gap, so nothing is frozen in px) and hands them here.
//
// A sibling one-line-fit measurer lives in ChannelTags (ChannelTagsControl); if a third appears,
// promote the DOM-measurement half to a shared hook and keep this as its policy core.
export interface ChipFitInput {
/** Number of chips. */
n: number;
/** Right edge (offsetLeft + offsetWidth) of the leading label, in the measurer. */
labelRight: number;
/** Right edge of each chip, in the measurer (index-aligned to the chips). */
chipRights: number[];
/** Width of the "+N" overflow pill. */
plusW: number;
/** Width the trailing "Clear all" button needs INCLUDING its leading gap (0 when n <= 1, since it
* only renders with more than one chip). Reserved only in the "all" branch, which renders it. */
clearW: number;
/** Available lane width. */
avail: number;
/** The real (rem-scaled) inter-item gap. */
gap: number;
/** At or below this lane width an overflow collapses to a single count pill instead of "+N". */
narrowPx: number;
}
export type ChipFit = { mode: "all" | "some" | "count"; visible: number };
export function decideChipLayout(p: ChipFitInput): ChipFit {
const { n, labelRight, chipRights, plusW, clearW, avail, gap, narrowPx } = p;
if (n <= 0) return { mode: "all", visible: 0 };
// Do all chips (plus the Clear all button it renders) fit? Use the last chip's right edge.
const allRight = chipRights[n - 1] ?? labelRight;
if (allRight + clearW <= avail) return { mode: "all", visible: n };
// Overflow. On a narrow lane, collapse everything into one count pill.
if (avail < narrowPx) return { mode: "count", visible: 0 };
// Otherwise fit as many chips as leave room for the "+N" pill after them.
let k = 0;
for (let i = 0; i < n; i++) {
const right = chipRights[i] ?? 0;
if (right + gap + plusW <= avail) k = i + 1;
else break;
}
// If not even one chip fits beside the pill, fall back to the count pill.
if (k === 0 && labelRight + gap + plusW > avail) return { mode: "count", visible: 0 };
return { mode: "some", visible: k };
}