Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(pie-radio-group): DSW-2632 add keyboard navigation and focusing #2108

Merged
merged 6 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/breezy-experts-hear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@justeattakeaway/pie-radio": minor
---

[Added] - Ensure focus is correctly delegated to the underlying input element
5 changes: 5 additions & 0 deletions .changeset/ten-chefs-rescue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@justeattakeaway/pie-radio-group": minor
---

[Added] - Keyboard navigation and tab/shift + tab focus behaviour
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { html, nothing } from 'lit';
import { type Meta } from '@storybook/web-components';

import '@justeattakeaway/pie-radio-group';
import {
defaultProps,
type RadioGroupProps as RadioGroupPropsBase,
} from '@justeattakeaway/pie-radio-group';
import '@justeattakeaway/pie-link';
import '@justeattakeaway/pie-radio';
import '@justeattakeaway/pie-form-label';
import '@justeattakeaway/pie-button';

import { createStory } from '../../utilities';

type RadioGroupProps = RadioGroupPropsBase & {
labelSlot: keyof typeof labelSlotOptions;
};

type RadioGroupStoryMeta = Meta<RadioGroupProps>;

const defaultArgs: RadioGroupProps = {
...defaultProps,
labelSlot: 'None',
};

const labelSlotOptions = {
None: nothing,
Label: html`<pie-form-label slot="label">Radio Group Label</pie-form-label>`,
};

const radioGroupStoryMeta: RadioGroupStoryMeta = {
title: 'Radio Group',
component: 'pie-radio-group',
parameters: {
controls: {
disable: true,
},
design: {
type: 'figma',
url: 'https://www.figma.com/design/pPSC73rPin4csb8DiK1CRr/branch/6u3sopt3trAp9wdJi7lUfY/%E2%9C%A8-%5BCore%5D-Web-Components-%5BPIE-3%5D?node-id=6369-3799&node-type=frame&t=pbk7ibGYRutGCO3z-0',
},
},
};

export default radioGroupStoryMeta;

const KeyboardNavigationTemplate = () => html`
<h2>Radio group 1</h2>
<p><pie-button size="small-productive" data-test-id="btn-1">Button 1</pie-button></p>
<pie-radio-group data-test-id="radio-group-1">
<pie-radio data-test-id="radio-1" value="chinese">Chinese</pie-radio>
<pie-radio data-test-id="radio-2" value="shawarma">Shawarma</pie-radio>
<pie-radio data-test-id="radio-3" value="pizza">Pizza</pie-radio>
<pie-radio data-test-id="radio-4" value="fish-and-chips">Fish and Chips</pie-radio>
<pie-radio data-test-id="radio-5" value="indian">Indian</pie-radio>
</pie-radio-group>
<p><pie-button size="small-productive" variant="secondary" data-test-id="btn-2">Button 2</pie-button></p>

<h2>Radio group 2</h2>
<p><pie-button size="small-productive" data-test-id="btn-3">Button 3</pie-button></p>
<pie-radio-group data-test-id="radio-group-2" value="fish-and-chips">
<pie-radio data-test-id="radio-6" value="chinese">Chinese</pie-radio>
<pie-radio data-test-id="radio-7" value="shawarma">Shawarma</pie-radio>
<pie-radio data-test-id="radio-8" value="pizza">Pizza</pie-radio>
<pie-radio data-test-id="radio-9" value="fish-and-chips">Fish and Chips</pie-radio>
<pie-radio data-test-id="radio-10" value="indian">Indian</pie-radio>
</pie-radio-group>
<p><pie-button size="small-productive" variant="secondary" data-test-id="btn-4">Button 4</pie-button></p>
`;

export const KeyboardNavigation = createStory<RadioGroupProps>(KeyboardNavigationTemplate, defaultArgs)();
4 changes: 2 additions & 2 deletions apps/pie-storybook/turbo.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"cache": false,
"dependsOn": [
"^build",
"generate:component-statuses"
"^generate:component-statuses"
]
},
"build": {
Expand All @@ -39,4 +39,4 @@
"dependsOn": []
}
}
}
}
4 changes: 2 additions & 2 deletions bundlewatch.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@
},
{
"path": "./packages/components/pie-radio-group/dist/index.js",
"maxSize": "2.5 KB"
"maxSize": "5 KB"
},
{
"path": "./packages/components/pie-radio-group/dist/react.js",
Expand Down Expand Up @@ -199,4 +199,4 @@
"main"
]
}
}
}
158 changes: 157 additions & 1 deletion packages/components/pie-radio-group/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import {
type PropertyValues,
type TemplateResult,
} from 'lit';
import { property, queryAssignedElements, state } from 'lit/decorators.js';
import {
property, query, queryAssignedElements, state,
} from 'lit/decorators.js';
import {
RtlMixin,
defineCustomElement,
Expand Down Expand Up @@ -63,8 +65,19 @@ export class PieRadioGroup extends FormControlMixin(RtlMixin(LitElement)) implem
@queryAssignedElements({ selector: 'pie-radio' })
_slottedChildren!: Array<HTMLInputElement>;

@query('fieldset')
private _fieldset!: HTMLInputElement;

private _abortController!: AbortController;

/**
* Tracks whether the `Shift` key was held during the last `Tab` key press.
*
* The property is static because it needs to be shared across all instances of the
* `PieRadioGroup` component on the same page, ensuring consistent behavior.
*/
private static _wasShiftTabPressed = false;

/**
* Dispatches a custom event to notify each slotted child radio element
* when the radio group is disabled.
Expand Down Expand Up @@ -147,12 +160,154 @@ export class PieRadioGroup extends FormControlMixin(RtlMixin(LitElement)) implem
}
}

protected firstUpdated (): void {
// Make all radios impossible to tab to
fernandofranca marked this conversation as resolved.
Show resolved Hide resolved
// This is because by default, we are able to tab to each individual radio button.
// This is not the behaviour we want, so applying -1 tabindex prevents it.
this._slottedChildren.forEach((radio) => radio.setAttribute('tabindex', '-1'));
}

connectedCallback (): void {
super.connectedCallback();
this._abortController = new AbortController();
const { signal } = this._abortController;

this.shadowRoot?.addEventListener('change', this._handleRadioChange.bind(this), { signal });

this.addEventListener('focusin', this._handleFocusIn, { signal });
this.addEventListener('focusout', this._handleFocusOut, { signal });

this.addEventListener('keydown', this._handleKeyDown, { signal });
document.addEventListener('keydown', this._updateShiftTabState.bind(this), { signal });
}

/**
* Updates the state of `_wasShiftTabPressed` based on the last `Tab` key press.
*/
private _updateShiftTabState (event: KeyboardEvent): void {
if (event.key === 'Tab') {
PieRadioGroup._wasShiftTabPressed = event.shiftKey;
}
}

/**
* Handles the `focusin` event to manage focus within the radio group.
*
* This method determines the appropriate element to focus when the radio group
* gains focus. It considers the last navigation action (whether `Shift+Tab` was used)
* and focuses the checked option, the first option, or the last option as needed.
*/
private _handleFocusIn (event: FocusEvent): void {
if (this !== event.target) return;

const isShiftTab = PieRadioGroup._wasShiftTabPressed;
const focusTarget = this._slottedChildren?.find((child) => child.checked) ||
(isShiftTab ? this._slottedChildren.at(-1) : this._slottedChildren[0]);

if (!focusTarget) return;

focusTarget.focus();
this._toggleFieldsetTabindex(false);
}

/**
* Handles the `focusout` event to restore the `tabindex` on the radio group's `fieldset`.
*
* When focus leaves the radio group, this method enables the `tabindex` attribute
* on the `fieldset` element. This ensures the radio group remains accessible for
* keyboard navigation and can be re-focused when tabbing back into the group.
*/
private _handleFocusOut (): void {
this._toggleFieldsetTabindex(true);
}

private _toggleFieldsetTabindex (enable: boolean): void {
if (enable) {
this._fieldset.setAttribute('tabindex', '0');
} else {
this._fieldset.removeAttribute('tabindex');
}
}

private _moveFocus (currentIndex: number, step: number): void {
const newIndex = (currentIndex + step + this._slottedChildren.length) % this._slottedChildren.length;
this._focusAndClickOption(this._slottedChildren[newIndex]);
}

/**
* Determines if a key press indicates forward navigation within the radio group.
*
* This method evaluates a keyboard event to check if the pressed key corresponds
* to forward navigation based on the current text direction (LTR or RTL).
*
* **Behaviour:**
* - For LTR (Left-to-Right) layouts:
* - `ArrowRight` and `ArrowDown` indicate forward navigation.
* - For RTL (Right-to-Left) layouts:
* - `ArrowLeft` and `ArrowDown` indicate forward navigation.
*/
private _isForwardKey (event: KeyboardEvent): boolean {
return (event.code === 'ArrowRight' && !this.isRTL) ||
(event.code === 'ArrowLeft' && this.isRTL) ||
event.code === 'ArrowDown';
}

/**
* Determines if a key press indicates backward navigation within the radio group.
*
* This method evaluates a keyboard event to check if the pressed key corresponds
* to backward navigation based on the current text direction (LTR or RTL).
*
* **Behaviour:**
* - For LTR (Left-to-Right) layouts:
* - `ArrowLeft` and `ArrowUp` indicate backward navigation.
* - For RTL (Right-to-Left) layouts:
* - `ArrowRight` and `ArrowUp` indicate backward navigation.
*/
private _isBackwardKey (event: KeyboardEvent): boolean {
return (event.code === 'ArrowLeft' && !this.isRTL) ||
(event.code === 'ArrowRight' && this.isRTL) ||
event.code === 'ArrowUp';
}

/**
* Handles keyboard navigation within the radio group using arrow keys.
*
* This method responds to `keydown` events and determines the appropriate navigation
* action (forward or backward) based on the pressed key and the current focus. It prevents
* the default browser behaviour (e.g., scrolling) when arrow keys are used for navigation.
*/
private _handleKeyDown (event: KeyboardEvent): void {
const currentlyFocusedChild = this._slottedChildren.find((child) => child === document.activeElement);

if (!currentlyFocusedChild) {
return;
}

const currentIndex = this._slottedChildren.indexOf(currentlyFocusedChild);
if (currentIndex === -1) {
return;
}

// Prevent default scrolling behavior when using Arrow keys for Radio Group navigation
if (['ArrowRight', 'ArrowDown', 'ArrowLeft', 'ArrowUp'].includes(event.code)) {
event.preventDefault();
}

if (this._isForwardKey(event)) {
this._moveFocus(currentIndex, 1);
} else if (this._isBackwardKey(event)) {
this._moveFocus(currentIndex, -1);
}
}

private _focusAndClickOption (option: HTMLInputElement): void {
option.focus();
// This is quite hacky, but it ensures the radio elements correct emit a real change event.
// Simply setting option.checked as true would require re-architecture of both this component and the radio button
// to ensure that property changes are observed and correctly propagated up.
option.shadowRoot?.querySelector('input')?.click();
this._toggleFieldsetTabindex(false);
}

disconnectedCallback (): void {
Expand All @@ -179,6 +334,7 @@ export class PieRadioGroup extends FormControlMixin(RtlMixin(LitElement)) implem

return html`
<fieldset
tabindex="0"
name=${ifDefined(name)}
?disabled=${disabled}
data-test-id="pie-radio-group"
Expand Down
Loading
Loading