[RAC] ComboBox onSelectionChange not consistent with click and keypress

[ozguruysal]

Provide a general summary of the issue here

ComboBox onSelectionChange works differently with mouse click and keyboard. When I reselect the selected item by clicking with a mouse, onSelectionChange is fired every time. However when I do the same using keyboard by pressing Enter, the event is not fired on consecutive selections.

🤔 Expected Behavior?

I'd expect onSelectionChange to be fired each time like the mouse click to have a consistent behavior.

😯 Current Behavior

onSelectionChange is not fired on consecutive selects when an item is reselected using keyboard.

💁 Possible Solution

No response

🔦 Context

I'm working on a searchable menu component where we want the component to work as a searchable select as well as a menu, to trigger an event when an item is clicked. So there'll be no selection with menu behavior and a user should be able to fire the onSelectionChange when clicking on the same item next time.

🖥️ Steps to Reproduce

Please go to https://stackblitz.com/edit/vitejs-vite-eqeusx?file=src%2FApp.tsx

1. Open console
2. Select an item in the ComboBox by a mouse click.
3. Select the same item again a few times. Every time you select the same item, you can see onSelectionChange logs the selection in the console.
4. Select an item using keyboard only onSelectionChange logs the selection.
5. Select the same item using the keyboard and nothing will be logged to console.

Version

react-aria-components 1.3.1

What browsers are you seeing the problem on?

Firefox, Chrome, Safari, Other

If other, please specify.

Brave

What operating system are you using?

MacOS

🧢 Your Company/Team

Hygraph / Baukasten Design System

🕷 Tracking Issue

No response

[snowystinger]

Looks like we have some specific logic around this. You can get the behavior you want by putting the ComboBox into a fully controlled state by following this example. https://react-spectrum.adobe.com/react-aria/ComboBox.html#fully-controlled

Here's the line code path which causes this https://github.com/adobe/react-spectrum/blob/aa9f32d3cc2011a4d85564ca81a04d8b69ac3dad/packages/%40react-stately/combobox/src/useComboBoxState.ts#L308

@LFDanLu there any reason we don't want to always call the onSelectionChange? I know it's a little weird to call it again with the same value, so it's possible the bug is also that we call it at all for the mouse. Theoretically we could compare the value we're about to call onChange with against the controlled value from props/current state. But I don't see harm in calling it with the same key, it'd be ignored by anything using it to control since calling setState with the same value won't cause a re-render.

[LFDanLu]

Haha, I was just about to comment, but I think it should be fine, the only tests that failed weren't explicitly relying on onSelectionChange not being called. I quite frankly don't quite remember if there was an intentional reason not to fire onSelectionChange always when triggered via keyboard though nor could I find anything indicating the same. We'll have to double check all the various configurations just to be extra safe due to the complexity of the logic here.

Some related history around onSelectionChange: https://github.com/adobe/react-spectrum/pull/802 https://github.com/adobe/react-spectrum/issues/2536

[ozguruysal]

Thanks a lot for quick feedback. Using useComboBox, I was able to workaround it by chaining an onKeyDown to the input element and calling onSelectionChange manually on consecutive selections.

Just feels like the behavior should be the same both for mouse and keyboard users but don't know the implications of course 🙂

[LFDanLu]

Team discussed this some more, one alternative would be to support onAction on the ComboBox item where the behavior would be similar to a link where it wouldn't fill in the input but it would fire the action tied to said item. That way you wouldn't need to use onSelectionChange. That is handled here for link and would probably need something similar of onAction. Will need to double check what needs to be done for click handling.

[ozguruysal]

onAction sounds great. Thanks for the update!