diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index d383fee596..bdd31ca275 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Ensure we handle `null` dataRef values correctly ([#2258](https://github.com/tailwindlabs/headlessui/pull/2258)) - Move `aria-multiselectable` to `[role=listbox]` in the `Combobox` component ([#2271](https://github.com/tailwindlabs/headlessui/pull/2271)) - Re-focus `Combobox.Input` when a `Combobox.Option` is selected ([#2272](https://github.com/tailwindlabs/headlessui/pull/2272)) +- Ensure we reset the `activeOptionIndex` if the active option is unmounted ([#2274](https://github.com/tailwindlabs/headlessui/pull/2274)) ## [1.7.10] - 2023-02-06 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/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index cb0e90bfd4..ca897347f7 100644 --- a/packages/@headlessui-vue/CHANGELOG.md +++ b/packages/@headlessui-vue/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Don’t fire `afterLeave` event more than once for a given transition ([#2267](https://github.com/tailwindlabs/headlessui/pull/2267)) - Move `aria-multiselectable` to `[role=listbox]` in the `Combobox` component ([#2271](https://github.com/tailwindlabs/headlessui/pull/2271)) - Re-focus `Combobox.Input` when a `Combobox.Option` is selected ([#2272](https://github.com/tailwindlabs/headlessui/pull/2272)) +- Ensure we reset the `activeOptionIndex` if the active option is unmounted ([#2274](https://github.com/tailwindlabs/headlessui/pull/2274)) ## [1.7.9] - 2023-02-03 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)