Button onPointerUp event bubbling to MenuItem.

[AlexanderKositsyn]

Provide a general summary of the issue here

Button onPointerUp event bubbling to MenuItem.

🤔 Expected Behavior?

Button calls stopPropagation on onPointerUp event

😯 Current Behavior

Button onPointerUp event bubbling to MenuItem. MenuItem catches onPointerUp event

💁 Possible Solution

No response

🔦 Context

No response

🖥️ Steps to Reproduce

<MenuItem>
  <Button
      onPress={(e) => {
        triggerStateContext.close();
      }}
    >
        Close
  </Button>
</MenuItem>

Version

react-aria-components 1.1.1

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

macos

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response

[LFDanLu]

Historically, we've had the stance that menuitem shouldn't support interactive children (like a button) due to the ARIA spec saying only presentational children are allowed. It is a bit ambiguous as to whether that is still the case or not since I don't see "Children Presentational: true" listed here anymore, only for menuitemcheckbox and menuitemradio, but w3c/aria#1711 seems to imply that only a submenu is allowed as a non-presentational child for a menu and thus a button still isn't allowed.

[AlexanderKositsyn]

@LFDanLu Im not talking about Button in MenuItem. I say that Button should stop onPointerUp event propagation.
Here example without ManeItem.
https://codesandbox.io/p/sandbox/dreamy-visvesvaraya-866sp2
As you can see onPointerUp event bubbles up to div. But as react aria components doc says Button should not propagate this event.

[sookmax]

As you can see onPointerUp event bubbles up to div. But as react aria components doc says Button should not propagate this event.

I found a related test, and by looking at it, I'm afraid it doesn't work like that?

react-spectrum/packages/@react-aria/interactions/test/usePress.test.js

Lines 2871 to 2894 in b2c9961

	it('should stop propagation by default', () => {
	let outerPressMock = jest.fn();
	let innerPressMock = jest.fn();
	let res = render(
	<Pressable
	onPressStart={outerPressMock}
	onPressEnd={outerPressMock}
	onPress={outerPressMock}>
	<Pressable
	data-testid="test"
	onPressStart={innerPressMock}
	onPressEnd={innerPressMock}
	onPress={innerPressMock}>
	inner
	</Pressable>
	</Pressable>
	);
	let el = res.getByTestId('test');
	start(el);
	end(el);
	expect(outerPressMock.mock.calls).toHaveLength(0);
	expect(innerPressMock.mock.calls).toHaveLength(3);
	});

Looks like only press events (i.e., onPress, onPressStart, onPressEnd) are guaranteed not to propagate to the parent by default.

[AlexanderKositsyn]

@sookmax There are no onPress onPressStart onPressEnd events in DOM. They are synthetic in react-aria. Events that are triggered by onPress, onPressStart, onPressEnd must be stop propagated.

[LFDanLu]

Apologies for the delay. We can't actually stop propagation on the pointer up events because we actually rely on listening to that event here. I haven't tried but I don't believe we could change that listener to capturing + stopping propagation without breaking a bunch of things in usePress and related components.

[snowystinger]

@LFDanLu I think that's only there for our hit testing, so I think this should be fixed when we do #1720

[snowystinger]

Behaviour appears unchanged in 3.40 https://codesandbox.io/p/sandbox/zealous-euler-ql8kgm?file=%2Fsrc%2FApp.js%3A12%2C1
Might be related to leaking events @LFDanLu ?