From a6ae3b8a3b7f3de555d97abcad60a9ee0e4856c9 Mon Sep 17 00:00:00 2001 From: HelloOjasMutreja Date: Mon, 2 Nov 2026 02:18:56 +0530 Subject: [PATCH 1/4] fix(Item): the row's onClick ignores clicks from portals and role-based controls handleContainerClick skipped only button, a, input, select and textarea, and did not check that the target was inside the row. A click on a MoreMenu row in endContent bubbled through the portal, and a click on a role="tab" or role="switch" control ran the row's onClick too. It now asks the shared hasInteractiveAncestor walk (the rule TreeListItem and the Table row plugins use) and ignores a target outside the row's own DOM. Fixes #8977 --- .changeset/item-click-foreign-targets.md | 6 ++ packages/core/src/Item/Item.test.tsx | 67 ++++++++++++++++++++ packages/core/src/Item/Item.tsx | 9 ++- packages/core/src/MoreMenu/MoreMenu.test.tsx | 22 +++++++ 4 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 .changeset/item-click-foreign-targets.md diff --git a/.changeset/item-click-foreign-targets.md b/.changeset/item-click-foreign-targets.md new file mode 100644 index 0000000000000..5c9d40b259328 --- /dev/null +++ b/.changeset/item-click-foreign-targets.md @@ -0,0 +1,6 @@ +--- +'@astryxdesign/core': patch +--- + +[fix] A clickable Item, ListItem or other row with `onClick` no longer runs it for a click that belongs to something else: a click inside a portal rendered from the row (a MoreMenu's menu in `endContent`), or on a role-based control (`role="tab"`, `"switch"`, `"menuitem"` and the rest of the shared interactive list) nested in the row. (#8977) +@HelloOjasMutreja diff --git a/packages/core/src/Item/Item.test.tsx b/packages/core/src/Item/Item.test.tsx index aaf6529f40c06..cf333a521761e 100644 --- a/packages/core/src/Item/Item.test.tsx +++ b/packages/core/src/Item/Item.test.tsx @@ -10,6 +10,7 @@ */ import {Activity, use, useRef, type ReactNode} from 'react'; +import {createPortal} from 'react-dom'; import { afterEach, beforeAll, @@ -1872,3 +1873,69 @@ describe('Item empty slots', () => { expect(screen.getAllByText('0')).toHaveLength(3); }); }); + +describe('Item row onClick: clicks that belong to something else', () => { + it('runs for a click on the row itself', async () => { + const user = userEvent.setup(); + const onClick = vi.fn(); + render(); + await user.click(screen.getByText('Invoice 42')); + expect(onClick).toHaveBeenCalledTimes(1); + }); + + it('does not run for a click inside a portal rendered from the row', async () => { + const user = userEvent.setup(); + const onClick = vi.fn(); + const onPortalClick = vi.fn(); + render( + + Edit + , + document.body, + )} + />, + ); + await user.click(screen.getByTestId('portal-content')); + expect(onPortalClick).toHaveBeenCalledTimes(1); + expect(onClick).not.toHaveBeenCalled(); + }); + + it.each(['tab', 'switch', 'menuitem', 'checkbox', 'option'])( + 'does not run for a click on a role="%s" control in endContent', + async role => { + const user = userEvent.setup(); + const onClick = vi.fn(); + render( + + Control + + } + />, + ); + await user.click(screen.getByTestId('control')); + expect(onClick).not.toHaveBeenCalled(); + }, + ); + + it('does not run for a click on a native control in endContent', async () => { + const user = userEvent.setup(); + const onClick = vi.fn(); + render( + Action} + />, + ); + await user.click(screen.getByRole('button', {name: 'Action'})); + expect(onClick).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/core/src/Item/Item.tsx b/packages/core/src/Item/Item.tsx index 19ac9ef222cc3..5373b47e7c12d 100644 --- a/packages/core/src/Item/Item.tsx +++ b/packages/core/src/Item/Item.tsx @@ -61,6 +61,7 @@ import {computeTargetAndRel} from '../Link/computeTargetAndRel'; import {useLinkComponent} from '../Link/useLinkComponent'; import { cancelNestedTouchPress, + hasInteractiveAncestor, PRESSABLE_CONTAINER_ATTRIBUTE, useClickableContainerProxy, } from '../hooks/useClickableContainer'; @@ -824,7 +825,13 @@ export function Item({ return; } const target = e.target as HTMLElement; - if (target.closest('button, a, input, select, textarea')) { + if ( + // React bubbles a click through a portal rendered from the row (a + // MoreMenu's menu in `endContent`), but its target is not in the row. + !e.currentTarget.contains(target) || + // A nested control, native or role-based, owns the click. + hasInteractiveAncestor(target, e.currentTarget) + ) { return; } if (isLinkRow) { diff --git a/packages/core/src/MoreMenu/MoreMenu.test.tsx b/packages/core/src/MoreMenu/MoreMenu.test.tsx index 8d37cf7b01e59..ecf351c24ad40 100644 --- a/packages/core/src/MoreMenu/MoreMenu.test.tsx +++ b/packages/core/src/MoreMenu/MoreMenu.test.tsx @@ -15,6 +15,7 @@ import {render, screen, fireEvent, waitFor} from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import {MoreMenu} from './MoreMenu'; import {DropdownMenu} from '../DropdownMenu/DropdownMenu'; +import {Item} from '../Item/Item'; import {warmMenuBottomSheet} from '../DropdownMenu/__tests__/warmMenuBottomSheet'; // DropdownMenu's touch sheet is loaded lazily; resolve it once up front. @@ -365,3 +366,24 @@ describe('MoreMenu', () => { }); }); }); + +describe('MoreMenu in a clickable Item', () => { + it('a click on a menu row does not run the Item onClick', async () => { + const user = userEvent.setup(); + const onClick = vi.fn(); + const onEdit = vi.fn(); + render( + } + />, + ); + await user.click(screen.getByRole('button', {name: 'More options'})); + await user.click( + screen.getByRole('menuitem', {name: 'Edit', hidden: true}), + ); + expect(onEdit).toHaveBeenCalledTimes(1); + expect(onClick).not.toHaveBeenCalled(); + }); +}); From 1a806d32e2bebc70042e129240f71d89293e75e9 Mon Sep 17 00:00:00 2001 From: HelloOjasMutreja Date: Mon, 2 Nov 2026 22:10:47 +0530 Subject: [PATCH 2/4] fix(Item): keep the native-control check and narrow the note to Item and ListItem The row's click handler kept its native check (button, a, input, select, textarea, looked up through the ancestors) and adds the portal and role-based checks on top, so a read-only input, or a row inside a link or button, stays inert as before. The changeset names Item and ListItem only. --- .changeset/item-click-foreign-targets.md | 2 +- packages/core/src/Item/Item.test.tsx | 21 +++++++++++++++++++++ packages/core/src/Item/Item.tsx | 5 ++++- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/.changeset/item-click-foreign-targets.md b/.changeset/item-click-foreign-targets.md index 5c9d40b259328..5b39f403cc3a5 100644 --- a/.changeset/item-click-foreign-targets.md +++ b/.changeset/item-click-foreign-targets.md @@ -2,5 +2,5 @@ '@astryxdesign/core': patch --- -[fix] A clickable Item, ListItem or other row with `onClick` no longer runs it for a click that belongs to something else: a click inside a portal rendered from the row (a MoreMenu's menu in `endContent`), or on a role-based control (`role="tab"`, `"switch"`, `"menuitem"` and the rest of the shared interactive list) nested in the row. (#8977) +[fix] A clickable Item or ListItem (one with `onClick`) no longer runs it for a click that belongs to something else: a click inside a portal rendered from the row (a MoreMenu's menu in `endContent`), or on a role-based control (`role="tab"`, `"switch"`, `"menuitem"` and the rest of the shared interactive list) nested in the row. (#8977) @HelloOjasMutreja diff --git a/packages/core/src/Item/Item.test.tsx b/packages/core/src/Item/Item.test.tsx index cf333a521761e..e362359acb534 100644 --- a/packages/core/src/Item/Item.test.tsx +++ b/packages/core/src/Item/Item.test.tsx @@ -1925,6 +1925,27 @@ describe('Item row onClick: clicks that belong to something else', () => { }, ); + it('does not run for a click on a read-only native input in endContent', async () => { + const user = userEvent.setup(); + const onClick = vi.fn(); + render( + + } + />, + ); + await user.click(screen.getByRole('checkbox', {name: 'Paid'})); + expect(onClick).not.toHaveBeenCalled(); + }); + it('does not run for a click on a native control in endContent', async () => { const user = userEvent.setup(); const onClick = vi.fn(); diff --git a/packages/core/src/Item/Item.tsx b/packages/core/src/Item/Item.tsx index 5373b47e7c12d..655f115ae1376 100644 --- a/packages/core/src/Item/Item.tsx +++ b/packages/core/src/Item/Item.tsx @@ -829,7 +829,10 @@ export function Item({ // React bubbles a click through a portal rendered from the row (a // MoreMenu's menu in `endContent`), but its target is not in the row. !e.currentTarget.contains(target) || - // A nested control, native or role-based, owns the click. + // A native control owns the click, as it always has (this also keeps a + // read-only input and a row inside a link or button inert). + target.closest('button, a, input, select, textarea') || + // So does a nested role-based control (role="tab", role="switch"). hasInteractiveAncestor(target, e.currentTarget) ) { return; From 859bc67cd294db5e3418aa9231a06959fd7408ad Mon Sep 17 00:00:00 2001 From: HelloOjasMutreja Date: Tue, 3 Nov 2026 08:55:27 +0530 Subject: [PATCH 3/4] fix(Item): name only caller-made portals and role-based controls; drop the MoreMenu test that passes on main --- .changeset/item-click-foreign-targets.md | 2 +- packages/core/src/Item/Item.tsx | 4 ++-- packages/core/src/MoreMenu/MoreMenu.test.tsx | 22 -------------------- 3 files changed, 3 insertions(+), 25 deletions(-) diff --git a/.changeset/item-click-foreign-targets.md b/.changeset/item-click-foreign-targets.md index 5b39f403cc3a5..482083894c882 100644 --- a/.changeset/item-click-foreign-targets.md +++ b/.changeset/item-click-foreign-targets.md @@ -2,5 +2,5 @@ '@astryxdesign/core': patch --- -[fix] A clickable Item or ListItem (one with `onClick`) no longer runs it for a click that belongs to something else: a click inside a portal rendered from the row (a MoreMenu's menu in `endContent`), or on a role-based control (`role="tab"`, `"switch"`, `"menuitem"` and the rest of the shared interactive list) nested in the row. (#8977) +[fix] A clickable Item or ListItem (one with `onClick`) no longer runs it for a click that belongs to something else: a click inside content the caller portals out of the row with its own `createPortal`, or on a role-based control (`role="tab"`, `"switch"`, `"menuitem"` and the rest of the shared interactive list) nested in the row. (#8977) @HelloOjasMutreja diff --git a/packages/core/src/Item/Item.tsx b/packages/core/src/Item/Item.tsx index 655f115ae1376..7aa68aa3f62f6 100644 --- a/packages/core/src/Item/Item.tsx +++ b/packages/core/src/Item/Item.tsx @@ -826,8 +826,8 @@ export function Item({ } const target = e.target as HTMLElement; if ( - // React bubbles a click through a portal rendered from the row (a - // MoreMenu's menu in `endContent`), but its target is not in the row. + // React bubbles a click through a portal the caller renders from the + // row (its own `createPortal`), but its target is not in the row. !e.currentTarget.contains(target) || // A native control owns the click, as it always has (this also keeps a // read-only input and a row inside a link or button inert). diff --git a/packages/core/src/MoreMenu/MoreMenu.test.tsx b/packages/core/src/MoreMenu/MoreMenu.test.tsx index ecf351c24ad40..8d37cf7b01e59 100644 --- a/packages/core/src/MoreMenu/MoreMenu.test.tsx +++ b/packages/core/src/MoreMenu/MoreMenu.test.tsx @@ -15,7 +15,6 @@ import {render, screen, fireEvent, waitFor} from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import {MoreMenu} from './MoreMenu'; import {DropdownMenu} from '../DropdownMenu/DropdownMenu'; -import {Item} from '../Item/Item'; import {warmMenuBottomSheet} from '../DropdownMenu/__tests__/warmMenuBottomSheet'; // DropdownMenu's touch sheet is loaded lazily; resolve it once up front. @@ -366,24 +365,3 @@ describe('MoreMenu', () => { }); }); }); - -describe('MoreMenu in a clickable Item', () => { - it('a click on a menu row does not run the Item onClick', async () => { - const user = userEvent.setup(); - const onClick = vi.fn(); - const onEdit = vi.fn(); - render( - } - />, - ); - await user.click(screen.getByRole('button', {name: 'More options'})); - await user.click( - screen.getByRole('menuitem', {name: 'Edit', hidden: true}), - ); - expect(onEdit).toHaveBeenCalledTimes(1); - expect(onClick).not.toHaveBeenCalled(); - }); -}); From 83ffa3a77da2bbbf55e8a1c93fe9e1527c1cc992 Mon Sep 17 00:00:00 2001 From: HelloOjasMutreja Date: Tue, 3 Nov 2026 14:32:54 +0530 Subject: [PATCH 4/4] docs(changeset): the Item click scope covers rows with no role --- .changeset/item-click-foreign-targets.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/item-click-foreign-targets.md b/.changeset/item-click-foreign-targets.md index 482083894c882..3e6ef72165910 100644 --- a/.changeset/item-click-foreign-targets.md +++ b/.changeset/item-click-foreign-targets.md @@ -2,5 +2,5 @@ '@astryxdesign/core': patch --- -[fix] A clickable Item or ListItem (one with `onClick`) no longer runs it for a click that belongs to something else: a click inside content the caller portals out of the row with its own `createPortal`, or on a role-based control (`role="tab"`, `"switch"`, `"menuitem"` and the rest of the shared interactive list) nested in the row. (#8977) +[fix] A clickable Item or ListItem (one with `onClick` and no `role`) no longer runs it for a click that belongs to something else: a click inside content the caller portals out of the row with its own `createPortal`, or on a role-based control (`role="tab"`, `"switch"`, `"menuitem"` and the rest of the shared interactive list) nested in the row. (#8977) @HelloOjasMutreja