radix-ui / primitives

Radix Primitives is an open-source UI component library for building high-quality, accessible design systems and web apps. Maintained by @workos.
https://radix-ui.com/primitives
MIT License
15.93k stars 833 forks source link

[Select] fix: onClick event propagation #3183

Open lucaserly opened 1 month ago

lucaserly commented 1 month ago

Description

Prevent Select.Root's onValueChange event to propagate to other event handlers.

akselinurmio commented 1 month ago

Why would this be needed? Is there some reason you can't do this in your codebase? I would be very careful when planning to call stopPropagation anywhere, as it makes it harder to predict the behaviour of other event handlers listening to the same event.

lucaserly commented 1 month ago

This would be needed because Select's onValueChange triggers event bubbling, which causes unwanted actions on parent elements. I noticed this happening only on mobile version. Tried on an actual phone as well as in device mode on Chrome. Unfortunately, I can't handle this in my codebase since onValueChange doesn't provide access to the event object. I understand the caution around using stopPropagation() and agree it should be used carefully. If you have a better way to prevent this unintended behaviour without affecting other event listeners, I'd love to hear it. Let me know if a specific example would help clarify.

akselinurmio commented 1 month ago

I understand and I would like to try and help you. You can add event listeners to the event.target element itself, or any parent of the event.target and the event stops bubbling to any further parents. Radix Primitives components call the event handler props you pass to them in the event handlers they add (see here). So instead of calling event.stopPropagation() in Radix UI code, could you try out if adding a new event listener to the target element as onClick prop in your app code would solve the issue?

For example:

<Select.Root>
  {...}
  <Select.Portal>
    <Select.Content>
      {...}
      <Select.Item onClick={event => event.stopPropagation()}>
        {...}
      </Select.Item>
    </Select.Content>
  </Select.Portal>
</Select.Root>
lucaserly commented 4 weeks ago

@akselinurmio thanks for the suggestion. Tried but still doesn't work.