From 29808667ccf32038d467242b1df413f64be0455b Mon Sep 17 00:00:00 2001 From: Marian Moldovan Date: Wed, 29 Jul 2026 16:59:05 +0200 Subject: [PATCH] fix(ui): make clickable Card activate from the keyboard Tabbing to a collection card on the home page and pressing Enter did nothing, and pressing Space scrolled the page away from the focused card. The card was already focusable (role="button" + tabIndex={0} when onClick is present), but the keyboard handling was wrong in two ways: - It was bound to onKeyPress, which is deprecated and no longer a reliable activation hook on React 19. The handler simply never ran for a keydown, so Enter never activated the card. - It never called preventDefault(), so Space additionally performed its default browser action and scrolled the page, moving the focused card out of view. Switch to onKeyDown and preventDefault() for both Enter and Space. The handler only activates when the event target is the card itself, so key events bubbling up from focusable children (the action IconButtons that NavigationCard renders inside the card, whose wrapper stops click but not key propagation) no longer activate the card as well. A caller supplied onKeyDown is now composed rather than silently overwritten by the props spread, and can opt out of activation via preventDefault(). Also add a focus-visible ring to cardClickableMixin, following the existing convention, so keyboard users can see which card holds focus. Refs #726 This fixes the deprecated `onKeyPress` handler, but not the dnd-kit wrapper in `HomePageDnD` that is the likely primary cause of the reported disappearing card. Leaving the issue open for that. Co-Authored-By: Claude Opus 5 --- packages/ui/src/components/Card.tsx | 23 ++++- packages/ui/src/styles.ts | 2 +- packages/ui/test/Card.test.tsx | 128 ++++++++++++++++++++++++++++ 3 files changed, 148 insertions(+), 5 deletions(-) create mode 100644 packages/ui/test/Card.test.tsx diff --git a/packages/ui/src/components/Card.tsx b/packages/ui/src/components/Card.tsx index 6d7105af1..9d088057d 100644 --- a/packages/ui/src/components/Card.tsx +++ b/packages/ui/src/components/Card.tsx @@ -15,18 +15,33 @@ const Card = React.forwardRef(({ className, onClick, style, + onKeyDown: onKeyDownProp, ...props }, ref) => { - const onKeyPress = useCallback((e: React.KeyboardEvent) => { + + const onKeyDown = useCallback((e: React.KeyboardEvent) => { + // Always let a caller supplied handler run first, and let it opt out of + // the default activation by calling preventDefault(). + onKeyDownProp?.(e); + if (!onClick || e.defaultPrevented) + return; + // Only activate when the card itself is focused. Cards may render their + // own focusable children (e.g. action buttons), which handle their own + // activation; their key events must not activate the card as well. + if (e.target !== e.currentTarget) + return; if (e.key === "Enter" || e.key === " ") { - onClick?.(); + // Without this, Space also performs its default browser action and + // scrolls the page away from the focused card. + e.preventDefault(); + onClick(); } - }, [onClick]); + }, [onClick, onKeyDownProp]); return (
{ + + it("renders its children", () => { + render(Products); + expect(screen.getByText("Products")).toBeInTheDocument(); + }); + + it("is not focusable and has no button role when not clickable", () => { + render(Static); + const card = screen.getByText("Static"); + expect(card).not.toHaveAttribute("role"); + expect(card).not.toHaveAttribute("tabindex"); + }); + + it("exposes a button role and is focusable when clickable", () => { + render( undefined}>Products); + expect(screen.getByRole("button", { name: "Products" })).toHaveAttribute("tabindex", "0"); + }); + + it("fires onClick on mouse click", () => { + const onClick = jest.fn(); + render(Products); + fireEvent.click(screen.getByRole("button")); + expect(onClick).toHaveBeenCalledTimes(1); + }); + + // Regression test for https://github.com/firecmsco/firecms/issues/726 + it("activates on Enter keydown", () => { + const onClick = jest.fn(); + render(Products); + fireEvent.keyDown(screen.getByRole("button"), { key: "Enter" }); + expect(onClick).toHaveBeenCalledTimes(1); + }); + + // Regression test for https://github.com/firecmsco/firecms/issues/726 + it("activates on Space keydown", () => { + const onClick = jest.fn(); + render(Products); + fireEvent.keyDown(screen.getByRole("button"), { key: " " }); + expect(onClick).toHaveBeenCalledTimes(1); + }); + + // The reported "the collection just disappears": without preventDefault, + // Space also scrolls the page away from the focused card. + it("prevents the default browser action for Enter and Space", () => { + render( undefined}>Products); + const card = screen.getByRole("button"); + // fireEvent returns false when the event was cancelled via preventDefault + expect(fireEvent.keyDown(card, { key: " " })).toBe(false); + expect(fireEvent.keyDown(card, { key: "Enter" })).toBe(false); + }); + + it("ignores other keys", () => { + const onClick = jest.fn(); + render(Products); + const card = screen.getByRole("button"); + fireEvent.keyDown(card, { key: "a" }); + fireEvent.keyDown(card, { key: "Tab" }); + fireEvent.keyDown(card, { key: "ArrowDown" }); + expect(onClick).not.toHaveBeenCalled(); + }); + + it("does not activate or preventDefault when the card is not clickable", () => { + render(Static); + expect(fireEvent.keyDown(screen.getByText("Static"), { key: "Enter" })).toBe(true); + }); + + // NavigationCard renders action IconButtons *inside* the card. Their key + // events bubble up to the card, which must not activate as well. + it("does not activate when a key event bubbles from a focusable child", () => { + const onClick = jest.fn(); + render( + + + + ); + const action = screen.getByTestId("card-action"); + fireEvent.keyDown(action, { key: "Enter" }); + fireEvent.keyDown(action, { key: " " }); + expect(onClick).not.toHaveBeenCalled(); + }); + + it("does not swallow the default action of a focusable child", () => { + render( + undefined}> + + + ); + // the child button keeps its own native Enter/Space activation + expect(fireEvent.keyDown(screen.getByTestId("card-action"), { key: " " })).toBe(true); + }); + + it("does not drop a caller supplied onKeyDown", () => { + const onClick = jest.fn(); + const onKeyDown = jest.fn(); + render(Products); + fireEvent.keyDown(screen.getByRole("button"), { key: "Enter" }); + expect(onKeyDown).toHaveBeenCalledTimes(1); + expect(onClick).toHaveBeenCalledTimes(1); + }); + + it("lets a caller supplied onKeyDown opt out of activation", () => { + const onClick = jest.fn(); + render( e.preventDefault()}>Products); + fireEvent.keyDown(screen.getByRole("button"), { key: "Enter" }); + expect(onClick).not.toHaveBeenCalled(); + }); + + it("has a visible focus indicator for keyboard users when clickable", () => { + render( undefined}>Products); + expect(screen.getByRole("button").className).toContain("focus-visible:ring-2"); + }); + + it("forwards refs and extra div props", () => { + const ref = React.createRef(); + render( undefined}>C); + expect(ref.current).toBeInstanceOf(HTMLDivElement); + expect(ref.current).toHaveAttribute("aria-label", "Products card"); + }); +});