Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Select] fix: onClick event propagation #3183

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

lucaserly
Copy link

Description

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

@lucaserly lucaserly changed the title Fix select onclick propagation [Select] fix onClick event propagation Oct 20, 2024
@lucaserly lucaserly changed the title [Select] fix onClick event propagation [Select] fix: onClick event propagation Oct 20, 2024
@akselinurmio
Copy link

akselinurmio commented Oct 22, 2024

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
Copy link
Author

lucaserly commented Oct 24, 2024

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
Copy link

akselinurmio commented Oct 25, 2024

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
Copy link
Author

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

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.

2 participants