Skip to content

Commit

Permalink
fix(radio-button-group): no longer focus first radio button on label …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
anveshmekala authored Jun 7, 2023
1 parent eec0785 commit 4267b8c
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -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", () => {
Expand All @@ -20,6 +21,22 @@ describe("calcite-radio-button-group", () => {
]);
});

describe("is focusable", () => {
focusable(
html`<calcite-radio-button-group name="Options" layout="vertical">
<calcite-label layout="inline">
<calcite-radio-button value="flowers" disabled></calcite-radio-button>
Flowers
</calcite-label>
<calcite-label layout="inline">
<calcite-radio-button value="trees"></calcite-radio-button>
Trees
</calcite-label>
</calcite-radio-button-group>`,
{ focusTargetSelector: "calcite-radio-button" }
);
});

describe("honors hidden attribute", () => {
hidden("calcite-radio-button");

Expand Down Expand Up @@ -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`
<calcite-radio-button-group name="Options" layout="vertical">
<calcite-label layout="inline">
<calcite-radio-button value="trees" disabled id="trees"></calcite-radio-button>
Trees
</calcite-label>
<calcite-label layout="inline">
<calcite-radio-button value="shrubs" id="shrubs"></calcite-radio-button>
Shrubs
</calcite-label>
<calcite-label layout="inline">
<calcite-radio-button value="flowers" id="flowers" checked></calcite-radio-button>
Flowers
</calcite-label>
</calcite-radio-button-group>
`);
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`
<calcite-radio-button-group name="Options" layout="vertical">
<calcite-label layout="inline">
<calcite-radio-button value="trees" disabled id="trees"></calcite-radio-button>
Trees
</calcite-label>
<calcite-label layout="inline">
<calcite-radio-button value="shrubs" id="shrubs"></calcite-radio-button>
Shrubs
</calcite-label>
<calcite-label layout="inline">
<calcite-radio-button value="flowers" id="flowers"></calcite-radio-button>
Flowers
</calcite-label>
</calcite-radio-button-group>
`);
const group = await page.find("calcite-radio-button-group");
await group.callMethod("setFocus");
await page.waitForChanges();
expect(await getFocusedElementProp(page, "id")).toBe("shrubs");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,30 @@ 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.
*/
@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
Expand Down Expand Up @@ -91,6 +97,8 @@ export class RadioButtonGroup {

mutationObserver = createObserver("mutation", () => this.passPropsToRadioButtons());

@State() radioButtons: HTMLCalciteRadioButtonElement[] = [];

//--------------------------------------------------------------------------
//
// Lifecycle
Expand All @@ -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();
}
Expand All @@ -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;
Expand All @@ -126,6 +143,10 @@ export class RadioButtonGroup {
}
};

private getFocusableRadioButton(): HTMLCalciteRadioButtonElement | null {
return this.radioButtons.find((radiobutton) => !radiobutton.disabled) ?? null;
}

//--------------------------------------------------------------------------
//
// Events
Expand All @@ -137,6 +158,25 @@ export class RadioButtonGroup {
*/
@Event({ cancelable: false }) calciteRadioButtonGroupChange: EventEmitter<void>;

//--------------------------------------------------------------------------
//
// Public Method
//
//--------------------------------------------------------------------------

/** Sets focus on the fist focusable `calcite-radio-button` element in the component. */
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
if (this.selectedItem && !this.selectedItem.disabled) {
this.selectedItem.setFocus();
return;
}
if (this.radioButtons.length > 0) {
this.getFocusableRadioButton()?.setFocus();
}
}

//--------------------------------------------------------------------------
//
// Event Listeners
Expand Down

0 comments on commit 4267b8c

Please sign in to comment.