fix(ui): stable shelf-card menu triggers, a named Button by type, true layout comments
- Home's rail handed each card a fresh `() => menu.triggerProps(c)` per
render, defeating memo(VideoCard) on every tile whenever the menu's own
state changed. `useItemTriggers` (ui/useContextMenu) keeps one trigger per
item, index-aligned; tested for identity across renders.
- The Button accessible-name union had one hole: `<Button icon={…} />` with
no children and no size typechecked into a nameless button. A textful
button now needs children or a name; pinned with @ts-expect-error.
- comfortPin's header said the pin belongs "on every card and row", which
the code never meant (an episode, a season, a person are not pin kinds).
It now says what is pinnable — and the show page's Related strip, whose
shows ARE pinnable, gets the pin menu it was missing (and a rem width).
- gridColumns / feedView / VirtualGrid claimed layouts 1024–2560 are
"byte-identical" under the column cap; the small cards are capped at 2560
with the filter rail collapsed. The comments say so.
This commit is contained in:
@@ -161,6 +161,9 @@ describe("Button accessible-name contract (type level)", () => {
|
||||
<Button size="icon" icon={<span />} data-testid="bad-a" />
|
||||
{/* @ts-expect-error iconOnly with no label/aria-label/title has no accessible name */}
|
||||
<Button iconOnly icon={<span />} data-testid="bad-b" />
|
||||
{/* @ts-expect-error an icon with no children and no size renders no text and no name */}
|
||||
<Button icon={<span />} data-testid="bad-c" />
|
||||
<Button icon={<span />} label="an icon button named by label" />
|
||||
<Button size="icon" icon={<span />} label="by label" />
|
||||
<Button size="icon-sm" icon={<span />} aria-label="by aria-label" />
|
||||
<Button iconOnly icon={<span />} title="by title" />
|
||||
|
||||
@@ -119,7 +119,12 @@ interface ButtonOwnProps extends ButtonHTMLAttributes<HTMLButtonElement> {
|
||||
// use it for the tooltip that IS the name.
|
||||
type NamedSomehow = { label: string } | { "aria-label": string } | { title: string };
|
||||
type TextlessButton = ({ iconOnly: true } | { size: "icon" | "icon-sm" }) & NamedSomehow;
|
||||
type TextfulButton = { iconOnly?: false; size?: Exclude<ButtonSize, "icon" | "icon-sm"> };
|
||||
// …and a button of any other size must actually HAVE text, or be named: `<Button icon={x} />` with
|
||||
// no children and no size typechecked into a nameless button (E10 review).
|
||||
type TextfulButton = { iconOnly?: false; size?: Exclude<ButtonSize, "icon" | "icon-sm"> } & (
|
||||
| { children: ReactNode }
|
||||
| NamedSomehow
|
||||
);
|
||||
export type ButtonProps = ButtonOwnProps & (TextlessButton | TextfulButton);
|
||||
|
||||
/** The shared action button. `type` defaults to "button" so it never accidentally submits a form. */
|
||||
|
||||
@@ -47,8 +47,8 @@ export interface VirtualGridProps<T> {
|
||||
* just above its minimum column, so on an ultrawide it answers "more room" with more, smaller
|
||||
* items forever — 3440px gave the feed 11 columns of 264px. With a cap the extra width goes into
|
||||
* the items instead (8 columns of ~370px), which is what a screen watched from a distance wants.
|
||||
* It only ever binds ABOVE the width where that many columns already fit, so narrower layouts —
|
||||
* including the 1024px floor and 1920/2560 — are untouched. */
|
||||
* It only ever binds ABOVE the width where that many columns already fit, so the 1024px floor and
|
||||
* 1920 are untouched; at 2560 it depends on the caller's cap (see `feedView.maxCols`). */
|
||||
maxCols?: number | null;
|
||||
/** Estimated row height before measurement; real heights are measured per-row, so this only
|
||||
* affects the very first paint and the scrollbar's initial guess. Measure it, don't guess — a
|
||||
|
||||
@@ -6,8 +6,10 @@
|
||||
// just above its minimum column width no matter how wide the screen is. Measured at 3440px (E10 S4),
|
||||
// the feed laid out 11 columns of 264px — a diluted feed is as bad a failure of the viewport contract
|
||||
// as a broken one, so `maxCols` stops the multiplication and lets the leftover width flow into the
|
||||
// items instead. The cap can only bind ABOVE the width at which that many columns already fit, which
|
||||
// is what keeps every narrower layout — the 1024px floor included — bit-for-bit unchanged.
|
||||
// items instead. The cap can only bind ABOVE the width at which that many columns already fit, so the
|
||||
// 1024px floor and everything near it is unchanged. How far up that reaches is per view: the feed's
|
||||
// small cards (`feedView.cardsSmall`, 11) are already capped at 2560 with the filter rail collapsed —
|
||||
// 12 would fit there — while the regular cards stay uncapped up to 2560.
|
||||
|
||||
/** The grid's gutter in px; the column maths has to net it out on both sides of the division. */
|
||||
export const GRID_GAP = 16;
|
||||
|
||||
@@ -1,4 +1,10 @@
|
||||
import { useCallback, useRef, useState, type PointerEvent as ReactPointerEvent } from "react";
|
||||
import {
|
||||
useCallback,
|
||||
useMemo,
|
||||
useRef,
|
||||
useState,
|
||||
type PointerEvent as ReactPointerEvent,
|
||||
} from "react";
|
||||
|
||||
import { matchesShortcut, useKeyScope } from "../../lib/keymap";
|
||||
import { useAnchoredPopover } from "./anchoredPopoverHooks";
|
||||
@@ -177,3 +183,14 @@ export function useContextMenu<T>(): ContextMenuController<T> {
|
||||
|
||||
return { target, open, close: () => setOpen(false), triggerProps, popover: pop };
|
||||
}
|
||||
|
||||
/** One trigger per item, stable while the items and the menu are — for a list whose card component
|
||||
* takes `menuProps: () => props` but whose menu acts on a WRAPPER of what the card holds (Home's
|
||||
* unified shelf card around a `Video`). A fresh `() => menu.triggerProps(c)` per render defeats a
|
||||
* `memo` card on every tile each time the menu's own state changes. Index-aligned with `items`. */
|
||||
export function useItemTriggers<T>(
|
||||
items: readonly T[],
|
||||
triggerProps: (payload: T) => Record<string, unknown>,
|
||||
): (() => Record<string, unknown>)[] {
|
||||
return useMemo(() => items.map((it) => () => triggerProps(it)), [items, triggerProps]);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,18 @@
|
||||
// @vitest-environment jsdom
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { renderHook } from "@testing-library/react";
|
||||
|
||||
import { useItemTriggers } from "./useContextMenu";
|
||||
|
||||
describe("useItemTriggers", () => {
|
||||
it("hands each item the same trigger across renders, so a memo card is not re-rendered", () => {
|
||||
const items = [{ id: 1 }, { id: 2 }];
|
||||
const triggerProps = vi.fn((it: { id: number }) => ({ "data-id": it.id }));
|
||||
const { result, rerender } = renderHook(() => useItemTriggers(items, triggerProps));
|
||||
const first = result.current;
|
||||
rerender();
|
||||
expect(result.current[0]).toBe(first[0]);
|
||||
expect(result.current[1]).toBe(first[1]);
|
||||
expect(result.current[1]!()).toEqual({ "data-id": 2 });
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user