diff --git a/frontend/src/components/ActiveFilterChips.tsx b/frontend/src/components/ActiveFilterChips.tsx index cfc909c7..28e42875 100644 --- a/frontend/src/components/ActiveFilterChips.tsx +++ b/frontend/src/components/ActiveFilterChips.tsx @@ -1,8 +1,9 @@ -import { useLayoutEffect, useRef, useState } from "react"; +import { 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 { useChipOverflowFit } from "../lib/useChipOverflowFit"; import { PopoverItem, PopoverSurface, useAnchoredPopover } from "./AnchoredPopover"; // The applied-filter row above the feed: every narrowing currently in force, each removable on its @@ -131,52 +132,36 @@ export default function ActiveFilterChips({ const chipRefs = useRef>([]); const plusRef = useRef(null); const clearRef = useRef(null); - // How the row is currently laid out: all chips, a fit-count + "+N", or a single count pill. - const [layout, setLayout] = useState<{ mode: "all" | "some" | "count"; visible: number }>({ - mode: "all", - visible: filters.length, - }); const n = filters.length; // Re-measure whenever the chip set changes (labels or count) — captures both in one dep. const chipsKey = filters.map((f) => f.key + f.label).join("|"); - useLayoutEffect(() => { - const lane = laneRef.current; - const labelEl = labelRef.current; - if (!lane || !labelEl || n === 0) return; - - const measure = () => { - const rightOf = (el: HTMLElement | null | undefined) => - el ? el.offsetLeft + el.offsetWidth : 0; - const chips = chipRefs.current.slice(0, n); - 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; - - 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(); - const ro = new ResizeObserver(measure); - ro.observe(lane); - return () => ro.disconnect(); - }, [n, chipsKey]); + // The shared measurement core hands over the chip right edges + gap + pill width + lane width; this + // policy adds the feed row's own framing (the leading label's right edge, and the trailing "Clear + // all" button's width, reserved only when it renders — n > 1) before deciding all/some/count. + const layout = useChipOverflowFit<{ mode: "all" | "some" | "count"; visible: number }>({ + rowRef: laneRef, + chipsRef: chipRefs, + pillRef: plusRef, + deps: `${n}|${chipsKey}`, + initial: { mode: "all", visible: n }, + decide: (g) => { + const labelEl = labelRef.current; + const labelRight = labelEl ? labelEl.offsetLeft + labelEl.offsetWidth : 0; + const clearW = n > 1 ? g.gap + (clearRef.current?.offsetWidth ?? 0) : 0; + return decideChipLayout({ + n, + labelRight, + chipRights: g.chipRights, + plusW: g.pillW, + clearW, + avail: g.avail, + gap: g.gap, + narrowPx: NARROW_PX, + }); + }, + }); if (n === 0) return null; diff --git a/frontend/src/components/ChannelTags.tsx b/frontend/src/components/ChannelTags.tsx index 99ad3c95..3ed2017a 100644 --- a/frontend/src/components/ChannelTags.tsx +++ b/frontend/src/components/ChannelTags.tsx @@ -1,9 +1,11 @@ -import { useEffect, useLayoutEffect, useRef, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { Check, Plus } from "lucide-react"; import { useTranslation } from "react-i18next"; import { type Tag } from "../lib/api"; import { useAnchoredPopover, PopoverSurface, PopoverItem } from "./AnchoredPopover"; import { useScrollFade } from "../lib/useScrollFade"; +import { fitChipsWithPill } from "../lib/chipOverflow"; +import { useChipOverflowFit } from "../lib/useChipOverflowFit"; // The channel tag surface, shared by the channel manager (table + cards) and the channel detail // page. Built on the R8 popover platform. Two design goals it fixes over the old inline TagsCell: @@ -154,62 +156,25 @@ export function ChannelTagsControl({ const attached = userTags.filter((tg) => attachedIds.includes(tg.id)); // Deterministic single-line fit: a hidden measurer lays out ALL chips + a sample "+N" pill at their - // natural widths; we read each chip's right edge and compute EXACTLY how many fit, reserving the - // pill's width whenever any overflow. One pass (no iterative re-render), so the "+N" count is always - // right as tags are added/removed. Re-measures on width/scale change via ResizeObserver — off real - // rem boxes, never a px guess. + // natural widths; the shared measure core (useChipOverflowFit) reads each chip's right edge, the rem + // gap and the pill width off real boxes, and fitChipsWithPill computes EXACTLY how many fit (reserving + // the pill whenever any overflow). One pass — the count is right as tags are added/removed/renamed — + // and it re-measures on width/scale change via the hook's ResizeObserver. const rowRef = useRef(null); - const measureRef = useRef(null); - const [visible, setVisible] = useState(attached.length); + const chipRefs = useRef>([]); + const pillRef = useRef(null); // Key on id AND name so a RENAME (which changes a chip's width) re-runs the fit, not just add/remove. const tagsKey = attached.map((tg) => `${tg.id}:${tg.name}`).join("|"); - useLayoutEffect(() => { - const row = rowRef.current; - const measure = measureRef.current; - if (!row || !measure) return; - const compute = () => { - const nodes = Array.from(measure.children) as HTMLElement[]; - const pill = nodes[nodes.length - 1]; - if (!pill) return; - const pillW = pill.offsetWidth; - const chips = nodes.slice(0, -1); - // Measure the real rendered gap (gap-1 is rem — it scales with the font slider) off the box - // between the last chip and the sample pill, rather than assuming a px constant. - const lastChip = chips[chips.length - 1]; - const gap = lastChip - ? Math.max(0, pill.offsetLeft - (lastChip.offsetLeft + lastChip.offsetWidth)) - : 0; - const avail = row.clientWidth; - // Whole-set fast path: if the LAST chip's right edge already fits, every chip fits and no "+N" - // pill is needed. The reservation loop below reserves the pill's width for every chip except - // the last and breaks at the first overflow, so on its own it never reaches the only "does the - // whole set fit?" test — it would collapse chips that would have fitted (e.g. a short trailing - // chip narrower than the pill), leaving "+N" beside empty space. - const lastRight = lastChip ? lastChip.offsetLeft + lastChip.offsetWidth : 0; - if (lastRight <= avail) { - setVisible(chips.length); - return; - } - let count = chips.length; - for (let i = 0; i < chips.length; i++) { - const chip = chips[i]; - if (!chip) break; - const right = chip.offsetLeft + chip.offsetWidth; // right edge within the measurer - const moreAfter = i < chips.length - 1; - if (right > avail - (moreAfter ? pillW + gap : 0)) { - count = i; - break; - } - } - setVisible(count); - }; - compute(); - if (typeof ResizeObserver === "undefined") return; - const ro = new ResizeObserver(compute); - ro.observe(row); - return () => ro.disconnect(); - }, [tagsKey]); + const visible = useChipOverflowFit({ + rowRef, + chipsRef: chipRefs, + pillRef, + deps: tagsKey, + initial: attached.length, + decide: (g) => + fitChipsWithPill({ chipRights: g.chipRights, pillW: g.pillW, avail: g.avail, gap: g.gap }), + }); const hidden = attached.length - visible; @@ -249,16 +214,27 @@ export function ChannelTagsControl({
- {/* Hidden measurer: all chips + a sample pill, laid out unbounded to read their true widths. */} + {/* Hidden measurer: all chips + a sample pill, laid out unbounded to read their true widths. Each + chip is wrapped in a ref span (index-aligned) so the shared fit hook can read its right edge; + the pill is the sample the hook reserves room for. */}
- {attached.map((tg) => ( - + {attached.map((tg, i) => ( + { + chipRefs.current[i] = el; + }} + className="inline-flex" + > + + ))} - +{attached.length} + + +{attached.length} +
diff --git a/frontend/src/lib/chipOverflow.test.ts b/frontend/src/lib/chipOverflow.test.ts index 02ae1d99..bb19275d 100644 --- a/frontend/src/lib/chipOverflow.test.ts +++ b/frontend/src/lib/chipOverflow.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "vitest"; -import { decideChipLayout, type ChipFitInput } from "./chipOverflow"; +import { decideChipLayout, fitChipsWithPill, type ChipFitInput } from "./chipOverflow"; // Build measurer-style right edges for `n` chips of a fixed width, starting after a label, with a gap. function rights(labelRight: number, widths: number[], gap: number): number[] { @@ -91,3 +91,29 @@ describe("decideChipLayout", () => { expect(decideChipLayout(input)).toEqual({ mode: "count", visible: 0 }); }); }); + +describe("fitChipsWithPill", () => { + // Three chips of width 50, gap 6, starting at 0 -> right edges 50, 106, 162. + const chipRights = rights(0, [50, 50, 50], 6); + + it("shows all chips (no pill) when the last chip's right edge fits — the whole-set fast path", () => { + expect(fitChipsWithPill({ chipRights, pillW: 40, gap: 6, avail: 200 })).toBe(3); + }); + + it("returns 0 for an empty chip set", () => { + expect(fitChipsWithPill({ chipRights: [], pillW: 40, gap: 6, avail: 200 })).toBe(0); + }); + + it("reserves the pill width for all but the last chip when overflowing", () => { + // avail 120: whole set (162) doesn't fit. chip0 right 50 <= 120-(40+6)=74 ok; chip1 right 106 > 74 + // -> dropped at i=1 -> visible 1. + expect(fitChipsWithPill({ chipRights, pillW: 40, gap: 6, avail: 120 })).toBe(1); + }); + + it("keeps the whole set via the fast path even when a reserve-for-all loop would drop chips", () => { + // Right edges 56, 112, 118; avail 120: the whole set fits (118 <= 120). A naive loop that reserves + // pill+gap (46) for every non-last chip would drop at chip1 (112 > 120-46=74) and show "+2" beside + // empty space — the fast path returns all 3 instead. + expect(fitChipsWithPill({ chipRights: [56, 112, 118], pillW: 40, gap: 6, avail: 120 })).toBe(3); + }); +}); diff --git a/frontend/src/lib/chipOverflow.ts b/frontend/src/lib/chipOverflow.ts index 7b4ba6a9..3611fc39 100644 --- a/frontend/src/lib/chipOverflow.ts +++ b/frontend/src/lib/chipOverflow.ts @@ -1,11 +1,13 @@ -// 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. +// Pure fit decisions for a single-line chip lane that collapses its overflow into a "+N" pill. +// Lifted out of the components so the math is unit-testable without a layout engine (jsdom reports +// 0-width boxes), the same split as lib/dateFilter and lib/columnSort. The shared DOM-measurement +// half — the layout-effect + ResizeObserver that reads real rem boxes off a hidden mirror — lives in +// lib/useChipOverflowFit; it hands the measured geometry to one of these policies: +// - decideChipLayout: the feed's applied-filter row, which also shares its width with fixed trailing +// controls (Sort/View), reserves a leading label + a trailing "Clear all", and collapses to a +// single count pill on a narrow lane. +// - fitChipsWithPill: the chips-only channel tag cell (ChannelTagsControl) — just "how many fit +// before the +N pill". export interface ChipFitInput { /** Number of chips. */ @@ -51,3 +53,37 @@ export function decideChipLayout(p: ChipFitInput): ChipFit { if (k === 0 && labelRight + gap + plusW > avail) return { mode: "count", visible: 0 }; return { mode: "some", visible: k }; } + +// The chips-only policy (ChannelTagsControl): how many chips fit on the line, reserving the "+N" pill's +// width whenever any overflow. Returns the visible count. +export interface ChipsWithPillInput { + /** Right edge (offsetLeft + offsetWidth) of each chip, index-aligned. */ + chipRights: number[]; + /** Width of the "+N" pill. */ + pillW: number; + /** Available lane width. */ + avail: number; + /** The real (rem-scaled) inter-item gap. */ + gap: number; +} + +export function fitChipsWithPill(p: ChipsWithPillInput): number { + const { chipRights, pillW, avail, gap } = p; + const n = chipRights.length; + if (n === 0) return 0; + // Whole-set fast path: if the LAST chip's right edge already fits, every chip fits and NO pill is + // needed. The reservation loop below reserves the pill for every chip except the last and breaks at + // the first overflow, so on its own it would collapse a short trailing chip (narrower than the pill) + // that would actually have fitted, leaving "+N" beside empty space. + if ((chipRights[n - 1] ?? 0) <= avail) return n; + let count = n; + for (let i = 0; i < n; i++) { + const right = chipRights[i] ?? 0; + const moreAfter = i < n - 1; + if (right > avail - (moreAfter ? pillW + gap : 0)) { + count = i; + break; + } + } + return count; +} diff --git a/frontend/src/lib/useChipOverflowFit.dom.test.tsx b/frontend/src/lib/useChipOverflowFit.dom.test.tsx new file mode 100644 index 00000000..388c459e --- /dev/null +++ b/frontend/src/lib/useChipOverflowFit.dom.test.tsx @@ -0,0 +1,102 @@ +// @vitest-environment jsdom +// +// The shared chip-overflow measure core (S3). jsdom has no layout engine — every offset* / clientWidth +// is 0 — so we stub those on the real ref'd nodes and assert the hook (a) reads the geometry off them +// (chip right edges, pill width, the gap between the last chip and the pill, and the lane's available +// width), (b) returns the caller's `decide` result, and (c) re-measures when `deps` changes. +import { describe, it, expect, afterEach } from "vitest"; +import { useRef } from "react"; +import { render, screen, cleanup, act } from "@testing-library/react"; +import { useChipOverflowFit, type ChipRowGeometry } from "./useChipOverflowFit"; + +function stub( + el: HTMLElement, + props: Partial>, +) { + for (const [k, v] of Object.entries(props)) { + Object.defineProperty(el, k, { configurable: true, get: () => v }); + } +} + +let lastGeom: ChipRowGeometry | null = null; + +function Harness({ deps }: { deps: string }) { + const rowRef = useRef(null); + const chipRefs = useRef>([]); + const pillRef = useRef(null); + const visible = useChipOverflowFit({ + rowRef, + chipsRef: chipRefs, + pillRef, + deps, + initial: -1, + decide: (g) => { + lastGeom = g; + // A trivial policy: count the chips whose right edge fits the lane. + return g.chipRights.filter((r) => r <= g.avail).length; + }, + }); + return ( +
+
+ { + chipRefs.current[0] = el; + }} + data-testid="chip0" + /> + { + chipRefs.current[1] = el; + }} + data-testid="chip1" + /> + +
{visible}
+
+ ); +} + +afterEach(() => { + cleanup(); + lastGeom = null; +}); + +describe("useChipOverflowFit", () => { + it("reads geometry off the refs and returns the decide() result; re-measures on deps change", () => { + const { rerender } = render(); + + // First pass ran against jsdom's 0-sized boxes. + expect(lastGeom).toEqual({ avail: 0, chipRights: [0, 0], pillW: 0, gap: 0 }); + + // Give the boxes real geometry, then bump deps to force a re-measure. + stub(screen.getByTestId("row"), { clientWidth: 100 }); + stub(screen.getByTestId("chip0"), { offsetLeft: 0, offsetWidth: 30 }); // right edge 30 + stub(screen.getByTestId("chip1"), { offsetLeft: 36, offsetWidth: 30 }); // right edge 66 + stub(screen.getByTestId("pill"), { offsetLeft: 72, offsetWidth: 20 }); // gap = 72 - 66 = 6 + + act(() => { + rerender(); + }); + + expect(lastGeom).toEqual({ avail: 100, chipRights: [30, 66], pillW: 20, gap: 6 }); + // decide counted both chips (30 and 66 both <= 100). + expect(screen.getByTestId("visible").textContent).toBe("2"); + }); + + it("drops a chip whose right edge exceeds the lane, via the caller's policy", () => { + const { rerender } = render(); + stub(screen.getByTestId("row"), { clientWidth: 40 }); + stub(screen.getByTestId("chip0"), { offsetLeft: 0, offsetWidth: 30 }); // right edge 30 <= 40 + stub(screen.getByTestId("chip1"), { offsetLeft: 36, offsetWidth: 30 }); // right edge 66 > 40 + stub(screen.getByTestId("pill"), { offsetLeft: 72, offsetWidth: 20 }); + + act(() => { + rerender(); + }); + + expect(lastGeom).toEqual({ avail: 40, chipRights: [30, 66], pillW: 20, gap: 6 }); + // Only chip0 (right edge 30) fits the 40-wide lane; chip1 (66) is dropped by the policy. + expect(screen.getByTestId("visible").textContent).toBe("1"); + }); +}); diff --git a/frontend/src/lib/useChipOverflowFit.ts b/frontend/src/lib/useChipOverflowFit.ts new file mode 100644 index 00000000..792a9dd6 --- /dev/null +++ b/frontend/src/lib/useChipOverflowFit.ts @@ -0,0 +1,79 @@ +import { useLayoutEffect, useRef, useState, type RefObject } from "react"; + +// The shared DOM-measure-and-fit core for a single-line chip lane that collapses its overflow into a +// trailing "+N" pill. Owns the layout-effect + ResizeObserver + initial-measure wiring and reads the +// common geometry off a hidden mirror (each chip's right edge, the real rem gap, the pill width, and +// the lane's available width); each caller layers its OWN fit policy in `decide` and gets that result +// back. This was hand-rolled twice — the feed's applied-filter row (ActiveFilterChips, whose policy +// also reserves a leading label + a trailing "Clear all" and can collapse to a single count pill) and +// the channel tag cell (ChannelTagsControl, chips-only) — with the same measurement half copied in +// both. The policy halves live in lib/chipOverflow.ts; this owns the measurement half. +// +// Coordinate note: `chipRights` are offsets within the hidden mirror (its children's offsetParent), +// while `avail` is the VISIBLE lane's clientWidth — the two are comparable because the mirror is laid +// out `absolute left-0 top-0` over the lane's start, so a mirror-relative right edge is the position a +// chip would take in the real lane. Reads real rem boxes, so nothing is frozen in px (the text-size +// slider scales the gap and chip widths, and the fit follows). + +export interface ChipRowGeometry { + /** Available lane width — the visible row's clientWidth. */ + avail: number; + /** Right edge (offsetLeft + offsetWidth) of each chip in the measurer, index-aligned to the chips. */ + chipRights: number[]; + /** Width of the trailing sample "+N" pill in the measurer. */ + pillW: number; + /** The real rendered inter-item gap (rem-scaled), read off the box between the last chip and the pill. */ + gap: number; +} + +export function useChipOverflowFit(args: { + /** The VISIBLE lane; its clientWidth is the available width, and resize is observed on it. */ + rowRef: RefObject; + /** The measurer's chip elements, index-aligned (a hidden mirror of the full chip set). */ + chipsRef: RefObject>; + /** The sample "+N" pill in the measurer. */ + pillRef: RefObject; + /** Re-measure when this changes (a key over the chip set — count + labels, so a rename re-fits too). */ + deps: unknown; + /** The caller's fit policy over the measured geometry. */ + decide: (geom: ChipRowGeometry) => R; + /** Value used until the first measure (and while the refs aren't mounted). */ + initial: R; +}): R { + const { rowRef, chipsRef, pillRef, deps, decide, initial } = args; + const [result, setResult] = useState(initial); + // Read `decide` fresh each run without making it an effect dependency: the effect re-runs on `deps` + // (and resize), and a caller's decide closure is rebuilt every render — depending on it would + // resubscribe the ResizeObserver on every render. + const decideRef = useRef(decide); + decideRef.current = decide; + + useLayoutEffect(() => { + const row = rowRef.current; + const pill = pillRef.current; + if (!row || !pill) return; + const rightOf = (el: HTMLElement | null | undefined) => + el ? el.offsetLeft + el.offsetWidth : 0; + const run = () => { + const chips = (chipsRef.current ?? []).filter((c): c is HTMLElement => c != null); + const lastChip = chips[chips.length - 1]; + setResult( + decideRef.current({ + avail: row.clientWidth, + chipRights: chips.map(rightOf), + pillW: pill.offsetWidth, + gap: lastChip ? Math.max(0, pill.offsetLeft - rightOf(lastChip)) : 0, + }), + ); + }; + run(); + // jsdom has no ResizeObserver — the one-shot measure above still runs there (unit tests drive it). + if (typeof ResizeObserver === "undefined") return; + const ro = new ResizeObserver(run); + ro.observe(row); + return () => ro.disconnect(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [deps]); + + return result; +}