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.
This commit is contained in:
@@ -112,7 +112,12 @@ describe("VideoCard — a title you select", () => {
|
|||||||
range.selectNodeContents(title);
|
range.selectNodeContents(title);
|
||||||
window.getSelection()!.removeAllRanges();
|
window.getSelection()!.removeAllRanges();
|
||||||
window.getSelection()!.addRange(range);
|
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);
|
title.dispatchEvent(click);
|
||||||
expect(onOpen).not.toHaveBeenCalled();
|
expect(onOpen).not.toHaveBeenCalled();
|
||||||
expect(click.defaultPrevented).toBe(true);
|
expect(click.defaultPrevented).toBe(true);
|
||||||
|
|||||||
@@ -122,7 +122,7 @@ function openInApp(
|
|||||||
e.preventDefault();
|
e.preventDefault();
|
||||||
// The click that ends selecting the title's words (to copy them) neither plays nor follows the
|
// The click that ends selecting the title's words (to copy them) neither plays nor follows the
|
||||||
// link (docs/DESIGN.md "Text stays selectable").
|
// link (docs/DESIGN.md "Text stays selectable").
|
||||||
if (endsTextSelection(e.currentTarget)) return;
|
if (endsTextSelection(e)) return;
|
||||||
onOpen(video);
|
onOpen(video);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -306,10 +306,10 @@ describe("a Plex card's title stays selectable", () => {
|
|||||||
range.selectNodeContents(title);
|
range.selectNodeContents(title);
|
||||||
window.getSelection()!.removeAllRanges();
|
window.getSelection()!.removeAllRanges();
|
||||||
window.getSelection()!.addRange(range);
|
window.getSelection()!.addRange(range);
|
||||||
fireEvent.click(title);
|
fireEvent.click(title, { detail: 1 });
|
||||||
expect(onOpen).not.toHaveBeenCalled();
|
expect(onOpen).not.toHaveBeenCalled();
|
||||||
window.getSelection()!.removeAllRanges();
|
window.getSelection()!.removeAllRanges();
|
||||||
fireEvent.click(title);
|
fireEvent.click(title, { detail: 1 });
|
||||||
expect(onOpen).toHaveBeenCalledTimes(1);
|
expect(onOpen).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -155,7 +155,7 @@ function CardTitle({
|
|||||||
onClick={
|
onClick={
|
||||||
onClick &&
|
onClick &&
|
||||||
((e) => {
|
((e) => {
|
||||||
if (!endsTextSelection(e.currentTarget) && !movedPastSlop(pressedAt.current, e))
|
if (!endsTextSelection(e) && !movedPastSlop(pressedAt.current, e))
|
||||||
onClick();
|
onClick();
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -31,10 +31,24 @@ describe("a selectable button", () => {
|
|||||||
const btn = screen.getByRole("button");
|
const btn = screen.getByRole("button");
|
||||||
expect(btn.classList.contains("select-text")).toBe(true);
|
expect(btn.classList.contains("select-text")).toBe(true);
|
||||||
selectText(screen.getByText("Andor"));
|
selectText(screen.getByText("Andor"));
|
||||||
btn.click();
|
fireEvent.click(btn, { detail: 1 }); // the mouseup that ended the drag-select
|
||||||
expect(onClick).not.toHaveBeenCalled();
|
expect(onClick).not.toHaveBeenCalled();
|
||||||
window.getSelection()!.removeAllRanges();
|
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(
|
||||||
|
<Pressable selectable onClick={onClick}>
|
||||||
|
<span>Andor</span>
|
||||||
|
</Pressable>,
|
||||||
|
);
|
||||||
|
selectText(screen.getByText("Andor"));
|
||||||
|
fireEvent.click(screen.getByRole("button"), { detail: 0 });
|
||||||
expect(onClick).toHaveBeenCalledTimes(1);
|
expect(onClick).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -80,11 +80,7 @@ export const FocusSafeButton = forwardRef<
|
|||||||
e.preventDefault();
|
e.preventDefault();
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (
|
if (selectable && (endsTextSelection(e) || movedPastSlop(pressedAt.current, e))) return;
|
||||||
selectable &&
|
|
||||||
(endsTextSelection(e.currentTarget) || movedPastSlop(pressedAt.current, e))
|
|
||||||
)
|
|
||||||
return;
|
|
||||||
onClick?.(e);
|
onClick?.(e);
|
||||||
}}
|
}}
|
||||||
onMouseDown={(e) => {
|
onMouseDown={(e) => {
|
||||||
|
|||||||
@@ -4,8 +4,13 @@
|
|||||||
// A plain click collapses any selection on mousedown, so a non-empty selection that touches the
|
// 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.
|
// 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`. */
|
/** True when this click ended a selection: the page's selection is non-empty and starts or ends
|
||||||
export function endsTextSelection(el: Element): boolean {
|
* 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();
|
const sel = typeof window === "undefined" ? null : window.getSelection();
|
||||||
if (!sel || sel.isCollapsed || !sel.toString().trim()) return false;
|
if (!sel || sel.isCollapsed || !sel.toString().trim()) return false;
|
||||||
return (
|
return (
|
||||||
|
|||||||
Reference in New Issue
Block a user