From 4267b8ca26db8047d42659d6062b606a90819abc Mon Sep 17 00:00:00 2001 From: Anveshreddy mekala Date: Wed, 7 Jun 2023 12:09:51 -0500 Subject: [PATCH] fix(radio-button-group): no longer focus first radio button on label click and adds `setFocus` method. (#7050) **Related Issue:** #6698 ## Summary This PR will remove the focus flash from the first `calcite-radio-button` in group on label click Additional enhancements: - Adds `setFocus( )` method in `calcite-radio-button-group` - Users will be able to select label text in chrome. #6357 Note: With this PR, `delegateFocus` is removed from `calcite-radio-button-group` which is causing #6698. Because the tabIndex is set to 0 for the first focusable element on page load to enable keyboard navigation into the group it makes `delegateFocus` to apply focus styles on the first focusable element when the user clicks on the label (invokes .focus( )) resulting in the behavior mentioned in the issue 6698. To resolve this, `delegateFocus` is removed and added `setFocus( )` method. --- .../radio-button-group.e2e.ts | 67 ++++++++++++++++++- .../radio-button-group/radio-button-group.tsx | 56 +++++++++++++--- 2 files changed, 114 insertions(+), 9 deletions(-) diff --git a/packages/calcite-components/src/components/radio-button-group/radio-button-group.e2e.ts b/packages/calcite-components/src/components/radio-button-group/radio-button-group.e2e.ts index d911653cac1..87628774f91 100644 --- a/packages/calcite-components/src/components/radio-button-group/radio-button-group.e2e.ts +++ b/packages/calcite-components/src/components/radio-button-group/radio-button-group.e2e.ts @@ -1,6 +1,7 @@ import { newE2EPage } from "@stencil/core/testing"; -import { accessible, defaults, hidden, reflects, renders } from "../../tests/commonTests"; +import { accessible, defaults, focusable, hidden, reflects, renders } from "../../tests/commonTests"; import { html } from "../../../support/formatting"; +import { getFocusedElementProp } from "../../tests/utils"; describe("calcite-radio-button-group", () => { describe("renders", () => { @@ -20,6 +21,22 @@ describe("calcite-radio-button-group", () => { ]); }); + describe("is focusable", () => { + focusable( + html` + + + Flowers + + + + Trees + + `, + { focusTargetSelector: "calcite-radio-button" } + ); + }); + describe("honors hidden attribute", () => { hidden("calcite-radio-button"); @@ -452,4 +469,52 @@ describe("calcite-radio-button-group", () => { expect(changeEvent).toHaveReceivedEventTimes(3); expect(await getSelectedItemValue()).toBe("three"); }); + + it("should focus the checked radio-button on setFocus()", async () => { + const page = await newE2EPage(); + await page.setContent(html` + + + + Trees + + + + Shrubs + + + + Flowers + + + `); + const group = await page.find("calcite-radio-button-group"); + await group.callMethod("setFocus"); + await page.waitForChanges(); + expect(await getFocusedElementProp(page, "id")).toBe("flowers"); + }); + + it("should focus the first focusable radio-button on setFocus()", async () => { + const page = await newE2EPage(); + await page.setContent(html` + + + + Trees + + + + Shrubs + + + + Flowers + + + `); + const group = await page.find("calcite-radio-button-group"); + await group.callMethod("setFocus"); + await page.waitForChanges(); + expect(await getFocusedElementProp(page, "id")).toBe("shrubs"); + }); }); diff --git a/packages/calcite-components/src/components/radio-button-group/radio-button-group.tsx b/packages/calcite-components/src/components/radio-button-group/radio-button-group.tsx index 0bac67fa7bd..9c5b6bc6f35 100644 --- a/packages/calcite-components/src/components/radio-button-group/radio-button-group.tsx +++ b/packages/calcite-components/src/components/radio-button-group/radio-button-group.tsx @@ -6,12 +6,20 @@ import { h, Host, Listen, + Method, Prop, + State, VNode, Watch } from "@stencil/core"; import { createObserver } from "../../utils/observers"; import { Layout, Scale } from "../interfaces"; +import { + componentLoaded, + LoadableComponent, + setComponentLoaded, + setUpLoadableComponent +} from "../../utils/loadable"; /** * @slot - A slot for adding `calcite-radio-button`s. @@ -19,11 +27,9 @@ import { Layout, Scale } from "../interfaces"; @Component({ tag: "calcite-radio-button-group", styleUrl: "radio-button-group.scss", - shadow: { - delegatesFocus: true - } + shadow: true }) -export class RadioButtonGroup { +export class RadioButtonGroup implements LoadableComponent { //-------------------------------------------------------------------------- // // Element @@ -91,6 +97,8 @@ export class RadioButtonGroup { mutationObserver = createObserver("mutation", () => this.passPropsToRadioButtons()); + @State() radioButtons: HTMLCalciteRadioButtonElement[] = []; + //-------------------------------------------------------------------------- // // Lifecycle @@ -102,6 +110,14 @@ export class RadioButtonGroup { this.mutationObserver?.observe(this.el, { childList: true, subtree: true }); } + componentWillLoad(): void { + setUpLoadableComponent(this); + } + + componentDidLoad(): void { + setComponentLoaded(this); + } + disconnectedCallback(): void { this.mutationObserver?.disconnect(); } @@ -113,10 +129,11 @@ export class RadioButtonGroup { //-------------------------------------------------------------------------- private passPropsToRadioButtons = (): void => { - const radioButtons = this.el.querySelectorAll("calcite-radio-button"); - this.selectedItem = Array.from(radioButtons).find((radioButton) => radioButton.checked) || null; - if (radioButtons.length > 0) { - radioButtons.forEach((radioButton) => { + this.radioButtons = Array.from(this.el.querySelectorAll("calcite-radio-button")); + this.selectedItem = + Array.from(this.radioButtons).find((radioButton) => radioButton.checked) || null; + if (this.radioButtons.length > 0) { + this.radioButtons.forEach((radioButton) => { radioButton.disabled = this.disabled || radioButton.disabled; radioButton.hidden = this.hidden; radioButton.name = this.name; @@ -126,6 +143,10 @@ export class RadioButtonGroup { } }; + private getFocusableRadioButton(): HTMLCalciteRadioButtonElement | null { + return this.radioButtons.find((radiobutton) => !radiobutton.disabled) ?? null; + } + //-------------------------------------------------------------------------- // // Events @@ -137,6 +158,25 @@ export class RadioButtonGroup { */ @Event({ cancelable: false }) calciteRadioButtonGroupChange: EventEmitter; + //-------------------------------------------------------------------------- + // + // Public Method + // + //-------------------------------------------------------------------------- + + /** Sets focus on the fist focusable `calcite-radio-button` element in the component. */ + @Method() + async setFocus(): Promise { + await componentLoaded(this); + if (this.selectedItem && !this.selectedItem.disabled) { + this.selectedItem.setFocus(); + return; + } + if (this.radioButtons.length > 0) { + this.getFocusableRadioButton()?.setFocus(); + } + } + //-------------------------------------------------------------------------- // // Event Listeners