From 7e51b41837604f26c992797be4ec86e44f643bc9 Mon Sep 17 00:00:00 2001 From: anveshmekala Date: Wed, 7 Jun 2023 17:21:39 -0500 Subject: [PATCH 1/7] forceUpdate other radiobuttons in group --- .../radio-button/radio-button.e2e.ts | 56 +++++++++++++++++++ .../components/radio-button/radio-button.tsx | 24 +++++++- 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts b/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts index d71b8e85ffa..e66dc995b8b 100644 --- a/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts +++ b/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts @@ -10,6 +10,8 @@ import { reflects, renders } from "../../tests/commonTests"; +import { html } from "../../../support/formatting"; +import { getFocusedElementProp } from "../../tests/utils"; describe("calcite-radio-button", () => { describe("renders", () => { @@ -452,4 +454,58 @@ describe("calcite-radio-button", () => { describe("is form-associated", () => { formAssociated("calcite-radio-button", { testValue: true, inputType: "radio" }); }); + + it("focuses first focusable item on Tab", async () => { + const page = await newE2EPage(); + await page.setContent(html` + + + + Trees + + + + Shrubs + + + + Flowers + + + submit + `); + const group = await page.find("calcite-radio-button-group"); + await group.press("Tab"); + await page.waitForChanges(); + expect(await getFocusedElementProp(page, "id")).toBe("shrubs"); + await group.press("Tab"); + expect(await getFocusedElementProp(page, "id")).toBe("submit"); + }); + + it("focuses checked item on Tab", async () => { + const page = await newE2EPage(); + await page.setContent(html` + + + + Trees + + + + Shrubs + + + + Flowers + + + submit + `); + const group = await page.find("calcite-radio-button-group"); + await group.press("Tab"); + await page.waitForChanges(); + expect(await getFocusedElementProp(page, "id")).toBe("flowers"); + await group.press("Tab"); + expect(await getFocusedElementProp(page, "id")).toBe("submit"); + }); }); diff --git a/packages/calcite-components/src/components/radio-button/radio-button.tsx b/packages/calcite-components/src/components/radio-button/radio-button.tsx index f91f42fa86d..33141a6890f 100644 --- a/packages/calcite-components/src/components/radio-button/radio-button.tsx +++ b/packages/calcite-components/src/components/radio-button/radio-button.tsx @@ -3,6 +3,7 @@ import { Element, Event, EventEmitter, + forceUpdate, h, Host, Listen, @@ -179,9 +180,11 @@ export class RadioButton ) as HTMLCalciteRadioButtonElement[]; }; - isDefaultSelectable = (): boolean => { + isFocusable = (): boolean => { const radioButtons = this.queryButtons(); - return !radioButtons.some((radioButton) => radioButton.checked) && radioButtons[0] === this.el; + const firstFocusable = radioButtons.find((radiobutton) => !radiobutton.disabled); + const checked = radioButtons.find((radiobutton) => radiobutton.checked); + return firstFocusable === this.el && !checked; }; check = (): void => { @@ -271,11 +274,26 @@ export class RadioButton }); } + private updateTabIndexOfOtherRadioButtonsInGroup(): void { + const radioButtons = this.queryButtons(); + const otherFocusableRadioButtons = radioButtons.filter( + (radioButton) => radioButton.guid !== this.guid && !radioButton.disabled + ); + otherFocusableRadioButtons.forEach((radiobutton) => { + forceUpdate(radiobutton); + }); + } + private getTabIndex(): number | undefined { if (this.disabled) { return undefined; } - return this.checked || this.isDefaultSelectable() ? 0 : -1; + if (this.checked || this.isFocusable()) { + this.updateTabIndexOfOtherRadioButtonsInGroup(); + return 0; + } else { + return -1; + } } //-------------------------------------------------------------------------- From 1e2c62d268aa340fd8e6fb1b456a876f38121ab7 Mon Sep 17 00:00:00 2001 From: anveshmekala Date: Wed, 7 Jun 2023 18:21:58 -0500 Subject: [PATCH 2/7] remove calcite-radio-group from tests --- .../radio-button/radio-button.e2e.ts | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts b/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts index e66dc995b8b..c292c897a61 100644 --- a/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts +++ b/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts @@ -458,54 +458,54 @@ describe("calcite-radio-button", () => { it("focuses first focusable item on Tab", async () => { const page = await newE2EPage(); await page.setContent(html` - +
- + Trees - + Shrubs - + Flowers - - submit + submit +
`); - const group = await page.find("calcite-radio-button-group"); - await group.press("Tab"); + const container = await page.find("div"); + await container.press("Tab"); await page.waitForChanges(); expect(await getFocusedElementProp(page, "id")).toBe("shrubs"); - await group.press("Tab"); + await container.press("Tab"); expect(await getFocusedElementProp(page, "id")).toBe("submit"); }); it("focuses checked item on Tab", async () => { const page = await newE2EPage(); await page.setContent(html` - +
- + Trees - + Shrubs - + Flowers - - submit + submit +
`); - const group = await page.find("calcite-radio-button-group"); - await group.press("Tab"); + const container = await page.find("div"); + await container.press("Tab"); await page.waitForChanges(); expect(await getFocusedElementProp(page, "id")).toBe("flowers"); - await group.press("Tab"); + await container.press("Tab"); expect(await getFocusedElementProp(page, "id")).toBe("submit"); }); }); From 4912760d6a8db964674f2d891fc8a981e13c7053 Mon Sep 17 00:00:00 2001 From: anveshmekala Date: Fri, 16 Jun 2023 12:09:06 -0500 Subject: [PATCH 3/7] feedback changes and add tests --- .../radio-button/radio-button.e2e.ts | 37 +++++++++++++++++++ .../components/radio-button/radio-button.tsx | 3 +- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts b/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts index c292c897a61..191ad306449 100644 --- a/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts +++ b/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts @@ -455,6 +455,43 @@ describe("calcite-radio-button", () => { formAssociated("calcite-radio-button", { testValue: true, inputType: "radio" }); }); + it.only("focuses first focusable item on Tab when new radio-button is added ", async () => { + const page = await newE2EPage(); + await page.setContent(html` +
+ + + Trees + + + + Shrubs + + + + Flowers + + submit +
+ `); + await page.waitForTimeout(3000); + await page.evaluate(() => { + const firstRadioButton = document.querySelector('calcite-label[id="tree-label"]'); + const newRadioButton = ` + + Plants + `; + firstRadioButton.insertAdjacentHTML("beforebegin", newRadioButton); + }); + await page.waitForTimeout(3000); + const container = await page.find("div"); + await container.press("Tab"); + await page.waitForChanges(); + expect(await getFocusedElementProp(page, "id")).toBe("plants"); + await container.press("Tab"); + expect(await getFocusedElementProp(page, "id")).toBe("submit"); + }); + it("focuses first focusable item on Tab", async () => { const page = await newE2EPage(); await page.setContent(html` diff --git a/packages/calcite-components/src/components/radio-button/radio-button.tsx b/packages/calcite-components/src/components/radio-button/radio-button.tsx index 33141a6890f..efe5704a261 100644 --- a/packages/calcite-components/src/components/radio-button/radio-button.tsx +++ b/packages/calcite-components/src/components/radio-button/radio-button.tsx @@ -289,7 +289,6 @@ export class RadioButton return undefined; } if (this.checked || this.isFocusable()) { - this.updateTabIndexOfOtherRadioButtonsInGroup(); return 0; } else { return -1; @@ -443,6 +442,7 @@ export class RadioButton } connectLabel(this); connectForm(this); + this.updateTabIndexOfOtherRadioButtonsInGroup(); } componentWillLoad(): void { @@ -460,6 +460,7 @@ export class RadioButton disconnectedCallback(): void { disconnectLabel(this); disconnectForm(this); + this.updateTabIndexOfOtherRadioButtonsInGroup(); } componentDidRender(): void { From 9beb381e4840e9cc87e39ff7fe3afdf7ae442cdf Mon Sep 17 00:00:00 2001 From: anveshmekala Date: Fri, 16 Jun 2023 17:58:25 -0500 Subject: [PATCH 4/7] refactor --- .../radio-button/radio-button.e2e.ts | 46 +++++++------------ .../components/radio-button/radio-button.tsx | 5 ++ 2 files changed, 22 insertions(+), 29 deletions(-) diff --git a/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts b/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts index 4019f6a72de..a57a1e7ec91 100644 --- a/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts +++ b/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts @@ -13,7 +13,6 @@ import { import { html } from "../../../support/formatting"; import { getFocusedElementProp } from "../../tests/utils"; - describe("calcite-radio-button", () => { describe("renders", () => { renders("calcite-radio-button", { display: "block" }); @@ -487,7 +486,7 @@ describe("calcite-radio-button", () => { const page = await newE2EPage(); await page.setContent(html`
- + Trees @@ -502,49 +501,38 @@ describe("calcite-radio-button", () => { submit
`); - await page.waitForTimeout(3000); + + const container = await page.find("div"); + await container.press("Tab"); + await page.waitForChanges(); + expect(await getFocusedElementProp(page, "id")).toBe("shrubs"); + await container.press("Tab"); + expect(await getFocusedElementProp(page, "id")).toBe("submit"); await page.evaluate(() => { - const firstRadioButton = document.querySelector('calcite-label[id="tree-label"]'); + const firstRadioButton = document.querySelector('calcite-label[id="1"]'); const newRadioButton = ` Plants `; firstRadioButton.insertAdjacentHTML("beforebegin", newRadioButton); }); - await page.waitForTimeout(3000); - const container = await page.find("div"); + + await container.press("Tab"); + await page.waitForChanges(); await container.press("Tab"); await page.waitForChanges(); expect(await getFocusedElementProp(page, "id")).toBe("plants"); await container.press("Tab"); + await page.waitForChanges(); expect(await getFocusedElementProp(page, "id")).toBe("submit"); - }); - it("focuses first focusable item on Tab", async () => { - const page = await newE2EPage(); - await page.setContent(html` -
- - - Trees - - - - Shrubs - - - - Flowers - - submit -
- `); - const container = await page.find("div"); + const radioButtonElement = await page.find('calcite-radio-button[id="plants"]'); + radioButtonElement.setProperty("disabled", true); await container.press("Tab"); await page.waitForChanges(); - expect(await getFocusedElementProp(page, "id")).toBe("shrubs"); await container.press("Tab"); - expect(await getFocusedElementProp(page, "id")).toBe("submit"); + await page.waitForChanges(); + expect(await getFocusedElementProp(page, "id")).toBe("shrubs"); }); it("focuses checked item on Tab", async () => { diff --git a/packages/calcite-components/src/components/radio-button/radio-button.tsx b/packages/calcite-components/src/components/radio-button/radio-button.tsx index beb57fbeda7..cd32a9ff57a 100644 --- a/packages/calcite-components/src/components/radio-button/radio-button.tsx +++ b/packages/calcite-components/src/components/radio-button/radio-button.tsx @@ -74,6 +74,11 @@ export class RadioButton /** When `true`, interaction is prevented and the component is displayed with lower opacity. */ @Prop({ reflect: true }) disabled = false; + @Watch("disabled") + disabledChanged(): void { + this.updateTabIndexOfOtherRadioButtonsInGroup(); + } + /** * The focused state of the component. * From a943ebee2b11293c87e72750ae5e7c92b1b76cca Mon Sep 17 00:00:00 2001 From: anveshmekala Date: Tue, 20 Jun 2023 11:31:19 -0500 Subject: [PATCH 5/7] refactor e2e tests --- .../radio-button/radio-button.e2e.ts | 176 ++++++++++-------- .../components/radio-button/radio-button.tsx | 7 + 2 files changed, 103 insertions(+), 80 deletions(-) diff --git a/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts b/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts index a57a1e7ec91..0b62988f31b 100644 --- a/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts +++ b/packages/calcite-components/src/components/radio-button/radio-button.e2e.ts @@ -67,6 +67,102 @@ describe("calcite-radio-button", () => { focusable("calcite-radio-button", { shadowFocusTargetSelector: ".container" }); + + it("focuses first focusable item on Tab when new radio-button is added", async () => { + const page = await newE2EPage(); + await page.setContent(html` +
+ + + Trees + + + + Shrubs + + + + Flowers + + submit +
+ `); + + await page.keyboard.press("Tab"); + await page.waitForChanges(); + expect(await getFocusedElementProp(page, "id")).toBe("shrubs"); + await page.keyboard.press("Tab"); + await page.waitForChanges(); + expect(await getFocusedElementProp(page, "id")).toBe("submit"); + + await page.evaluate(() => { + const firstRadioButton = document.querySelector('calcite-label[id="1"]'); + const newRadioButton = ` + + Plants + `; + firstRadioButton.insertAdjacentHTML("beforebegin", newRadioButton); + }); + + await page.keyboard.press("Tab"); + await page.waitForChanges(); + await page.keyboard.press("Tab"); + await page.waitForChanges(); + expect(await getFocusedElementProp(page, "id")).toBe("plants"); + await page.keyboard.press("Tab"); + await page.waitForChanges(); + expect(await getFocusedElementProp(page, "id")).toBe("submit"); + + const radioButtonElement = await page.find('calcite-radio-button[id="plants"]'); + radioButtonElement.setProperty("disabled", true); + await page.keyboard.press("Tab"); + await page.waitForChanges(); + await page.keyboard.press("Tab"); + await page.waitForChanges(); + expect(await getFocusedElementProp(page, "id")).toBe("shrubs"); + }); + + it("focuses checked item on Tab when new radio-button is added", async () => { + const page = await newE2EPage(); + await page.setContent(html` +
+ + + Trees + + + + Shrubs + + + + Flowers + + submit +
+ `); + + await page.keyboard.press("Tab"); + await page.waitForChanges(); + expect(await getFocusedElementProp(page, "id")).toBe("flowers"); + await page.keyboard.press("Tab"); + expect(await getFocusedElementProp(page, "id")).toBe("submit"); + + await page.evaluate(() => { + const firstRadioButton = document.querySelector('calcite-label[id="1"]'); + const newRadioButton = ` + + Plants + `; + firstRadioButton.insertAdjacentHTML("beforebegin", newRadioButton); + }); + + await page.keyboard.press("Tab"); + await page.waitForChanges(); + await page.keyboard.press("Tab"); + await page.waitForChanges(); + expect(await getFocusedElementProp(page, "id")).toBe("flowers"); + }); }); describe("reflects", () => { @@ -481,84 +577,4 @@ describe("calcite-radio-button", () => { describe("is form-associated", () => { formAssociated("calcite-radio-button", { testValue: true, inputType: "radio" }); }); - - it.only("focuses first focusable item on Tab when new radio-button is added ", async () => { - const page = await newE2EPage(); - await page.setContent(html` -
- - - Trees - - - - Shrubs - - - - Flowers - - submit -
- `); - - const container = await page.find("div"); - await container.press("Tab"); - await page.waitForChanges(); - expect(await getFocusedElementProp(page, "id")).toBe("shrubs"); - await container.press("Tab"); - expect(await getFocusedElementProp(page, "id")).toBe("submit"); - await page.evaluate(() => { - const firstRadioButton = document.querySelector('calcite-label[id="1"]'); - const newRadioButton = ` - - Plants - `; - firstRadioButton.insertAdjacentHTML("beforebegin", newRadioButton); - }); - - await container.press("Tab"); - await page.waitForChanges(); - await container.press("Tab"); - await page.waitForChanges(); - expect(await getFocusedElementProp(page, "id")).toBe("plants"); - await container.press("Tab"); - await page.waitForChanges(); - expect(await getFocusedElementProp(page, "id")).toBe("submit"); - - const radioButtonElement = await page.find('calcite-radio-button[id="plants"]'); - radioButtonElement.setProperty("disabled", true); - await container.press("Tab"); - await page.waitForChanges(); - await container.press("Tab"); - await page.waitForChanges(); - expect(await getFocusedElementProp(page, "id")).toBe("shrubs"); - }); - - it("focuses checked item on Tab", async () => { - const page = await newE2EPage(); - await page.setContent(html` -
- - - Trees - - - - Shrubs - - - - Flowers - - submit -
- `); - const container = await page.find("div"); - await container.press("Tab"); - await page.waitForChanges(); - expect(await getFocusedElementProp(page, "id")).toBe("flowers"); - await container.press("Tab"); - expect(await getFocusedElementProp(page, "id")).toBe("submit"); - }); }); diff --git a/packages/calcite-components/src/components/radio-button/radio-button.tsx b/packages/calcite-components/src/components/radio-button/radio-button.tsx index cd32a9ff57a..5be4897b85b 100644 --- a/packages/calcite-components/src/components/radio-button/radio-button.tsx +++ b/packages/calcite-components/src/components/radio-button/radio-button.tsx @@ -100,6 +100,11 @@ export class RadioButton /** When `true`, the component is not displayed and is not focusable or checkable. */ @Prop({ reflect: true }) hidden = false; + @Watch("hidden") + hiddenChanged(): void { + this.updateTabIndexOfOtherRadioButtonsInGroup(); + } + /** * The hovered state of the component. * @@ -300,6 +305,7 @@ export class RadioButton } private updateTabIndexOfOtherRadioButtonsInGroup(): void { + console.log("update tab index", this.el.value); const radioButtons = this.queryButtons(); const otherFocusableRadioButtons = radioButtons.filter( (radioButton) => radioButton.guid !== this.guid && !radioButton.disabled @@ -502,6 +508,7 @@ export class RadioButton render(): VNode { const tabIndex = this.getTabIndex(); + console.log("re render", this.el.value); return (
Date: Tue, 20 Jun 2023 16:31:46 -0500 Subject: [PATCH 6/7] remove console.log statements --- .../src/components/radio-button/radio-button.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/calcite-components/src/components/radio-button/radio-button.tsx b/packages/calcite-components/src/components/radio-button/radio-button.tsx index 5be4897b85b..beeb0905134 100644 --- a/packages/calcite-components/src/components/radio-button/radio-button.tsx +++ b/packages/calcite-components/src/components/radio-button/radio-button.tsx @@ -305,7 +305,6 @@ export class RadioButton } private updateTabIndexOfOtherRadioButtonsInGroup(): void { - console.log("update tab index", this.el.value); const radioButtons = this.queryButtons(); const otherFocusableRadioButtons = radioButtons.filter( (radioButton) => radioButton.guid !== this.guid && !radioButton.disabled @@ -508,7 +507,6 @@ export class RadioButton render(): VNode { const tabIndex = this.getTabIndex(); - console.log("re render", this.el.value); return (
Date: Wed, 21 Jun 2023 10:25:47 -0500 Subject: [PATCH 7/7] fix typos and refactor if else with ternary --- .../src/components/radio-button/radio-button.tsx | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/calcite-components/src/components/radio-button/radio-button.tsx b/packages/calcite-components/src/components/radio-button/radio-button.tsx index beeb0905134..0bd2b514338 100644 --- a/packages/calcite-components/src/components/radio-button/radio-button.tsx +++ b/packages/calcite-components/src/components/radio-button/radio-button.tsx @@ -197,8 +197,8 @@ export class RadioButton isFocusable = (): boolean => { const radioButtons = this.queryButtons(); - const firstFocusable = radioButtons.find((radiobutton) => !radiobutton.disabled); - const checked = radioButtons.find((radiobutton) => radiobutton.checked); + const firstFocusable = radioButtons.find((radioButton) => !radioButton.disabled); + const checked = radioButtons.find((radioButton) => radioButton.checked); return firstFocusable === this.el && !checked; }; @@ -309,8 +309,8 @@ export class RadioButton const otherFocusableRadioButtons = radioButtons.filter( (radioButton) => radioButton.guid !== this.guid && !radioButton.disabled ); - otherFocusableRadioButtons.forEach((radiobutton) => { - forceUpdate(radiobutton); + otherFocusableRadioButtons.forEach((radioButton) => { + forceUpdate(radioButton); }); } @@ -318,11 +318,7 @@ export class RadioButton if (this.disabled) { return undefined; } - if (this.checked || this.isFocusable()) { - return 0; - } else { - return -1; - } + return this.checked || this.isFocusable() ? 0 : -1; } //--------------------------------------------------------------------------