Skip to content

bug: ion-checkbox fires onclick twice #27506

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

Closed
3 tasks done
corysmc opened this issue May 18, 2023 · 6 comments
Closed
3 tasks done

bug: ion-checkbox fires onclick twice #27506

corysmc opened this issue May 18, 2023 · 6 comments
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@corysmc
Copy link
Contributor

corysmc commented May 18, 2023

Prerequisites

Ionic Framework Version

v7.x

Current Behavior

If an ion-checkbox has an onClick listener - it will fire onClick twice

Expected Behavior

Should only fire onClick once when you click the checkbox.

Steps to Reproduce

  1. Use an IonCheckbox with an onClick
  2. notice that the onClick is called twice when you click once

Code Reproduction URL

https://stackblitz.com/edit/8t9ymz?file=src%2Fmain.tsx

Ionic Info

See stackblitz repro above - not using ionic CLI

Additional Information

Maybe related - not sure: #27492

@ionitron-bot ionitron-bot bot added the triage label May 18, 2023
@corysmc
Copy link
Contributor Author

corysmc commented May 18, 2023

Here's a vanilla reproduction as well: https://stackblitz.com/edit/jd93fs?file=index.html

@sean-perkins sean-perkins self-assigned this May 18, 2023
@sean-perkins
Copy link
Contributor

Hello @corysmc thanks for reporting this issue! I can reproduce it with both ion-checkbox and ion-radio. It happens where we have a shadow DOM web component with the modern form control syntax and a label + input.

e.g.:

<my-component>
  #shadow-root
    <label>
      <input />
    </label>
</my-component>

The label is the click target. When a label is associated with an input, it will trigger a click on the inner input, which propagates the additional click event.

@sean-perkins sean-perkins added package: core @ionic/core package type: bug a confirmed bug report labels May 18, 2023
@ionitron-bot ionitron-bot bot removed the triage label May 18, 2023
@sean-perkins
Copy link
Contributor

Here is a dev-build to test with: 7.0.8-dev.11684439870.1ade4229

Forked reproduction with the dev-build: https://stackblitz.com/edit/8t9ymz-6biavu?file=package.json

@sean-perkins sean-perkins removed their assignment May 18, 2023
@corysmc
Copy link
Contributor Author

corysmc commented May 18, 2023

Awesome, thanks @sean-perkins !

fcamblor added a commit to voxxrin/voxxrin3 that referenced this issue Jun 4, 2023
Note that I had to upgrade ionic version to a dev nightly build because of bug on ion-checkbox triggering @click twice
see: ionic-team/ionic-framework#27506
liamdebeasi added a commit to ionic-team/ionic-docs that referenced this issue Jun 9, 2023
There is some confusion around event retargeting: ionic-team/ionic-framework#27506. I added docs on how events work in Ionic.
@liamdebeasi
Copy link
Contributor

Hi everyone,

I discussed this with the team, and we have determined this is not a bug in Ionic.

This behavior is happening because the click event is being retargeted as it moves from the Shadow DOM to the Light DOM. In the case of ion-checkbox, clicking the checkbox dispatches a click on the <label>. However, clicking the label also dispatches a click on the native input inside of the label. That click event then bubbles up and gets retargeted. This results in what look like a duplicate click event but is really two separate click events.

You can see this here: https://codepen.io/liamdebeasi/pen/OJaPqpR

There appears to be duplicate click events, but calling composedPath() reveals that the click events are coming from two separate elements.

We have decided not to suppress this behavior because it would mean suppressing what the browser is designed to do.

While you can use composedPath to differentiate the events, I don't recommend doing so because this exposes component internals which are subject to change. In this case, we recommend relying on Ionic events such as ionChange. I also proposed a PR to clarify this behavior on the docs: ionic-team/ionic-docs#2992

I am going to close this, but let me know if there are any questions.

@liamdebeasi liamdebeasi closed this as not planned Won't fix, can't repro, duplicate, stale Jun 9, 2023
@ionitron-bot
Copy link

ionitron-bot bot commented Jul 9, 2023

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Jul 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants