Skip to content

fix: radiogroup navigation #8488

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Jul 9, 2025
Merged

fix: radiogroup navigation #8488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b4d1222
fix: radio tab navigation in a focusscope
snowystinger Jul 4, 2025
a7c5524
fix navigation inside RadioGroup
snowystinger Jul 5, 2025
f7cee2b
fix lint
snowystinger Jul 5, 2025
a698e21
Merge branch 'main' into fix-radio-tab-navigation-in-focusscope
snowystinger Jul 7, 2025
7c3ec9c
build radio check into focusable tree walker so everyone benefits
snowystinger Jul 7, 2025
1ca3cf6
Merge branch 'fix-radio-tab-navigation-in-focusscope' of github.com:a…
snowystinger Jul 7, 2025
622279e
Merge branch 'main' into fix-radio-tab-navigation-in-focusscope
snowystinger Jul 7, 2025
f8155bf
make logic work outside a form as well
snowystinger Jul 8, 2025
8b7ae3f
Update packages/@react-aria/focus/src/FocusScope.tsx
snowystinger Jul 8, 2025
4628133
Merge branch 'main' into fix-radio-tab-navigation-in-focusscope
snowystinger Jul 8, 2025
a1dcabf
add to dialog story for testing
snowystinger Jul 8, 2025
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
34 changes: 34 additions & 0 deletions packages/@react-aria/focus/src/FocusScope.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,25 @@ function shouldContainFocus(scopeRef: ScopeRef) {
return true;
}

function isTabbableRadio(element: HTMLInputElement) {
if (element.checked) {
return true;
}
let radios: HTMLInputElement[] = [];
if (!element.form) {
radios = ([...getOwnerDocument(element).querySelectorAll(`input[type="radio"][name="${CSS.escape(element.name)}"]`)] as HTMLInputElement[]).filter(radio => !radio.form);
} else {
let radioList = element.form?.elements?.namedItem(element.name) as RadioNodeList;
radios = [...(radioList ?? [])] as HTMLInputElement[];
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah interesting, I hadn't considered that two radio groups in different forms with the same name are separate, but when outside a form the name must be unique. HTML is fun.

if (!radios) {
return false;
}
let anyChecked = radios.some(radio => radio.checked);

return !anyChecked;
}

function useFocusContainment(scopeRef: RefObject<Element[] | null>, contain?: boolean) {
let focusedNode = useRef<FocusableElement>(undefined);

Expand Down Expand Up @@ -757,6 +776,21 @@ export function getFocusableTreeWalker(root: Element, opts?: FocusManagerOptions
return NodeFilter.FILTER_REJECT;
}

if (opts?.tabbable
&& (node as Element).tagName === 'INPUT'
&& (node as HTMLInputElement).getAttribute('type') === 'radio') {
// If the radio is in a form, we can get all the other radios by name
if (!isTabbableRadio(node as HTMLInputElement)) {
return NodeFilter.FILTER_REJECT;
}
// If the radio is in the same group as the current node and none are selected, we can skip it
if ((walker.currentNode as Element).tagName === 'INPUT'
&& (walker.currentNode as HTMLInputElement).type === 'radio'
&& (walker.currentNode as HTMLInputElement).name === (node as HTMLInputElement).name) {
return NodeFilter.FILTER_REJECT;
}
}

if (filter(node as Element)
&& isElementVisible(node as Element)
&& (!scope || isElementInScope(node as Element, scope))
Expand Down
258 changes: 258 additions & 0 deletions packages/@react-aria/focus/test/FocusScope.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1280,6 +1280,264 @@ describe('FocusScope', function () {
});
});

it('skips radio buttons that are in the same group and are not the selectable one forwards', async function () {
function Test() {
return (
<FocusScope contain>
<button data-testid="button1">button</button>
<form>
<fieldset>
<legend>Select a maintenance drone:</legend>

<div>
<input type="radio" id="huey" name="drone" value="huey" defaultChecked />
<label htmlFor="huey">Huey</label>
</div>

<div>
<input type="radio" id="dewey" name="drone" value="dewey" />
<label htmlFor="dewey">Dewey</label>
</div>
<button data-testid="button2">button</button>
<div>
<input type="radio" id="louie" name="drone" value="louie" />
<label htmlFor="louie">Louie</label>
</div>
</fieldset>
<fieldset>
<legend>Select a ship:</legend>

<div>
<input type="radio" id="larry" name="ship" value="larry" />
<label htmlFor="larry">Larry</label>
</div>

<div>
<input type="radio" id="moe" name="ship" value="moe" />
<label htmlFor="moe">Moe</label>
</div>
<button data-testid="button3">button</button>
<div>
<input type="radio" id="curly" name="ship" value="curly" />
<label htmlFor="curly">Curly</label>
</div>
</fieldset>
</form>
<button data-testid="button4">button</button>
</FocusScope>
);
}

let {getByTestId, getAllByRole} = render(<Test />);
let radios = getAllByRole('radio');
await user.tab();
expect(document.activeElement).toBe(getByTestId('button1'));
await user.tab();
expect(document.activeElement).toBe(radios[0]);
await user.tab();
expect(document.activeElement).toBe(getByTestId('button2'));
await user.tab();
expect(document.activeElement).toBe(radios[3]);
await user.tab();
expect(document.activeElement).toBe(getByTestId('button3'));
await user.tab();
expect(document.activeElement).toBe(radios[5]);
await user.tab();
expect(document.activeElement).toBe(getByTestId('button4'));
});

it('skips radio buttons that are in the same group and are not the selectable one forwards outside of a form', async function () {
function Test() {
return (
<FocusScope contain>
<button data-testid="button1">button</button>
<fieldset>
<legend>Select a maintenance drone:</legend>

<div>
<input type="radio" id="huey" name="drone" value="huey" defaultChecked />
<label htmlFor="huey">Huey</label>
</div>

<div>
<input type="radio" id="dewey" name="drone" value="dewey" />
<label htmlFor="dewey">Dewey</label>
</div>
<button data-testid="button2">button</button>
<div>
<input type="radio" id="louie" name="drone" value="louie" />
<label htmlFor="louie">Louie</label>
</div>
</fieldset>
<fieldset>
<legend>Select a ship:</legend>

<div>
<input type="radio" id="larry" name="ship" value="larry" />
<label htmlFor="larry">Larry</label>
</div>

<div>
<input type="radio" id="moe" name="ship" value="moe" />
<label htmlFor="moe">Moe</label>
</div>
<button data-testid="button3">button</button>
<div>
<input type="radio" id="curly" name="ship" value="curly" />
<label htmlFor="curly">Curly</label>
</div>
</fieldset>
<button data-testid="button4">button</button>
</FocusScope>
);
}

let {getByTestId, getAllByRole} = render(<Test />);
let radios = getAllByRole('radio');
await user.tab();
expect(document.activeElement).toBe(getByTestId('button1'));
await user.tab();
expect(document.activeElement).toBe(radios[0]);
await user.tab();
expect(document.activeElement).toBe(getByTestId('button2'));
await user.tab();
expect(document.activeElement).toBe(radios[3]);
await user.tab();
expect(document.activeElement).toBe(getByTestId('button3'));
await user.tab();
expect(document.activeElement).toBe(radios[5]);
await user.tab();
expect(document.activeElement).toBe(getByTestId('button4'));
});

it('skips radio buttons that are in the same group and are not the selectable one backwards', async function () {
function Test() {
return (
<FocusScope contain>
<button data-testid="button1">button</button>
<form>
<fieldset>
<legend>Select a maintenance drone:</legend>

<div>
<input type="radio" id="huey" name="drone" value="huey" defaultChecked />
<label htmlFor="huey">Huey</label>
</div>

<div>
<input type="radio" id="dewey" name="drone" value="dewey" />
<label htmlFor="dewey">Dewey</label>
</div>
<button data-testid="button2">button</button>
<div>
<input type="radio" id="louie" name="drone" value="louie" />
<label htmlFor="louie">Louie</label>
</div>
</fieldset>
<fieldset>
<legend>Select a ship:</legend>

<div>
<input type="radio" id="larry" name="ship" value="larry" />
<label htmlFor="larry">Larry</label>
</div>

<div>
<input type="radio" id="moe" name="ship" value="moe" />
<label htmlFor="moe">Moe</label>
</div>
<button data-testid="button3">button</button>
<div>
<input type="radio" id="curly" name="ship" value="curly" />
<label htmlFor="curly">Curly</label>
</div>
</fieldset>
</form>
<button data-testid="button4">button</button>
</FocusScope>
);
}

let {getByTestId, getAllByRole} = render(<Test />);
let radios = getAllByRole('radio');
await user.click(getByTestId('button4'));
await user.tab({shift: true});
expect(document.activeElement).toBe(radios[5]);
await user.tab({shift: true});
expect(document.activeElement).toBe(getByTestId('button3'));
await user.tab({shift: true});
expect(document.activeElement).toBe(radios[4]);
await user.tab({shift: true});
expect(document.activeElement).toBe(getByTestId('button2'));
await user.tab({shift: true});
expect(document.activeElement).toBe(radios[0]);
await user.tab({shift: true});
expect(document.activeElement).toBe(getByTestId('button1'));
});

it('skips radio buttons that are in the same group and are not the selectable one backwards outside of a form', async function () {
function Test() {
return (
<FocusScope contain>
<button data-testid="button1">button</button>
<fieldset>
<legend>Select a maintenance drone:</legend>

<div>
<input type="radio" id="huey" name="drone" value="huey" defaultChecked />
<label htmlFor="huey">Huey</label>
</div>

<div>
<input type="radio" id="dewey" name="drone" value="dewey" />
<label htmlFor="dewey">Dewey</label>
</div>
<button data-testid="button2">button</button>
<div>
<input type="radio" id="louie" name="drone" value="louie" />
<label htmlFor="louie">Louie</label>
</div>
</fieldset>
<fieldset>
<legend>Select a ship:</legend>

<div>
<input type="radio" id="larry" name="ship" value="larry" />
<label htmlFor="larry">Larry</label>
</div>

<div>
<input type="radio" id="moe" name="ship" value="moe" />
<label htmlFor="moe">Moe</label>
</div>
<button data-testid="button3">button</button>
<div>
<input type="radio" id="curly" name="ship" value="curly" />
<label htmlFor="curly">Curly</label>
</div>
</fieldset>
<button data-testid="button4">button</button>
</FocusScope>
);
}

let {getByTestId, getAllByRole} = render(<Test />);
let radios = getAllByRole('radio');
await user.click(getByTestId('button4'));
await user.tab({shift: true});
expect(document.activeElement).toBe(radios[5]);
await user.tab({shift: true});
expect(document.activeElement).toBe(getByTestId('button3'));
await user.tab({shift: true});
expect(document.activeElement).toBe(radios[4]);
await user.tab({shift: true});
expect(document.activeElement).toBe(getByTestId('button2'));
await user.tab({shift: true});
expect(document.activeElement).toBe(radios[0]);
await user.tab({shift: true});
expect(document.activeElement).toBe(getByTestId('button1'));
});

describe('nested focus scopes', function () {
it('should make child FocusScopes the active scope regardless of DOM structure', function () {
function ChildComponent(props) {
Expand Down
7 changes: 5 additions & 2 deletions packages/@react-aria/radio/src/useRadioGroup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

import {AriaRadioGroupProps} from '@react-types/radio';
import {DOMAttributes, ValidationResult} from '@react-types/shared';
import {filterDOMProps, mergeProps, useId} from '@react-aria/utils';
import {filterDOMProps, getOwnerWindow, mergeProps, useId} from '@react-aria/utils';
import {getFocusableTreeWalker} from '@react-aria/focus';
import {radioGroupData} from './utils';
import {RadioGroupState} from '@react-stately/radio';
Expand Down Expand Up @@ -102,7 +102,10 @@ export function useRadioGroup(props: AriaRadioGroupProps, state: RadioGroupState
return;
}
e.preventDefault();
let walker = getFocusableTreeWalker(e.currentTarget, {from: e.target});
let walker = getFocusableTreeWalker(e.currentTarget, {
from: e.target,
accept: (node) => node instanceof getOwnerWindow(node).HTMLInputElement && node.type === 'radio'
});
let nextElem;
if (nextDir === 'next') {
nextElem = walker.nextNode();
Expand Down
41 changes: 37 additions & 4 deletions packages/react-aria-components/stories/RadioGroup.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,16 +79,49 @@ export const RadioGroupInDialogExample = () => {
position: 'relative'
}}>
{({close}) => (
<>
<div>
<RadioGroupExample />
<div style={{display: 'flex', flexDirection: 'column', gap: 10}}>
<div style={{display: 'flex', flexDirection: 'row', gap: 20}}>
<div>
<RadioGroupExample />
</div>
<Form>
<RadioGroup
className={styles.radiogroup}
data-testid="radio-group-example-2"
isRequired>
<Label>Second Favorite pet</Label>
<Radio className={styles.radio} value="dogs" data-testid="radio-dog">Dog</Radio>
<Button>About dogs</Button>
<Radio className={styles.radio} value="cats">Cat</Radio>
<Button>About cats</Button>
<Radio className={styles.radio} value="dragon">Dragon</Radio>
<Button>About dragons</Button>
<FieldError className={styles.errorMessage} />
</RadioGroup>
</Form>
<Form>
<RadioGroup
className={styles.radiogroup}
data-testid="radio-group-example-3"
defaultValue="dragon"
isRequired>
<Label>Third Favorite pet</Label>
<Radio className={styles.radio} value="dogs" data-testid="radio-dog">Dog</Radio>
<Button>About dogs</Button>
<Radio className={styles.radio} value="cats">Cat</Radio>
<Button>About cats</Button>
<Radio className={styles.radio} value="dragon">Dragon</Radio>
<Button>About dragons</Button>
<FieldError className={styles.errorMessage} />
</RadioGroup>
</Form>
</div>
<div>
<Button onPress={close} style={{marginTop: 10}}>
Close
</Button>
</div>
</>
</div>
)}
</Dialog>
</Modal>
Expand Down
Loading