Skip to content

Commit

Permalink
Remove leftover code in Combobox component (#1514)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
RobinMalfait committed May 28, 2022
1 parent eefc03c commit a7154dc
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 44 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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]

Expand All @@ -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]

Expand Down
24 changes: 0 additions & 24 deletions packages/@headlessui-react/src/components/combobox/combobox.tsx
Expand Up @@ -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)
}
})
},
})

Expand Down Expand Up @@ -804,18 +792,6 @@ let Button = forwardRefWithAs(function Button<TTag extends ElementType = typeof
event.stopPropagation()
if (data.comboboxState === 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.First)
}
})
}
return d.nextFrame(() => data.inputRef.current?.focus({ preventScroll: true }))

Expand Down
21 changes: 1 addition & 20 deletions packages/@headlessui-vue/src/components/combobox/combobox.ts
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit a7154dc

Please sign in to comment.