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

fix(select): click handlers on slotted content fire #28839

Merged
merged 3 commits into from
Jan 17, 2024
Merged

fix(select): click handlers on slotted content fire #28839

merged 3 commits into from
Jan 17, 2024

Conversation

liamdebeasi
Copy link
Contributor

@liamdebeasi liamdebeasi commented Jan 16, 2024

Issue number: resolves #28818


What is the current behavior?

Select has logic in place to prevent the overlay from opening when clicking the slotted content. As part of this, we need to deal with the <label> element dispatching another click that then bubbles up and causes the overlay to open when clicking the slotted content. We currently deal with this by calling preventDefault which prevents this extra event from being dispatched only when clicking the slotted content.

We also call stopPropagation. This code is not necessary, and in most cases should not have any adverse effects on developer applications. However, this does impact React applications due to how React handles events. When a developer places onClick on a slotted element, a native "click" listener is not immediately applied to that element. Instead, React adds a native "click" listener to the root element of an application. When that listener's callback fires, React will dispatch a synthetic click event on the slotted element. Since we are calling stopPropagation, the click event never bubbles up to this root node's listener. As a result, the synthetic event is never dispatched on the slotted element, and the developed onClick callback never fires.

What is the new behavior?

  • Select no longer prevents events from bubbling. The existing event.preventDefault is sufficient to prevent the label from dispatching another click (thereby causing the select overlay to open) when clicking the slotted content.

Does this introduce a breaking change?

  • Yes
  • No

Other information

Dev build: 7.6.5-dev.11705430252.1023daf3

@liamdebeasi liamdebeasi changed the title Fw 5895 fix(select): click handlers on slotted content fire Jan 16, 2024
@github-actions github-actions bot added the package: core @ionic/core package label Jan 16, 2024
@liamdebeasi liamdebeasi marked this pull request as ready for review January 16, 2024 18:44
@liamdebeasi liamdebeasi requested a review from a team as a code owner January 16, 2024 18:44
@liamdebeasi liamdebeasi requested review from thetaPC and removed request for a team January 16, 2024 18:44
@averyjohnston averyjohnston self-requested a review January 16, 2024 18:49
@liamdebeasi liamdebeasi added this pull request to the merge queue Jan 17, 2024
@liamdebeasi liamdebeasi removed this pull request from the merge queue due to a manual request Jan 17, 2024
@liamdebeasi liamdebeasi added this pull request to the merge queue Jan 17, 2024
Merged via the queue into main with commit aed7a03 Jan 17, 2024
46 checks passed
@liamdebeasi liamdebeasi deleted the FW-5895 branch January 17, 2024 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: v7 - React - onClick doesn't work in ionButton inside ionSelect
2 participants