Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion apps/remote-dom-demo/src/app/remote/list/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,13 @@ export default function Page() {
});
}}
</DemoList.LoaderAsync>
<DemoList.Item textValue={(d) => 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. */}
<DemoList.Item
textValue={(d) => d.name}
href={(d) => `#${d.name}`}
showTiles
>
{(d) => {
const c = useModalController();
return (
Expand Down
156 changes: 155 additions & 1 deletion packages/components/src/components/List/List.browser.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,18 @@ 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,
type SettingsJson,
} 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;
Expand Down Expand Up @@ -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) => (
<RouterProvider navigate={navigate}>
<List aria-label="Test" onAction={onAction}>
<ListStaticData<Data> data={[{ num: 42 }]} />
<ListItem<Data>
textValue={({ num }) => String(num)}
href={({ num }) => `${location.origin}/domains/${num}`}
target={target}
>
{({ num }) => (
<ListItemView>
<Heading>Item: {num}</Heading>
<ContextMenu onAction={menuAction}>
<MenuItem id="delete">Delete</MenuItem>
</ContextMenu>
</ListItemView>
)}
</ListItem>
</List>
</RouterProvider>
);

const row = page.getByRole("row");
const optionsButton = page.getByRole("button", { name: "Options" });

const getRowLink = async () =>
(await row.element()).querySelector<HTMLAnchorElement>("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 <a href> 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();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<never> & {
hasAction?: boolean;
Expand All @@ -10,7 +11,27 @@ export type GridListItemProps = Aria.GridListItemProps<never> & {

/** @flr-generate all */
export const GridListItem: FC<GridListItemProps> = (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.
<a
{...linkProps}
aria-hidden
className={styles.link}
draggable={false}
tabIndex={-1}
/>
) : null;

return (
<Aria.GridListItem
{...restProps}
Expand All @@ -22,7 +43,14 @@ export const GridListItem: FC<GridListItemProps> = (props) => {
renderProps.isSelected && styles.isSelected,
)
}
/>
>
{(renderProps) => (
<>
{typeof children === "function" ? children(renderProps) : children}
{linkOverlay}
</>
)}
</Aria.GridListItem>
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,41 @@ export const WithCheckbox: Story = {
},
};

export const WithLink: Story = {
render: () => {
const List = typedList<{ mail: string }>();

return (
<List.List aria-label="Mail addresses">
<List.StaticData data={[{ mail: "luke.skywalker@rebellion.org" }]} />
<List.Item
showTiles
textValue={(mail) => mail.mail}
href={(mail) => `https://flow.mittwald.de/#${mail.mail}`}
>
{(mail) => (
<List.ItemView>
<Avatar>
<IconEmail />
</Avatar>
<Heading>{mail.mail}</Heading>
<Content slot="bottom">
<Text>
Right-click, middle-click or Cmd/Ctrl-click the item to open
its link in a new tab.
</Text>
</Content>
<ContextMenu>
<MenuItem>Show details</MenuItem>
</ContextMenu>
</List.ItemView>
)}
</List.Item>
</List.List>
);
},
};

export const WithColumnLayout: Story = {
render: () => {
const List = typedList<{ mail: string }>();
Expand Down
Loading