Skip to content

fix(checkbox, radio): click event is triggered once #27508

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.

Sign up for GitHub

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
wants to merge 4 commits into from
Closed

fix(checkbox, radio): click event is triggered once #27508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ec0ca89
fix(checkbox,radio): click event is triggered once
sean-perkins May 18, 2023
cdf981b
chore: fix path
sean-perkins May 18, 2023
37c9508
chore: remove invalid tests
sean-perkins May 31, 2023
bc633a1
chore: prettier formatting
sean-perkins Jun 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions core/src/components/checkbox/checkbox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,14 @@ export class Checkbox implements ComponentInterface {
this.ionBlur.emit();
};

private onClick = (ev: MouseEvent) => {
// The modern control syntax renders a label as a parent element
// to the checkbox, which means when the label is clicked, the
// checkbox receives this event. To prevent a duplicate click event,
// we need to stop the event from bubbling.
ev.stopPropagation();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does pointer-events: none on the native element work here too? We fixed a similar bug in toggle a couple years ago: #23146

I'd prefer a CSS-only approach rather than adding even more JS.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pointer-events: none on the inner input control continues to result in duplicate clicks. The problem is the label invokes a click event on the input and does not take into consideration the pointer events.

};

// TODO(FW-3100): run contents of renderCheckbox directly instead
render() {
const { legacyFormController } = this;
Expand Down Expand Up @@ -256,6 +264,7 @@ export class Checkbox implements ComponentInterface {
id={inputId}
onChange={this.toggleChecked}
onFocus={() => this.onFocus()}
onClick={this.onClick}
onBlur={() => this.onBlur()}
ref={(focusEl) => (this.focusEl = focusEl)}
{...inheritedAttributes}
Expand Down
18 changes: 17 additions & 1 deletion core/src/components/radio/radio.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ export class Radio implements ComponentInterface {
};

private onClick = () => {
this.checked = this.nativeInput.checked;
this.setChecked();
};

private onFocus = () => {
Expand All @@ -211,6 +211,14 @@ export class Radio implements ComponentInterface {
this.ionBlur.emit();
};

private toggleChecked = () => {
this.setChecked();
};

private setChecked() {
this.checked = this.nativeInput.checked;
}

private get hasLabel() {
return this.el.textContent !== '';
}
Expand Down Expand Up @@ -262,6 +270,14 @@ export class Radio implements ComponentInterface {
id={inputId}
ref={(nativeEl) => (this.nativeInput = nativeEl as HTMLInputElement)}
{...inheritedAttributes}
onChange={this.toggleChecked}
onClick={(ev) => {
// The modern control syntax renders a label as a parent element
// to the radio, which means when the label is clicked, the
// radio receives this event. To prevent a duplicate click event,
// we need to stop the event from bubbling.
ev.stopPropagation();
}}
/>
<div
class={{
Expand Down