diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx index a8bcbfa0e6..62b56953f7 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx @@ -4400,6 +4400,10 @@ describe('Keyboard interactions', () => { let options: ReturnType + options = getComboboxOptions() + expect(options[0]).toHaveTextContent('person a') + assertActiveComboboxOption(options[0]) + await press(Keys.ArrowDown) // Person B should be active @@ -5646,6 +5650,50 @@ describe('Multi-select', () => { assertComboboxOption(options[2], { selected: true }) }) ) + + it( + 'should reset the active option, if the active option gets unmounted', + suppressConsoleLogs(async () => { + let users = ['alice', 'bob', 'charlie'] + function Example() { + let [value, setValue] = useState([]) + + return ( + setValue(value)} multiple> + {}} /> + Trigger + + {users + .filter((user) => !value.includes(user)) + .map((user) => ( + + {user} + + ))} + + + ) + } + + render() + + // Open combobox + await click(getComboboxButton()) + assertCombobox({ state: ComboboxState.Visible }) + + let options = getComboboxOptions() + + // Go to the next option + await press(Keys.ArrowDown) + assertActiveComboboxOption(options[1]) + + // Select the option + await press(Keys.Enter) + + // The active option is reset to the very first one + assertActiveComboboxOption(options[0]) + }) + ) }) describe('Form compatibility', () => { diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index efb19f45e9..6e40f0c253 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -488,6 +488,17 @@ function ComboboxFn { + let currentActiveOption = + data.activeOptionIndex !== null ? data.options[data.activeOptionIndex] : null + if (lastActiveOption.current !== currentActiveOption) { + lastActiveOption.current = currentActiveOption + } + }) + useIsoMorphicEffect(() => { state.dataRef.current = data }, [data]) @@ -553,7 +564,22 @@ function ComboboxFn { dispatch({ type: ActionTypes.RegisterOption, id, dataRef }) - return () => dispatch({ type: ActionTypes.UnregisterOption, id }) + return () => { + // When we are unregistering the currently active option, then we also have to make sure to + // reset the `defaultToFirstOption` flag, so that visually something is selected and the next + // time you press a key on your keyboard it will go to the proper next or previous option in + // the list. + // + // Since this was the active option and it could have been anywhere in the list, resetting to + // the very first option seems like a fine default. We _could_ be smarter about this by going + // to the previous / next item in list if we know the direction of the keyboard navigation, + // but that might be too complex/confusing from an end users perspective. + if (lastActiveOption.current?.id === id) { + defaultToFirstOption.current = true + } + + dispatch({ type: ActionTypes.UnregisterOption, id }) + } }) let registerLabel = useEvent((id) => { diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts index 13b8febdd3..0fd8168d95 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts @@ -5879,6 +5879,50 @@ describe('Multi-select', () => { assertComboboxOption(options[2], { selected: true }) }) ) + + it( + 'should reset the active option, if the active option gets unmounted', + suppressConsoleLogs(async () => { + renderTemplate({ + template: html` + + + Trigger + + {{ user }} + + + `, + setup: () => { + let users = ['alice', 'bob', 'charlie'] + + let value = ref([]) + return { users, value } + }, + }) + + // Open combobox + await click(getComboboxButton()) + assertCombobox({ state: ComboboxState.Visible }) + + let options = getComboboxOptions() + + // Go to the next option + await press(Keys.ArrowDown) + assertActiveComboboxOption(options[1]) + + // Select the option + await press(Keys.Enter) + + // The active option is reset to the very first one + assertActiveComboboxOption(options[0]) + }) + ) }) describe('Form compatibility', () => { diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts index d608968866..820b50c803 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.ts @@ -391,6 +391,22 @@ export let Combobox = defineComponent({ activationTrigger.value = ActivationTrigger.Other }, unregisterOption(id: string) { + // When we are unregistering the currently active option, then we also have to make sure to + // reset the `defaultToFirstOption` flag, so that visually something is selected and the + // next time you press a key on your keyboard it will go to the proper next or previous + // option in the list. + // + // Since this was the active option and it could have been anywhere in the list, resetting + // to the very first option seems like a fine default. We _could_ be smarter about this by + // going to the previous / next item in list if we know the direction of the keyboard + // navigation, but that might be too complex/confusing from an end users perspective. + if ( + api.activeOptionIndex.value !== null && + api.options.value[api.activeOptionIndex.value]?.id === id + ) { + defaultToFirstOption.value = true + } + let adjustedState = adjustOrderedState((options) => { let idx = options.findIndex((a) => a.id === id) if (idx !== -1) options.splice(idx, 1)