Skip to content

fix: prevent onPress from being called when RadioGroup is read only #9433

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

Closed
yihuiliao wants to merge 1 commit into from
+23 −1
Closed

fix: prevent onPress from being called when RadioGroup is read only #9433

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
2 changes: 1 addition & 1 deletion packages/@react-aria/radio/src/useRadio.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export function useRadio(props: AriaRadioProps, state: RadioGroupState, ref: Ref
onPressChange,
onPressUp,
onClick,
isDisabled,
isDisabled: isDisabled || state.isReadOnly,
Copy link
Member

@snowystinger snowystinger Jan 3, 2026

Choose a reason for hiding this comment

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

huh... useToggle seems more like the bug, it was done to prevent us from allowing the value to be changed by our manual handling, it wasn't done to prevent onPress from being called.

I would argue that onPress should be called, interactions aren't prevented by readonly, for example, an input. You can click into those and select and copy text. You just can't change it. I'll ask a question in the Issue

Copy link
Member Author

@yihuiliao yihuiliao Jan 4, 2026

Choose a reason for hiding this comment

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

hm yeah i think that's a valid argument. fine to close this issue if we think it this is the correct behavior

onPress(e) {
onPress?.(e);
state.setSelectedValue(value);
Expand Down
22 changes: 22 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 @@ -276,6 +276,28 @@ describe('RadioGroup', () => {
expect(label).toHaveClass('readonly');
});

it('should not trigger onPress when read only', async () => {
let onPress = jest.fn();
let className = ({isReadOnly}) => isReadOnly ? 'readonly' : '';
let {getByRole, getAllByRole} = renderGroup({isReadOnly: true, className}, {className, onPress});

let group = getByRole('radiogroup');
let label = getAllByRole('radio')[0].closest('label');

expect(group).toHaveAttribute('aria-readonly');
expect(group).toHaveClass('readonly');
Copy link
Member

@snowystinger snowystinger Jan 3, 2026

Choose a reason for hiding this comment

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

this is a little weird, and possibly a different issue, why are the group and the label both getting the readonly class?

Copy link
Member Author

@yihuiliao yihuiliao Jan 4, 2026

Choose a reason for hiding this comment

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

i just copied that from the previous test to confirm the ready only part so different issue?


expect(label).toHaveAttribute('data-readonly');
expect(label).toHaveClass('readonly');

let radios = getAllByRole('radio');
let radioA = radios[0];
let labelA = radioA.closest('label');

await user.click(labelA);
expect(onPress).not.toHaveBeenCalled();
});

it('should support invalid state', () => {
let className = ({isInvalid}) => isInvalid ? 'invalid' : null;
let {getByRole, getAllByRole} = renderGroup({isInvalid: true, className}, {className});
Expand Down