Skip to content

Commit

Permalink
fix(radio-button, radio-button-group): prevent emitting events when s…
Browse files Browse the repository at this point in the history
…electing a checked radio button (#7102)

**Related Issue:** #6712

## Summary

- Prevents emitting `calciteRadioButtonChange` and
`calciteRadioButtonGroupChange` events when the `checked` property is
already true.
- Update tests
  • Loading branch information
driskull authored Jun 15, 2023
1 parent c9215c0 commit 77fcc81
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -458,15 +458,15 @@ describe("calcite-radio-button-group", () => {
expect(changeEvent).toHaveReceivedEventTimes(0);

await firstRadio.click();
expect(changeEvent).toHaveReceivedEventTimes(1);
expect(changeEvent).toHaveReceivedEventTimes(0);
expect(await getSelectedItemValue()).toBe("one");

await secondRadio.click();
expect(changeEvent).toHaveReceivedEventTimes(2);
expect(changeEvent).toHaveReceivedEventTimes(1);
expect(await getSelectedItemValue()).toBe("two");

await thirdRadio.click();
expect(changeEvent).toHaveReceivedEventTimes(3);
expect(changeEvent).toHaveReceivedEventTimes(2);
expect(await getSelectedItemValue()).toBe("three");
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
reflects,
renders
} from "../../tests/commonTests";
import { html } from "../../../support/formatting";

describe("calcite-radio-button", () => {
describe("renders", () => {
Expand Down Expand Up @@ -184,6 +185,33 @@ describe("calcite-radio-button", () => {
expect(value).toBe("1");
});

it("should not emit calciteRadioButtonChange when checked already", async () => {
const page = await newE2EPage();
await page.setContent(html`<calcite-radio-button-group name="Options" layout="vertical">
<calcite-label layout="inline">
<calcite-radio-button checked value="trees"></calcite-radio-button>
Trees
</calcite-label>
<calcite-label layout="inline">
<calcite-radio-button value="layers" shrubs></calcite-radio-button>
Shrubs
</calcite-label>
</calcite-radio-button-group>`);

const checkedRadio = await page.find("calcite-radio-button[checked]");
expect(await checkedRadio.getProperty("checked")).toBe(true);

const changeEvent = await checkedRadio.spyOnEvent("calciteRadioButtonChange");

expect(changeEvent).toHaveReceivedEventTimes(0);

await checkedRadio.click();
await page.waitForChanges();
expect(await checkedRadio.getProperty("checked")).toBe(true);

expect(changeEvent).toHaveReceivedEventTimes(0);
});

it("clicking a radio updates its checked status", async () => {
const page = await newE2EPage();
await page.setContent(`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,17 @@ export class RadioButton
if (this.disabled) {
return;
}

this.focused = true;
this.setFocus();

if (this.checked) {
return;
}

this.uncheckAllRadioButtonsInGroup();
this.checked = true;
this.focused = true;
this.calciteRadioButtonChange.emit();
this.setFocus();
};

private clickHandler = (): void => {
Expand All @@ -204,25 +210,34 @@ export class RadioButton
};

onLabelClick(event: CustomEvent): void {
if (!this.disabled && !this.hidden) {
this.uncheckOtherRadioButtonsInGroup();
const label = event.currentTarget as HTMLCalciteLabelElement;
const radioButton = label.for
? this.rootNode.querySelector<HTMLCalciteRadioButtonElement>(
`calcite-radio-button[id="${label.for}"]`
)
: label.querySelector<HTMLCalciteRadioButtonElement>(
`calcite-radio-button[name="${this.name}"]`
);

if (radioButton) {
radioButton.checked = true;
radioButton.focused = true;
}
if (this.disabled || this.hidden) {
return;
}

this.calciteRadioButtonChange.emit();
this.setFocus();
const label = event.currentTarget as HTMLCalciteLabelElement;

const radioButton = label.for
? this.rootNode.querySelector<HTMLCalciteRadioButtonElement>(
`calcite-radio-button[id="${label.for}"]`
)
: label.querySelector<HTMLCalciteRadioButtonElement>(
`calcite-radio-button[name="${this.name}"]`
);

if (!radioButton) {
return;
}

radioButton.focused = true;
this.setFocus();

if (radioButton.checked) {
return;
}

this.uncheckOtherRadioButtonsInGroup();
radioButton.checked = true;
this.calciteRadioButtonChange.emit();
}

private checkLastRadioButton(): void {
Expand Down

0 comments on commit 77fcc81

Please sign in to comment.