diff --git a/.changeset/item-click-foreign-targets.md b/.changeset/item-click-foreign-targets.md new file mode 100644 index 0000000000000..3e6ef72165910 --- /dev/null +++ b/.changeset/item-click-foreign-targets.md @@ -0,0 +1,6 @@ +--- +'@astryxdesign/core': patch +--- + +[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 diff --git a/packages/core/src/Item/Item.test.tsx b/packages/core/src/Item/Item.test.tsx index a36e05f5de792..5cdc72ac44542 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, @@ -1967,3 +1968,90 @@ 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 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(); + 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 7867977d2cf7a..d9271a458dfb1 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'; @@ -825,7 +826,16 @@ 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 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). + target.closest('button, a, input, select, textarea') || + // So does a nested role-based control (role="tab", role="switch"). + hasInteractiveAncestor(target, e.currentTarget) + ) { return; } if (isLinkRow) {