Controlled ComboBox's onSelectionChange is fired when blurring the input

[estenp]

Provide a general summary of the issue here

Blurring the ComboBox input calls the onSelectionChange handler.

🤔 Expected Behavior?

onSelectionChange should only run when the selectedItem prop changes.

😯 Current Behavior

onSelectionChange runs on input blur (in addition to onBlur)

💁 Possible Solution

No response

🔦 Context

No response

🖥️ Steps to Reproduce

https://stackblitz.com/edit/vitejs-vite-w6je8woh?file=src%2FApp.tsx

Version

1.6.0

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

Mac OS

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response

[mversic]

hitting the same issue. This only happens if selectedKey prop is given. Otherwise if the key is uncontrolled, onSelectionChange works correctly

[mversic]

seems it's not so simple. You can:

1. focus the field
2. change a few letters
3. blur the field

the key selection hasn't changed but you would expect the input value to be restored as is done in this example. For an uncontrolled selection this happens automatically, but for the controlled it happens inside onSelectionChange.

tldr
I think the current behavior is correct because it has to support restoring the inputValue

[snowystinger]

Thanks for looking into that, yes, according to the docs for fully controlled, the onSelectionChange is needed in order to reset the input value. I'll close as working as intended.
If there are more concerns, we can reopen.
Thank you!

[estenp]

Hmm, not sure I'm tracking with why this would be intended. It seems like the primary (only?) use case for this behavior is if you have a ListItem active via Up/Down arrows and tab away from the ComboBox input. In that case, yes we want to select the active item on blur so an onSelectionChange callback would fire.

I don't see how this is intuitive in any other situation, as blurring the input only makes a selection in that scenario.

In terms of resetting the input value on blur, well, there is an onBlur handler, right? Wouldn't it make sense to do that there, rather than call onSelectionChange when selection never actually changed? Or is it not really possible to distinguish between those events?

[mversic]

I don't see how this is intuitive in any other situation, as blurring the input only makes a selection in that scenario.

IMO it's not intuitive at all. I didn't expect onSelectionChange would be called if no selection took place

In terms of resetting the input value on blur, well, there is an onBlur handler, right? Wouldn't it make sense to do that there, rather than call onSelectionChange when selection never actually changed? Or is it not really possible to distinguish between those events?

yeah, indeed it sounds as if onBlur handler would be a better fit here. I suppose this can be considered a bug after all

[LFDanLu]

Yeah, our current Combobox state hook implementation is pretty complex and we are hoping to refactor it in the future now that we've added Autocomplete support to RAC. We could possibly handle resetting the input value on blur, but then we'd need to do the following:

• Have onBlur be the only event fired when blurring the field, and include more information (aka the next selectedKey that the fully controlled combobox should update to, aka like in the case where a option in the field is focused and the user hits Tab) in the event returned by the blur handler. This makes it easier on the user to update their controlled state values since there is always only one event fired per interaction, aka the philosophy described in https://react-spectrum.adobe.com/react-spectrum/ComboBox.html#fully-controlled-combobox

This would be a breaking change unfortunately, and may still have unforeseen issues.

Alternatively, we could look to just not fire onSelectionChange when there isn't a selection to be made. Diving briefly into the code flow, this is the line that cause onSelection to be called when blurring:

react-spectrum/packages/@react-stately/combobox/src/useComboBoxState.ts

Lines 296 to 299 in bcc12ad

	let commitSelection = () => {
	// If multiple things are controlled, call onSelectionChange
	if (props.selectedKey !== undefined && props.inputValue !== undefined) {
	props.onSelectionChange?.(selectedKey);

, triggered by the following blur handling:

react-spectrum/packages/@react-stately/combobox/src/useComboBoxState.ts

Lines 344 to 346 in bcc12ad

	if (shouldCloseOnBlur) {
	commitValue();
	}

which then triggers the following:

react-spectrum/packages/@react-stately/combobox/src/useComboBoxState.ts

Lines 316 to 319 in bcc12ad

	} else {
	// Reset inputValue and close menu
	commitSelection();
	}

. Theoretically we could wrap the first props.onSelectionChange?.(selectedKey); in a check like if (selectedKey) {...

The problem is that this code is also called in cases such as the following flow:

• User types into a fully controlled combobox, types a couple of letter (say "blah"), then hits enter without focusing any of the items.

This flow SHOULD trigger onSelectionChange (or at least some event) so that the user knows to update their controlled inputValue from "blah" to "". We've somewhat classified firing onSelectionChange in these kinds of events as "the user has selected nothing" so that the fully controlled combobox's state can be updated to reflect the proper inputValue and selectedKey at all times.

[LFDanLu]

Again, just to reiterate, the team would love to untangle/refactor some of this interdependent logic in the combobox hooks are waiting till we finalize the Autocomplete hook logic and thus look to refactor ComboBox. I think it absolutely makes sense to not call onSelectionChange on blur if there isn't a need to reset the inputValue or update the selectedKey for a fully controlled case if possible.

However, in the scope of the original issue here, is there an issue that the onSelectionChange firing on blur is causing? Theoretically it is calling onSelectionChange with the same key that has been already set so long as you haven't focused another item in the dropdown before blurring, so you could probably safely ignore the call.