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

[LFDanLu]

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 #4821 (comment) 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

react-spectrum/packages/@react-aria/actiongroup/src/useActionGroup.ts

Lines 49 to 70 in 1775ff2

	switch (e.key) {
	case 'ArrowRight':
	case 'ArrowDown':
	e.preventDefault();
	e.stopPropagation();
	if (e.key === 'ArrowRight' && flipDirection) {
	focusManager.focusPrevious({wrap: true});
	} else {
	focusManager.focusNext({wrap: true});
	}
	break;
	case 'ArrowLeft':
	case 'ArrowUp':
	e.preventDefault();
	e.stopPropagation();
	if (e.key === 'ArrowLeft' && flipDirection) {
	focusManager.focusNext({wrap: true});
	} else {
	focusManager.focusPrevious({wrap: true});
	}
	break;
	}

🔦 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]

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]

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]

Sorry, I meant Tabs!🤪 I'll check back in on Monday.

[LFDanLu]

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

[LFDanLu]

@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!