From 0b1d9058667a6772ff6494a4ef4573a4b93ccd3b Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 27 Sep 2026 14:55:50 +0200 Subject: [PATCH] fix(ui): Enter on a selectable title opens it even with its words selected endsTextSelection takes the click event and exempts a keyboard click (detail 0): Enter / Space ends no selection, so a title selected and then tabbed to opened nothing. All three callers (FocusSafeButton, the Plex caption, the video card title) go through it. --- .../src/components/VideoCard.nav.dom.test.tsx | 7 ++++++- frontend/src/components/VideoCard.tsx | 2 +- frontend/src/components/plexCards.dom.test.tsx | 4 ++-- frontend/src/components/plexCards.tsx | 2 +- .../ui/FocusSafeButton.select.dom.test.tsx | 18 ++++++++++++++++-- frontend/src/components/ui/FocusSafeButton.tsx | 6 +----- frontend/src/lib/textSelection.ts | 9 +++++++-- 7 files changed, 34 insertions(+), 14 deletions(-) diff --git a/frontend/src/components/VideoCard.nav.dom.test.tsx b/frontend/src/components/VideoCard.nav.dom.test.tsx index d7c09fc5..783b52c6 100644 --- a/frontend/src/components/VideoCard.nav.dom.test.tsx +++ b/frontend/src/components/VideoCard.nav.dom.test.tsx @@ -112,7 +112,12 @@ describe("VideoCard — a title you select", () => { range.selectNodeContents(title); window.getSelection()!.removeAllRanges(); window.getSelection()!.addRange(range); - const click = new MouseEvent("click", { bubbles: true, cancelable: true, button: 0 }); + const click = new MouseEvent("click", { + bubbles: true, + cancelable: true, + button: 0, + detail: 1, + }); title.dispatchEvent(click); expect(onOpen).not.toHaveBeenCalled(); expect(click.defaultPrevented).toBe(true); diff --git a/frontend/src/components/VideoCard.tsx b/frontend/src/components/VideoCard.tsx index 3ad1e78f..a5887197 100644 --- a/frontend/src/components/VideoCard.tsx +++ b/frontend/src/components/VideoCard.tsx @@ -122,7 +122,7 @@ function openInApp( e.preventDefault(); // The click that ends selecting the title's words (to copy them) neither plays nor follows the // link (docs/DESIGN.md "Text stays selectable"). - if (endsTextSelection(e.currentTarget)) return; + if (endsTextSelection(e)) return; onOpen(video); } diff --git a/frontend/src/components/plexCards.dom.test.tsx b/frontend/src/components/plexCards.dom.test.tsx index 90c2c98a..70dc10a7 100644 --- a/frontend/src/components/plexCards.dom.test.tsx +++ b/frontend/src/components/plexCards.dom.test.tsx @@ -306,10 +306,10 @@ describe("a Plex card's title stays selectable", () => { range.selectNodeContents(title); window.getSelection()!.removeAllRanges(); window.getSelection()!.addRange(range); - fireEvent.click(title); + fireEvent.click(title, { detail: 1 }); expect(onOpen).not.toHaveBeenCalled(); window.getSelection()!.removeAllRanges(); - fireEvent.click(title); + fireEvent.click(title, { detail: 1 }); expect(onOpen).toHaveBeenCalledTimes(1); }); }); diff --git a/frontend/src/components/plexCards.tsx b/frontend/src/components/plexCards.tsx index 2e86f879..73dd39cb 100644 --- a/frontend/src/components/plexCards.tsx +++ b/frontend/src/components/plexCards.tsx @@ -155,7 +155,7 @@ function CardTitle({ onClick={ onClick && ((e) => { - if (!endsTextSelection(e.currentTarget) && !movedPastSlop(pressedAt.current, e)) + if (!endsTextSelection(e) && !movedPastSlop(pressedAt.current, e)) onClick(); }) } diff --git a/frontend/src/components/ui/FocusSafeButton.select.dom.test.tsx b/frontend/src/components/ui/FocusSafeButton.select.dom.test.tsx index 7f64fbbc..fbf9a47c 100644 --- a/frontend/src/components/ui/FocusSafeButton.select.dom.test.tsx +++ b/frontend/src/components/ui/FocusSafeButton.select.dom.test.tsx @@ -31,10 +31,24 @@ describe("a selectable button", () => { const btn = screen.getByRole("button"); expect(btn.classList.contains("select-text")).toBe(true); selectText(screen.getByText("Andor")); - btn.click(); + fireEvent.click(btn, { detail: 1 }); // the mouseup that ended the drag-select expect(onClick).not.toHaveBeenCalled(); window.getSelection()!.removeAllRanges(); - btn.click(); + fireEvent.click(btn, { detail: 1 }); + expect(onClick).toHaveBeenCalledTimes(1); + }); + + // E28 review H2: Enter / Space on a focused title with its words still selected (selected, then + // tabbed to) did nothing — a keyboard click ends no selection; it carries no pointer at all. + it("a keyboard click opens even with the title's words selected", () => { + const onClick = vi.fn(); + render( + + Andor + , + ); + selectText(screen.getByText("Andor")); + fireEvent.click(screen.getByRole("button"), { detail: 0 }); expect(onClick).toHaveBeenCalledTimes(1); }); diff --git a/frontend/src/components/ui/FocusSafeButton.tsx b/frontend/src/components/ui/FocusSafeButton.tsx index b3d1a53a..b40099e8 100644 --- a/frontend/src/components/ui/FocusSafeButton.tsx +++ b/frontend/src/components/ui/FocusSafeButton.tsx @@ -80,11 +80,7 @@ export const FocusSafeButton = forwardRef< e.preventDefault(); return; } - if ( - selectable && - (endsTextSelection(e.currentTarget) || movedPastSlop(pressedAt.current, e)) - ) - return; + if (selectable && (endsTextSelection(e) || movedPastSlop(pressedAt.current, e))) return; onClick?.(e); }} onMouseDown={(e) => { diff --git a/frontend/src/lib/textSelection.ts b/frontend/src/lib/textSelection.ts index 3189797b..a722f2ea 100644 --- a/frontend/src/lib/textSelection.ts +++ b/frontend/src/lib/textSelection.ts @@ -4,8 +4,13 @@ // A plain click collapses any selection on mousedown, so a non-empty selection that touches the // element at click time can only be the one this gesture just made. -/** True when the page's selection is non-empty and starts or ends inside `el`. */ -export function endsTextSelection(el: Element): boolean { +/** True when this click ended a selection: the page's selection is non-empty and starts or ends + * inside the clicked element. Takes the EVENT, not the element, because a keyboard click (Enter / + * Space, `detail` 0) ends no selection — it has no pointer — so a title whose words are still + * selected must open on Enter (E28 review H2). */ +export function endsTextSelection(e: { currentTarget: Element; detail: number }): boolean { + if (e.detail === 0) return false; + const el = e.currentTarget; const sel = typeof window === "undefined" ? null : window.getSelection(); if (!sel || sel.isCollapsed || !sel.toString().trim()) return false; return (