[useToolbar] Two toolbar items receive focus event, instead of one.

[zaboco]

Provide a general summary of the issue here

In the toolbar, when controlling the focus programmatically, via JS event handlers, two items receive the focus event when navigating to the toolbar with Tab. See "Steps to reproduce" below for an example.

🤔 Expected Behavior?

See "Steps to reproduce" below

😯 Current Behavior

See "Steps to reproduce" below

💁 Possible Solution

From what I investigated in the code, that's because there are two focus events - One is sent programmatically in useToolbar from onFocus and the other bubbles to the last button. The solution would be to stop the event propagation from the toolbar's onFocus

🔦 Context

No response

🖥️ Steps to Reproduce

This sandbox illustrates the issue.

Steps to reproduce:

• navigate to the toolbar using Tab
• select the middle item ("Cut"). Notice that the button turns green (that's programmatically applied via JS)
• navigate away from the toolbar using Tab
• navigate back with Shift + Tab

Expected:

• The previous item is selected (i.e. green), and only that one.

Actual

• Both the previous item and the last item (i.e. "Paste") are selected (i.e. green)

Version

"@react-aria/toolbar": "3.0.0-beta.0"

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

MacOS

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response

[snowystinger]

It appears this only happens with non-react-aria components. Our hooks take care of blocking out a potentially erroneous focus event https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/interactions/src/useFocus.ts#L62

You could use our useFocus hook to handle this (or useButton, which will use the focus hook for you).

I see two ways to address the particular line on our end.

react-spectrum/packages/@react-aria/toolbar/src/useToolbar.ts

Line 114 in 75417bb

lastFocused.current?.focus();

We can either set a timeout of 0 before moving the focus. This will allow the natural event cycle to complete in the expected order.
Or we can preventDefault&stopPropagation of the focus event. We'd still, however, get the blur event.

[zaboco]

Hi! Thanks for the quick response.

You could use our useFocus hook to handle this (or useButton, which will use the focus hook for you).

Unfortunately, we already have a components library in our app, with Buttons, etc.

Yes, setting the timeout 0 to re-arrange the events should work, i saw that there is already a PR in progress. Thanks 🙏🏻