-
Notifications
You must be signed in to change notification settings - Fork 13.5k
fix(labels): prevent clicking a label from triggering onClick twice on several components #30384
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
Conversation
…extarea, toggle, input) where clicking on a label would trigger onclick twice
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…igger onclick twice
Co-authored-by: Maria Hutt <thetaPC@users.noreply.github.com>
Co-authored-by: Maria Hutt <thetaPC@users.noreply.github.com>
Co-authored-by: Maria Hutt <thetaPC@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just missing two annotations but not blockers to move to testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested the click events locally, including inside of an ion-item
and verified all click events fire once now. Also verified the e2e tests fail without the fixes. Suggested a change to a couple test names for consistency. Looks good though! Great work. 🚀
Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
Issue number: resolves #30165
What is the current behavior?
Currently, several components will trigger their
onClick
twice if you click on their labels.What is the new behavior?
After this fix, the affected components will only trigger
onClick
once per click of their labels or click directly on the element.Does this introduce a breaking change?
Other information
The affected components are:
I also tested radio and range but could not reproduce the issue for them.
Note that two of the components, checkbox and toggle, had to have special implementations for both their test and fix because of how the host component acts as the component for accessibility purposes.
Current dev build:
8.5.7-dev.11746044124.147aab6c