<object/>'s used to display images with a fallback break keyboard navigation inside of RadioGroup

lithdew commented 5 months ago

Provide a general summary of the issue here

I have the following JSX inside of a <Radio/>:

<object
      className={cn("aspect-square size-10 rounded object-cover", className)}
      width={Number(pass.avatar.width)}
      height={Number(pass.avatar.height)}
      data={pass.avatar.url}
      type={type}
      aria-label={pass.name}
      ref={forwardedRef}
      tabIndex={-1}
    >
      <img
        className={cn("aspect-square size-10 rounded object-cover", className)}
        width={Number(pass.avatar.width)}
        height={Number(pass.avatar.height)}
        src={pass.avatar.thumbnailUrl}
        alt={pass.name}
     />
</object>

This markup is used to implement images with a fallback as described here: https://blog.sentry.io/fallbacks-for-http-404-images-in-html-and-javascript/

When navigating with the keyboard across different <Radio/>'s inside of a <RadioGroup/>, the <object/>'s inside of each <Radio/> gets focused on rather than the the currently selected <Radio/>'s preceding/proceeding <Radio/>.

This is because <object/>'s are marked as focusable. I don't believe there is any way to mark <object/>'s as not focusable based on the code in getFocusableTreeWalker(). If there is a way to explicitly mark JSX elements as not focusable however, that would solve the issue.

Based on what I read from getFocusableTreeWalker() as well, setting a tabIndex of -1 to an element only marks the element as not tabbable which thus still causes this bug.

🤔 Expected Behavior?

That keyboard navigation across <Radio/>'s with <object/>'s representing images with fallbacks works.

😯 Current Behavior

Proceeding/preceding <Radio/>'s upon keyboard navigation do not properly get focused.

💁 Possible Solution

No response

🔦 Context

No response

🖥️ Steps to Reproduce

Version

react-aria 3.33.1, react-aria-components 1.2.1

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

Mac

🧢 Your Company/Team

Only1

🕷 Tracking Issue

No response

snowystinger commented 5 months ago

Thanks for reporting that. I'm not sure why we explicitly allow for object to be focusable according to our tree walker. They don't appear to be focusable on their own in plain HTML

https://jsfiddle.net/v0c2kn5L/8/ It looks like they aren't focusable by keyboards even if a tab index is specified. They can be programmatically focused, but would need a tabIndex >= 0

I'll see if anyone on the team remembers more about this line https://github.com/adobe/react-spectrum/blob/0c17289de18041e6f8b99df26a5a3ca922cc5145/packages/%40react-aria/focus/src/FocusScope.tsx#L279

lithdew commented 5 months ago

It looks like what needs to be done to check if an element is focusable/tabbable is a lot more complex than it seems vs. the checks in getFocusableTreeWalker(): https://allyjs.io/data-tables/focusable.html#object-element

In the specific case of RadioGroup though, I think the code could be amended to solely walk the tree for preceeding/proceeding <Radio/> items based on my cursory read of the ARIA RadioGroup spec.

https://www.w3.org/WAI/ARIA/apg/patterns/radio/

For Radio Groups Not Contained in a Toolbar

Right Arrow and Down Arrow: move focus to the next radio button in the group, uncheck the previously focused button, and check the newly focused button. If focus is on the last button, focus moves to the first button.

Left Arrow and Up Arrow: move focus to the previous radio button in the group, uncheck the previously focused button, and check the newly focused button. If focus is on the first button, focus moves to the last button.

For Radio Group Contained in a Toolbar

Right Arrow: When focus is on a radio button and that radio button is not the last radio button in the radio group, moves focus to the next radio button. When focus is on the last radio button in the radio group and that radio button is not the last element in the toolbar, moves focus to the next element in the toolbar. When focus is on the last radio button in the radio group and that radio button is also the last element in the toolbar, moves focus to the first element in the toolbar.

Left Arrow: When focus is on a radio button and that radio button is not the first radio button in the radio group, moves focus to the previous radio button. When focus is on the first radio button in the radio group and that radio button is not the first element in the toolbar, moves focus to the previous element in the toolbar. When focus is on the first radio button in the radio group and that radio button is also the first element in the toolbar, moves focus to the last element in the toolbar.

Down Arrow (optional): Moves focus to the next radio button in the radio group. If focus is on the last radio button in the radio group, moves focus to the first radio button in the group.

Up Arrow (optional): Moves focus to the previous radio button in the radio group. If focus is on the first radio button in the radio group, moves focus to the last radio button in the group.

majornista commented 5 months ago

Thanks for reporting that. I'm not sure why we explicitly allow for object to be focusable according to our tree walker. They don't appear to be focusable on their own in plain HTML

https://jsfiddle.net/v0c2kn5L/8/ It looks like they aren't focusable by keyboards even if a tab index is specified. They can be programmatically focused, but would need a tabIndex >= 0

I'll see if anyone on the team remembers more about this line

https://github.com/adobe/react-spectrum/blob/0c17289de18041e6f8b99df26a5a3ca922cc5145/packages/%40react-aria/focus/src/FocusScope.tsx#L279

It was more relevant in the age of plugins, but object tags are focusable depending on the element being rendered.

adobe / react-spectrum