Skip to content

fix(radio): allow native form onChange to fire for Radio selection #10441

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

Open
starboyvarun wants to merge 1 commit into
base: main
Choose a base branch
from
+55 −2
Open

fix(radio): allow native form onChange to fire for Radio selection #10441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
40 changes: 40 additions & 0 deletions packages/react-aria-components/test/RadioGroup.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,46 @@ describe.each(['RadioGroup', 'RadioField'])('%s', comp => {
expect(onChange).toHaveBeenLastCalledWith('c');
});

it('should fire a native form onChange event when a radio is clicked', async () => {
let formOnChange = jest.fn();
let onChange = jest.fn();
let {getAllByRole} = render(
<form onChange={formOnChange}>
<TestRadioGroup groupProps={{onChange}} />
</form>
);
let radios = getAllByRole('radio');

await user.click(radios[1]);
expect(radios[1]).toBeChecked();
expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveBeenLastCalledWith('b');
expect(formOnChange).toHaveBeenCalledTimes(1);
});

it('should fire a native form onChange event when selection changes via the keyboard', async () => {
let formOnChange = jest.fn();
let onChange = jest.fn();
let {getAllByRole} = render(
<form onChange={formOnChange}>
<TestRadioGroup groupProps={{onChange}} />
</form>
);
let radios = getAllByRole('radio');

await user.tab();
expect(radios[0]).toHaveFocus();

// user-event implements its own radio-group arrow navigation, which bypasses our own code
fireEvent.keyDown(document.activeElement, {key: 'ArrowDown'});
fireEvent.keyUp(document.activeElement, {key: 'ArrowDown'});

expect(radios[1]).toBeChecked();
expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveBeenLastCalledWith('b');
expect(formOnChange).toHaveBeenCalledTimes(1);
});

it('should support read only state', () => {
let className = ({isReadOnly}) => (isReadOnly ? 'readonly' : '');
let {getByRole, getAllByRole} = renderGroup({isReadOnly: true, className}, {className});
Expand Down
3 changes: 1 addition & 2 deletions packages/react-aria/src/radio/useRadio.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@ export function useRadio(

let checked = state.selectedValue === value;

let onChange = e => {
e.stopPropagation();
let onChange = () => {
state.setSelectedValue(value);
};

Expand Down
14 changes: 14 additions & 0 deletions packages/react-aria/src/radio/useRadioGroup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,20 @@ export function useRadioGroup(props: AriaRadioGroupProps, state: RadioGroupState
// Call focus on nextElem so that keyboard navigation scrolls the radio into view
nextElem.focus();
state.setSelectedValue(nextElem.value);

// The state update above only changes the DOM `checked` property via React's controlled
// re-render — it does not dispatch any native event the way a real click does. React's own
// change detection for checkbox/radio inputs is keyed off the "click" event (a legacy
// browser-compat shim), not "change" or "input", so we replay a native click to make an
// ancestor <form onChange> (or any other native DOM listener) observe keyboard-driven
// selection the same way it observes a mouse click.
let checkedSetter = Object.getOwnPropertyDescriptor(

@snowystinger snowystinger Aug 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This pattern appears to be coming from some places like:
react/react#10135
https://gist.github.com/kentcdodds/5e54cdbadf1b08ed7003c33fc00bab4c

@snowystinger snowystinger Aug 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't love this approach, what else did you consider?

@starboyvarun starboyvarun Aug 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yeah, you caught me ,
I lifted that from the setNativeValue pattern floating around in those RTL/gist threads. Fair call, it shouldn't be in library code. It leans on React's internal value tracker, and now that I look again the new MouseEvent() is wrong too, since there's a getOwnerWindow call literally ten lines above it for iframe support.

Things I tried before landing on that one:-

First was just dispatching a bubbling change event. Cleaner, but it doesn't actually work — React uses click for radio/checkbox change detection (shouldUseClickEvent), so a native listener would see it but an ancestor React <form onChange> still wouldn't fire. Half a fix, so I dropped it.

Then I went down the tracker route, which is what you're looking at. It works, but yeah, I don't love it either now.

What I should have done from the start is just call nextElem.click():

       nextElem.focus();
-      state.setSelectedValue(nextElem.value);
+      nextElem.click();

The browser sets checkedness through its own activation behavior instead of the checked setter, so the tracker goes stale on its own and React fires onChange down the normal path. useRadio's onChange then calls setSelectedValue for us, so that line goes away too. Nothing poking at React internals.

Ran it locally — RAC RadioGroup 74/74, v3 Radio 57/57, S2 RadioGroup 4/4. I also went and checked read-only/disabled, since setSelectedValue no-ops in those cases and I'm deleting the call: read-only arrow keys, read-only click, and disabled arrow keys all still leave the selection alone and never fire onChange. React restores the controlled checked after the click, so it holds up.

One other thing I noticed while measuring all this — my PR description is wrong. I said the stopPropagation() removal was about native events bubbling to the form. It isn't. If you attach a real form.addEventListener('change', ...), that already works on main for mouse clicks; it was only ever broken for keyboard. What stopPropagation() actually killed was the synthetic event reaching an ancestor React onChange. I'll rewrite the description and add a test with an actual native listener, because right now both my tests use React's onChange on the form and don't cover that case at all.

Before I push anything though — is .click() the direction you'd want here, or is there an approach you'd rather I take? I'm also fine splitting this up and landing just the stopPropagation removal first, with the keyboard side as a separate PR if you think that's worth discussing on its own. Happy to go whichever way you prefer rather than guessing again.

Object.getPrototypeOf(nextElem),
'checked'
)!.set!;
checkedSetter.call(nextElem, true);
nextElem.dispatchEvent(new MouseEvent('click', {bubbles: true}));

return true;
}
return false;
Expand Down