From a7154dca1fdd0df6a4277619d4dcbfd1da4057e4 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Sat, 28 May 2022 17:17:19 +0200 Subject: [PATCH] Remove leftover code in Combobox component (#1514) * remove leftover code This code existed before we had the option to make the first option the "active" one. This also contains a bug in the React code where pressing "ArrowDown" in a closed Combobox opens the combobox and goes to the second item instead of the first option. * update changelog --- CHANGELOG.md | 2 ++ .../src/components/combobox/combobox.tsx | 24 ------------------- .../src/components/combobox/combobox.ts | 21 +--------------- 3 files changed, 3 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index abe3b4a26c..fb66a43a45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Ensure `Escape` propagates correctly in `Combobox` component ([#1511](https://github.com/tailwindlabs/headlessui/pull/1511)) +- Remove leftover code in Combobox component ([#1514](https://github.com/tailwindlabs/headlessui/pull/1514)) ## [Unreleased - @headlessui/vue] @@ -26,6 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Ensure `Escape` propagates correctly in `Combobox` component ([#1511](https://github.com/tailwindlabs/headlessui/pull/1511)) +- Remove leftover code in Combobox component ([#1514](https://github.com/tailwindlabs/headlessui/pull/1514)) ## [Unreleased - @headlessui/tailwindcss] diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index efad64f75c..1e68c36cb3 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -663,18 +663,6 @@ let Input = forwardRefWithAs(function Input< }, [ComboboxState.Closed]: () => { actions.openCombobox() - // TODO: We can't do this outside next frame because the options aren't rendered yet - // But doing this in next frame results in a flicker because the dom mutations are async here - // Basically: - // Sync -> no option list yet - // Next frame -> option list already rendered with selection -> dispatch -> next frame -> now we have the focus on the right element - - // TODO: The spec here is underspecified. There's mention of skipping to the next item when autocomplete has suggested something but nothing regarding a non-autocomplete selection/value - d.nextFrame(() => { - if (!data.value) { - actions.goToOption(Focus.Next) - } - }) }, }) @@ -804,18 +792,6 @@ let Button = forwardRefWithAs(function Button no option list yet - // Next frame -> option list already rendered with selection -> dispatch -> next frame -> now we have the focus on the right element - - // TODO: The spec here is underspecified. There's mention of skipping to the next item when autocomplete has suggested something but nothing regarding a non-autocomplete selection/value - d.nextFrame(() => { - if (!data.value) { - actions.goToOption(Focus.First) - } - }) } return d.nextFrame(() => data.inputRef.current?.focus({ preventScroll: true })) diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts index 29658ab1bb..76bb0c425b 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.ts @@ -545,18 +545,6 @@ export let ComboboxButton = defineComponent({ event.stopPropagation() if (api.comboboxState.value === ComboboxStates.Closed) { api.openCombobox() - // TODO: We can't do this outside next frame because the options aren't rendered yet - // But doing this in next frame results in a flicker because the dom mutations are async here - // Basically: - // Sync -> no option list yet - // Next frame -> option list already rendered with selection -> dispatch -> next frame -> now we have the focus on the right element - - // TODO: The spec here is underspecified. There's mention of skipping to the next item when autocomplete has suggested something but nothing regarding a non-autocomplete selection/value - nextTick(() => { - if (!api.value.value) { - api.goToOption(Focus.First) - } - }) } nextTick(() => api.inputRef.value?.focus({ preventScroll: true })) return @@ -689,14 +677,7 @@ export let ComboboxInput = defineComponent({ event.stopPropagation() return match(api.comboboxState.value, { [ComboboxStates.Open]: () => api.goToOption(Focus.Next), - [ComboboxStates.Closed]: () => { - api.openCombobox() - nextTick(() => { - if (!api.value.value) { - api.goToOption(Focus.First) - } - }) - }, + [ComboboxStates.Closed]: () => api.openCombobox(), }) case Keys.ArrowUp: