Skip to content

fix(ListItem): render linked items as real anchors - #3084

Merged
mfal merged 3 commits into
mainfrom
fix/list-item-link-new-tab-1250
Sep 4, 2026
Merged

fix(ListItem): render linked items as real anchors#3084
mfal merged 3 commits into
mainfrom
fix/list-item-link-new-tab-1250

Conversation

@mfal

@mfal mfal commented Sep 2, 2026

Copy link
Copy Markdown
Member

Linked list items now render a real <a href>, so the browser's own link
affordances work on them again.

Root cause

Read off the DOM: the row is a div with data-href and no anchor anywhere
inside it.

React Aria's GridListItem renders dom.div unconditionally
(react-aria-components@1.20.0, GridList.mjs). ListBoxItem has a
props.href ? dom.a : dom.div switch — GridListItem and table Row do not,
because a grid row is meant to hold nested interactive content and interactive
content may not nest inside <a>.

Instead, useGridListItem passes href through useSyntheticLinkProps, which
puts data-href/data-target on the row. On press, openLink's
getSyntheticLink creates an <a>, appends it to the row, dispatches a click on
it and removes it again. So:

  • context menu: nothing under the pointer is a link → no "open in new tab"
  • Cmd/Ctrl+Click: worked, because usePress reads the modifier keys and forwards
    them to that throwaway anchor
  • middle-click: nothing to middle-click

What changed

GridListItem overlays a real anchor when the item has a href
(useLinkProps resolves it through the RouterProvider, so a basePath is
applied the same way react-aria would):

<a {...linkProps} aria-hidden className={styles.link} draggable={false} tabIndex={-1} />
  • Activation stays with the row. React Aria already installs
    onClick = e => { if (!openLink.isOpening) e.preventDefault() } on link items
    — the contract for "the item is an anchor", which ListBoxItem relies on.
    A user click on the overlay bubbles to the row, gets its default cancelled,
    and router.open navigates. Client-side routing and exactly-one-navigation
    are unchanged.
  • tabIndex={-1} is load-bearing, not cosmetic. useSelectableItem guards
    the row's press with isTabbable(target) — a tabbable descendant is treated as
    interactive content and stops the row's press. A tabbable overlay would make a
    left click do nothing at all.
  • aria-hidden keeps the accessibility tree exactly as it is: the row keeps
    announcing itself, no second link, no interactive content inside a link.
  • CSS puts the overlay at z-index: 1 and lifts interactive content
    (a, button, input, label, …, [tabindex]) plus the whole bottom slot to
    z-index: 2. The bottom slot carries arbitrary consumer content and the
    accordion's expanded content, so it stays clickable and selectable.

This is where the first attempt (#2420, reverted) went wrong: replacing
GridList with ListBox also ran the item action when interactive content
inside an item was pressed. ListBox options are not built for nested
interactive content; GridList is, and it stays.

Compatibility for remote consumers

No prop changed — not on List/ListItem, not on the @flr-generate
GridListItem. href and target were already the API; only the host-side
rendering of them changed. pnpm build produces no diff under
src/auto-generated/**, no new view.ts, no src/views/* change. Nothing to
deprecate, so no useWarnDeprecation path is needed.

Verification

Tests in packages/components/src/components/List/List.browser.test.tsx
(describe("Linked items")), all run in webkit:

Path How it is covered
Context menu A real <a href> exists in the row, and document.elementFromPoint over the item's content returns that anchor — what the browser's context menu acts on
Middle-click Same anchor plus its target attribute; the browser handles auxclick natively and react-aria never sees a click to cancel
Cmd/Ctrl+Click Left unchanged on purpose — react-aria's press path already handles it (the one thing that worked before). Not clicked in a test, because it would open a real tab in the test browser
Keyboard Row focused, {Enter}navigate called once
Nested interactive content Options button opens its menu; onAction and navigate are not called (regression guard for #2420). Choosing a menu item runs only the menu's action
Layering elementFromPoint over the Options button returns the button, not the overlay
No leak An item without href renders no anchor

The layering tests were verified to actually guard: raising the overlay above
everything makes three of them fail, two with Playwright's
<a …> intercepts pointer events.

Also run: pnpm nx test:browser components --browser.name=webkit (38 files, 271
tests), pnpm nx test:unit components (253), pnpm nx test:compile components,
pnpm lint (0 errors), pnpm build (no generated diff).

Visual: no pixel change — the overlay is transparent and layout-neutral. The
18 List and 2 FileCardList visual scenarios pass unchanged against the local
-darwin baselines, in both the Local and Remote environments. No new
scenario was added: a new one would have no committed baseline and would fail
the verify-only run. run-visual-tests is set so CI confirms this on Linux.

Manual: apps/remote-dom-demo /remote/list now sets a href, so the
whole remote path can be right-clicked / middle-clicked over the real iframe
connection. Storybook has a ListItem/WithLink story for the local path.

Left out

Item.module.scss contains a dead selector next to the new rules:
:global(.flow--list--items--item--view--bottom-content) — the rendered class is
flow--list--list-item-view--bottom-content, so the intended "no item hover
while hovering the bottom content" carve-out never applied. Pre-existing, a
visual change to fix, and unrelated to this bug — filed separately rather than
folded in here.

fixes #1250

🤖 Generated with Claude Code

@mfal mfal self-assigned this Sep 2, 2026
@mfal mfal added the run-visual-tests Runs the full visual regression suite against the existing baselines and fails the check on mismatch label Sep 2, 2026
@github-actions github-actions Bot removed the run-visual-tests Runs the full visual regression suite against the existing baselines and fails the check on mismatch label Sep 2, 2026
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Coverage Report for ./packages/components/

Status Category Percentage Covered / Total
🔵 Lines 78.69% 746 / 948
🔵 Statements 78.57% 763 / 971
🔵 Functions 80.09% 165 / 206
🔵 Branches 70.33% 377 / 536
File CoverageNo changed files found.
Generated in workflow #6672 for commit 0c1e233 by the Vitest Coverage Report Action

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

🚀 Preview Deployment

Preview environments are ready:

Type URL
docs pr-3084.docs.review.flow-components.de
storybook pr-3084.storybook.review.flow-components.de

Images:

  • docs: ghcr.io/mittwald/flow/docs:pr-3084
  • storybook: ghcr.io/mittwald/flow/storybook:pr-3084

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

✅ Visual Regression Tests Passed

All visual snapshots match the committed baselines.

Run details

React Aria's `GridListItem` always renders a `div` — unlike `ListBoxItem`, it
has no `href ? 'a' : 'div'` switch. `useGridListItem` turns `href` into
`data-href`/`data-target` and navigates on press by creating a throwaway `<a>`,
clicking it and removing it again. The DOM therefore never held a link, so the
browser had nothing to offer in its context menu. Cmd+Click only worked because
`usePress` reads the modifier keys and forwards them to that throwaway anchor.

Overlay a real `<a href>` inside the row. The row keeps owning activation:
react-aria already installs `onClick = e => { if (!openLink.isOpening)
e.preventDefault() }` for link items, which cancels the anchor's default action
on a user click and lets `router.open` navigate — so client-side routing and the
single-navigation guarantee are unchanged. Context menu, middle-click and
modifier-click are handled by the browser on the anchor itself.

The anchor is `tabIndex={-1}` and `aria-hidden`. Untabbable is load-bearing:
`useSelectableItem` treats a tabbable descendant as interactive content
(`isTabbable`) and stops the row's own press. Hidden from the accessibility tree
keeps the row's announcement unchanged.

CSS lifts interactive content and the bottom slot back above the overlay, so
context menus, buttons and checkboxes keep receiving their own clicks. This is
what broke #2420 (reverted): switching to `ListBox` also ran the item action
when interactive content inside an item was pressed.

No prop changed, so the remote contract and the generated artifacts stay as they
are.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mfal mfal added run-visual-tests Runs the full visual regression suite against the existing baselines and fails the check on mismatch run-cross-version-tests labels Sep 3, 2026
@github-actions github-actions Bot removed run-visual-tests Runs the full visual regression suite against the existing baselines and fails the check on mismatch run-cross-version-tests labels Sep 3, 2026
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

✅ Cross-Version Tests Passed

The current host still renders old published remote versions correctly (iframe) and in the same DOM shape (in-process).

Iframe harness (attribute-accurate): ✅ passed

In-process harness (structure-only): ✅ passed

Run details

@mfal mfal added run-cross-version-tests run-visual-tests Runs the full visual regression suite against the existing baselines and fails the check on mismatch labels Sep 4, 2026
@github-actions github-actions Bot removed run-visual-tests Runs the full visual regression suite against the existing baselines and fails the check on mismatch run-cross-version-tests labels Sep 4, 2026
@mfal
mfal merged commit de6dc0a into main Sep 4, 2026
37 checks passed
@mfal
mfal deleted the fix/list-item-link-new-tab-1250 branch September 4, 2026 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Linked ListItems can not be opened in a new tab

2 participants