diff --git a/apps/remote-dom-demo/src/app/remote/list/page.tsx b/apps/remote-dom-demo/src/app/remote/list/page.tsx index 131898c58f..5a1badd27c 100644 --- a/apps/remote-dom-demo/src/app/remote/list/page.tsx +++ b/apps/remote-dom-demo/src/app/remote/list/page.tsx @@ -44,7 +44,13 @@ export default function Page() { }); }} - d.name} showTiles> + {/* The href makes the host render a real anchor, so the browser's + context menu, middle-click and modifier-click work on an item. */} + d.name} + href={(d) => `#${d.name}`} + showTiles + > {(d) => { const c = useModalController(); return ( diff --git a/packages/components/src/components/List/List.browser.test.tsx b/packages/components/src/components/List/List.browser.test.tsx index ccb5199af0..ce39747ac2 100644 --- a/packages/components/src/components/List/List.browser.test.tsx +++ b/packages/components/src/components/List/List.browser.test.tsx @@ -10,8 +10,9 @@ import { } from "@/components/List"; import type { AsyncDataLoader } from "@/components/List/model/loading/types"; import { use, useState, type ReactNode } from "react"; -import { test } from "vitest"; +import { test, type Mock } from "vitest"; import { page, userEvent } from "vitest/browser"; +import { RouterProvider } from "react-aria-components"; import { SettingsProvider, type SettingsBackend, @@ -19,6 +20,8 @@ import { } from "../SettingsProvider"; import { FilterValue } from "./model/filter/FilterValue"; import Content from "../Content"; +import { Heading } from "../Heading"; +import { ContextMenu, MenuItem } from "../ContextMenu"; interface Data { num: number; @@ -652,3 +655,154 @@ describe("Item rendering", () => { .toBeInTheDocument(); }); }); + +describe("Linked items", () => { + const itemHref = `${location.origin}/domains/42`; + + let navigate: Mock; + let onAction: Mock; + let menuAction: Mock; + + beforeEach(() => { + navigate = vitest.fn(); + onAction = vitest.fn(); + menuAction = vitest.fn(); + }); + + const getTestElementWithLink = (target?: string) => ( + + + data={[{ num: 42 }]} /> + + textValue={({ num }) => String(num)} + href={({ num }) => `${location.origin}/domains/${num}`} + target={target} + > + {({ num }) => ( + + Item: {num} + + Delete + + + )} + + + + ); + + const row = page.getByRole("row"); + const optionsButton = page.getByRole("button", { name: "Options" }); + + const getRowLink = async () => + (await row.element()).querySelector("a"); + + test("a linked item renders a real anchor carrying the item's href", async () => { + await render(getTestElementWithLink()); + await expect.element(page.getByText("Item: 42")).toBeInTheDocument(); + + // Only a real gives the browser something to offer in its context + // menu and to open on a middle- or modifier-click. + expect(await getRowLink()).toHaveAttribute("href", itemHref); + }); + + test("the anchor carries the item's link target", async () => { + await render(getTestElementWithLink("_blank")); + await expect.element(page.getByText("Item: 42")).toBeInTheDocument(); + + expect(await getRowLink()).toHaveAttribute("target", "_blank"); + }); + + test("an item without a href renders no anchor", async () => { + await render(getTestElement([42])); + await expect.element(page.getByText("Item: 42")).toBeInTheDocument(); + + expect((await row.element()).querySelector("a")).toBeNull(); + }); + + test("the anchor adds neither a tab stop nor a second link for screen readers", async () => { + await render(getTestElementWithLink()); + await expect.element(page.getByText("Item: 42")).toBeInTheDocument(); + + // The row keeps owning activation and semantics — the anchor exists purely + // for the browser's own link affordances. It also has to stay untabbable + // because react-aria treats a tabbable descendant as interactive content + // and then stops the row's own press. + const link = await getRowLink(); + expect(link?.tabIndex).toBe(-1); + expect(link).toHaveAttribute("aria-hidden", "true"); + }); + + test("the anchor sits under the pointer, interactive content above it", async () => { + await render(getTestElementWithLink()); + await expect.element(optionsButton).toBeInTheDocument(); + + const elementAtCenterOf = (element: Element) => { + const { left, top, width, height } = element.getBoundingClientRect(); + return document.elementFromPoint(left + width / 2, top + height / 2); + }; + + // The browser's context menu acts on whatever sits under the pointer, so + // the anchor has to win over the item's plain content … + const link = await getRowLink(); + expect(elementAtCenterOf(await page.getByText("Item: 42").element())).toBe( + link, + ); + + // … and lose against everything the user is meant to interact with. + const button = await optionsButton.element(); + expect(button.contains(elementAtCenterOf(button))).toBe(true); + }); + + test("clicking a linked item navigates exactly once", async () => { + await render(getTestElementWithLink()); + await expect.element(page.getByText("Item: 42")).toBeInTheDocument(); + + await userEvent.click(row); + + // Both the anchor's default action and react-aria's press handler could + // navigate — react-aria cancels the former, so this must stay at one. + expect(navigate).toHaveBeenCalledTimes(1); + expect(navigate).toHaveBeenCalledWith(itemHref, undefined); + expect(onAction).toHaveBeenCalledTimes(1); + }); + + test("keyboard activation navigates", async () => { + await render(getTestElementWithLink()); + await expect.element(page.getByText("Item: 42")).toBeInTheDocument(); + + (await row.element()).focus(); + await userEvent.keyboard("{Enter}"); + + expect(navigate).toHaveBeenCalledTimes(1); + expect(navigate).toHaveBeenCalledWith(itemHref, undefined); + }); + + test("a nested context menu stays clickable and does not trigger the item", async () => { + await render(getTestElementWithLink()); + await expect.element(optionsButton).toBeInTheDocument(); + + await userEvent.click(optionsButton); + await expect + .element(page.getByRole("menuitem", { name: "Delete" })) + .toBeInTheDocument(); + + // Regression guard for #1250's first attempt (#2420, reverted): pressing + // interactive content inside an item must not additionally run the item's + // action or follow its link. + expect(onAction).not.toHaveBeenCalled(); + expect(navigate).not.toHaveBeenCalled(); + }); + + test("a nested menu item runs its own action only", async () => { + await render(getTestElementWithLink()); + await expect.element(optionsButton).toBeInTheDocument(); + + await userEvent.click(optionsButton); + await userEvent.click(page.getByRole("menuitem", { name: "Delete" })); + + expect(menuAction).toHaveBeenCalledWith("delete", undefined); + expect(onAction).not.toHaveBeenCalled(); + expect(navigate).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/components/src/components/List/components/Items/components/Item/Item.module.d.scss.ts b/packages/components/src/components/List/components/Items/components/Item/Item.module.d.scss.ts index bf78f16485..be629de668 100644 --- a/packages/components/src/components/List/components/Items/components/Item/Item.module.d.scss.ts +++ b/packages/components/src/components/List/components/Items/components/Item/Item.module.d.scss.ts @@ -4,6 +4,8 @@ declare const classNames: { readonly hasAction: "hasAction"; readonly "flow--list--items--item--view--bottom-content": "flow--list--items--item--view--bottom-content"; readonly "flow--avatar": "flow--avatar"; + readonly link: "link"; + readonly "flow--list--list-item-view--bottom-content": "flow--list--list-item-view--bottom-content"; readonly tile: "tile"; }; export default classNames; diff --git a/packages/components/src/components/List/components/Items/components/Item/Item.module.scss b/packages/components/src/components/List/components/Items/components/Item/Item.module.scss index 676a1ac59c..85b094996f 100644 --- a/packages/components/src/components/List/components/Items/components/Item/Item.module.scss +++ b/packages/components/src/components/List/components/Items/components/Item/Item.module.scss @@ -1,6 +1,7 @@ @use "@/styles/mixins/focus"; .item { + position: relative; cursor: default; background-color: var(--list-item--background-color--default); transition-property: background-color; @@ -45,6 +46,48 @@ transition-duration: var(--transition--duration--default); } + /* Elements */ + + /* + * Covers the whole item so a right-click anywhere on it hits a real link. + * Interactive content is lifted back above the overlay, so buttons, context + * menus and checkboxes keep receiving their own clicks. + */ + .link { + position: absolute; + inset: 0; + z-index: 1; + } + + &:has(.link) { + :where( + a, + button, + input, + label, + select, + summary, + textarea, + [role="button"], + [role="checkbox"], + [role="link"], + [role="switch"], + [tabindex] + ):not(.link) { + position: relative; + z-index: 2; + } + + /* + * The bottom slot carries arbitrary consumer content (and the accordion's + * expanded content), so it stays fully clickable and selectable. + */ + :global(.flow--list--list-item-view--bottom-content) { + position: relative; + z-index: 2; + } + } + &.tile { border: none; border-radius: var(--list-item--corner-radius); diff --git a/packages/components/src/components/List/components/Items/views/GridListItem/GridListItem.tsx b/packages/components/src/components/List/components/Items/views/GridListItem/GridListItem.tsx index a3cae68b07..0bf8d1d070 100644 --- a/packages/components/src/components/List/components/Items/views/GridListItem/GridListItem.tsx +++ b/packages/components/src/components/List/components/Items/views/GridListItem/GridListItem.tsx @@ -2,6 +2,7 @@ import * as Aria from "react-aria-components"; import type { FC } from "react"; import styles from "@/components/List/components/Items/components/Item/Item.module.scss"; import clsx from "clsx"; +import { useLinkProps } from "@react-aria/utils"; export type GridListItemProps = Aria.GridListItemProps & { hasAction?: boolean; @@ -10,7 +11,27 @@ export type GridListItemProps = Aria.GridListItemProps & { /** @flr-generate all */ export const GridListItem: FC = (props) => { - const { hasAction, isTile, ...restProps } = props; + const { hasAction, isTile, children, ...restProps } = props; + + // React Aria turns `href` into data attributes and navigates on press, so + // the row itself is always a `div`. Overlay a real anchor to give the browser + // back its own link affordances: context menu, middle-click, modifier-click. + const linkProps = useLinkProps(props); + + const linkOverlay = linkProps.href ? ( + // The row keeps owning activation and the accessible semantics. The anchor + // must stay untabbable — React Aria treats a tabbable descendant as + // interactive content and would stop the row's own press — and out of the + // accessibility tree, so screen readers announce the row unchanged. + + ) : null; + return ( = (props) => { renderProps.isSelected && styles.isSelected, ) } - /> + > + {(renderProps) => ( + <> + {typeof children === "function" ? children(renderProps) : children} + {linkOverlay} + + )} + ); }; diff --git a/packages/components/src/components/List/stories/ListItem.stories.tsx b/packages/components/src/components/List/stories/ListItem.stories.tsx index 823138c9d8..2010281a02 100644 --- a/packages/components/src/components/List/stories/ListItem.stories.tsx +++ b/packages/components/src/components/List/stories/ListItem.stories.tsx @@ -157,6 +157,41 @@ export const WithCheckbox: Story = { }, }; +export const WithLink: Story = { + render: () => { + const List = typedList<{ mail: string }>(); + + return ( + + + mail.mail} + href={(mail) => `https://flow.mittwald.de/#${mail.mail}`} + > + {(mail) => ( + + + + + {mail.mail} + + + Right-click, middle-click or Cmd/Ctrl-click the item to open + its link in a new tab. + + + + Show details + + + )} + + + ); + }, +}; + export const WithColumnLayout: Story = { render: () => { const List = typedList<{ mail: string }>();