Skip to content

Commit

Permalink
fix(radio-button, radio-button-group): emit change events only when c…
Browse files Browse the repository at this point in the history
…hecked is toggled. #6633
  • Loading branch information
driskull committed Jun 2, 2023
1 parent 9e48114 commit f7f1753
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 5 deletions.
6 changes: 3 additions & 3 deletions src/components/radio-button-group/radio-button-group.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -441,15 +441,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");
});
});
28 changes: 28 additions & 0 deletions src/components/radio-button/radio-button.e2e.ts
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 @@ -183,6 +184,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
4 changes: 2 additions & 2 deletions src/components/radio-button/radio-button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ export class RadioButton
};

check = (): void => {
if (this.disabled) {
if (this.disabled || this.checked) {
return;
}
this.uncheckAllRadioButtonsInGroup();
Expand All @@ -204,7 +204,7 @@ export class RadioButton
};

onLabelClick(event: CustomEvent): void {
if (!this.disabled && !this.hidden) {
if (!this.disabled && !this.hidden && !this.checked) {
this.uncheckOtherRadioButtonsInGroup();
const label = event.currentTarget as HTMLCalciteLabelElement;
const radioButton = label.for
Expand Down

0 comments on commit f7f1753

Please sign in to comment.