adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.72k stars 1.09k forks source link

ActionGroup's ArrowRight/Left focus navigation is incorrectly flipped in orientation="vertical" + RTL locales #4896

Open LFDanLu opened 1 year ago

LFDanLu commented 1 year ago

Provide a general summary of the issue here

In LTR locales, ArrowRight/ArrowDown always move focus to the "next" button in the ActionGroup regardless of the ActionGroup's orientation. Similarly ArrowLeft/ArrowUp move focus to the "prev" button. Note how the arrow key pairings and the behavior never change.

In RTL ArrowLeft/ArrowDown move focus to the "next" button and ArrowRight/ArrowUp move focus to the "prev" button in ActionGroup's "horizontal" orientation. However, when the orientation switches to "vertical", ArrowRight/ArrowDown move focus to the "next" button and ArrowLeft/ArrowUp moves focus to the "prev" button. Note that pairings have changed and ArrowRight and ArrowLeft's behavior have been flipped

See https://github.com/adobe/react-spectrum/pull/4821#discussion_r1287787419 for a investigation of the history of this behavior

๐Ÿค” Expected Behavior?

In RTL ArrowLeft/ArrowDown should always move focus to the "next" button and ArrowRight/ArrowUp should move focus to the "prev" button regardless of orientation.

๐Ÿ˜ฏ Current Behavior

In RTL ArrowLeft/ArrowDown move focus to the "next" button and ArrowRight/ArrowUp move focus to the "prev" button in ActionGroup's "horizontal" orientation. However, when the orientation switches to "vertical", ArrowRight/ArrowDown move focus to the "next" button and ArrowLeft/ArrowUp moves focus to the "prev" button. Note that pairings have changed and ArrowRight and ArrowLeft's behavior have been flipped

This is the earliest storybook that had the correct behavior: https://reactspectrum.blob.core.windows.net/reactspectrum/046831ed00bf013c6f728bf474b270bf5656b208/storybook/index.html?path=/story/buttongroup--default

๐Ÿ’ Possible Solution

Changes need to be made in https://github.com/adobe/react-spectrum/blob/1775ff2a331be32c3b27417acc2705ad11019b37/packages/%40react-aria/actiongroup/src/useActionGroup.ts#L49-L70

๐Ÿ”ฆ Context

No response

๐Ÿ–ฅ๏ธ Steps to Reproduce

Behavior can be tested here: https://reactspectrum.blob.core.windows.net/reactspectrum/1775ff2a331be32c3b27417acc2705ad11019b37/storybook/index.html?path=/story/actiongroup--default&providerSwitcher-express=false&providerSwitcher-toastPosition=bottom

Version

3.29

What browsers are you seeing the problem on?

Other

If other, please specify.

All

What operating system are you using?

not OS specific, seen on MacOS and Windows

๐Ÿงข Your Company/Team

RSP

๐Ÿ•ท Tracking Issue

No response

mariareedstrom commented 1 year ago

Happy to work on this as I'm already familiar with the logic. Question, should I include TagGroup as well since the two should follow the same behavior?

LFDanLu commented 1 year ago

Thanks! Perhaps hold off until Monday or so, just want to run this by the team in case someone remembers a good reason for the current behavior haha. For TagGroup, I think we can hold off on that since we don't support "vertical" orientation for that yet.

mariareedstrom commented 1 year ago

Sorry, I meant Tabs!๐Ÿคช I'll check back in on Monday.

LFDanLu commented 1 year ago

@mariareedstrom just an update, seeing if I can get some opinions from our internationalization/globalization team at the moment.

LFDanLu commented 1 year ago

@mariareedstrom Got feedback from our internationalization/globalization team and they confirmed that ArrowLeft/Right should remain consistent regardless of ActionGroup/Tab orientation in RTL (aka ArrowLeft should move focus to the next item and ArrowRight should move focus to the prev item). Feel free to pick this issue up if you'd like!