useRadioGroup intercepts all arrow key presses leading to unexpected behavior

[hzr]

🐛 Bug Report

useRadioGroup intercepts arrow key presses (https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/radio/src/useRadioGroup.ts#L67) with the intention of selecting the next/previous radio button. However, if there are other focusable element in between the radio buttons, those elements will be focused. Using arrow keys in e.g. text inputs breaks, because it navigates between elements instead of moving the caret.

🤔 Expected Behavior

Using arrow keys in a radio group, I expect the previous/next radio button to become the selected and focused element.

😯 Current Behavior

Focusable elements that are not radio buttons get focus.

💻 Code Sample

https://codesandbox.io/s/clever-davinci-o474mu?file=/src/App.js

🌍 Your Environment

Software	Version(s)
react-spectrum	3.18.0
Browser
Operating System

[LFDanLu]

Mind sharing your use case for having non-radio elements within the radiogroup? I'm looking at the aria guidelines to see if non-radio elements are prohibited within the radio group, haven't found anything explicit yet though

[devongovett]

This is a duplicate of #2379. See the response here: #2379 (comment)

[hzr]

(I swear I searched before posting this, but I probably forgot to search for closed issues. My apologies.)

@LFDanLu My use case is very similar to the example in #2379 (comment)

@devongovett I read your comment in that issue, but I don't see how "Required Owned Elements" prohibits (in this case) non-radio's. But maybe other parts of the spec makes this clear.

For what it's worth, I found an example in MacOS that wouldn't be implementable (Preferences > Create a new Service (+) > Interface > VPN > Create and then click Authentication Settings):

I've also seen examples like:

◯ Option 1
◯ Option 2
◯ Other: [___________]

And I can imagine things like:

Do you want insurance?
◯ Yes (Read more about what our insurance covers)
◯ No

Of course, this might be disallowed by ARIA as you say, but people will inevitably implement stuff like this, and making sure arrow key presses only navigate between radio buttons will lead to less broken (and hence more accessible) behavior. IMHO. :)

[devongovett]

Yeah, "Required Owned Elements" means that the children must be one of the listed roles, and other non-presentational roles are not allowed. However, in this case the inverse is not true: the radio role does not have a "Required context role", so a radio is therefore allowed outside a radiogroup. Hence my suggestion to use a role=group or native <fieldset> instead.

Of course, this might be disallowed by ARIA as you say, but people will inevitably implement stuff like this, and making sure arrow key presses only navigate between radio buttons will lead to less broken (and hence more accessible) behavior.

True though due to the above, it would still produce an invalid accessibility tree which may cause issues with assistive technology like screen readers. I think we'd say that this is not actually a radio group, but a different pattern entirely, so it either shouldn't use the useRadioGroup hook (and use standalone radio buttons), or the interactive content should be placed outside the radio group rather than inline.

[hzr]

Yeah, "Required Owned Elements" means that the children must be one of the listed roles, and other non-presentational roles are not allowed.

The way I parse that section, a radiogroup must have at least one radio. I don't see that it says that anything else is not permitted.

I don't find a lot of info when searching, but this comment says:

[...] required owned elements has never been restrictive. In other words, it does not mean the same thing as "allowed" or "permitted" elements.

E.g. list has listitem as "required owned elements", and this is fine (with a button as a descendent):

<div role="list">
  <div role="listitem">Item 1</div>
  <div role="listitem"><button>Item 2</button></div>
</div>

[devongovett]

Yes, the wording is confusing, but "Allowed Child Elements" is how it has been interpreted by AT. See w3c/aria#1454 for a clarification to the spec.

E.g. list has listitem as "required owned elements", and this is fine (with a button as a descendent):

Yes, a button is allowed inside a listitem, but the button is not valid as a direct child of list.

[hzr]

[...] is how it has been interpreted by AT.

Ah, I see.

Yes, a button is allowed inside a listitem, but the button is not valid as a direct child of list.

Right, and I thought this was similar to

<div role="radiogroup" ...>
  <label>
    <input type="radio" ...>Option 1 <button>...</button>
  </label>
  <label>
    <input type="radio" ...>Option 2</a>
  </label>
</div>

where the button is not a direct child (and the element with role=radio is also not a direct child). But having read the new definition I can see how it's different if the label has a generic role(?)

I saw this comment on the linked PR:

Radiogroups do not require radio nodes as children, any descendant is fine.

But I'm not sure if this is talking about the same thing.

Anyway, thanks for the pointer @devongovett!