Fix Listbox not focusing first or last option on ArrowUp/ArrowDown (#3721)

This PR fixes an issue where if a Listbox does not have a value yet, and
it's opened via an ArrowUp or ArrowDown (on the ListboxButton) then it
didn't correctly go to the firs or last option.

Before, we were opening the listbox in a `flushSync()` call, after that
call we were focusing the first or last option depending on if you used
the ArrowDown or ArrowUp keys.

However, the options can and will be registered at a later point in
time, which means that the focus of first or last option is technically
going to fail because no options are available yet.

With this fix we don't need the `flushSync` call, and instead we
passthrough a pending focus. Once the options are registered, if a
pending focus is present, only then will we focus the correct option.

This gets rid of timing issues.
This commit is contained in:
Robin Malfait
2025-05-09 11:58:00 +02:00
committed by GitHub
parent 0b8deaf7a9
commit c9f8f30b90
3 changed files with 33 additions and 10 deletions
+1
View File
@@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fix clicking `Label` component should open `<Input type="file">` ([#3707](https://github.com/tailwindlabs/headlessui/pull/3707))
- Ensure clicking on interactive elements inside `Label` component works ([#3709](https://github.com/tailwindlabs/headlessui/pull/3709))
- Fix focus not returned to SVG Element ([#3704](https://github.com/tailwindlabs/headlessui/pull/3704))
- Fix `Listbox` not focusing first or last option on ArrowUp / ArrowDown ([#3721](https://github.com/tailwindlabs/headlessui/pull/3721))
## [2.2.2] - 2025-04-17
@@ -61,6 +61,7 @@ interface State<T> {
optionsElement: HTMLElement | null
pendingShouldSort: boolean
pendingFocus: { focus: Exclude<Focus, Focus.Specific> } | { focus: Focus.Specific; id: string }
}
export enum ActionTypes {
@@ -111,7 +112,10 @@ function adjustOrderedState<T>(
type Actions<T> =
| { type: ActionTypes.CloseListbox }
| { type: ActionTypes.OpenListbox }
| {
type: ActionTypes.OpenListbox
focus: { focus: Exclude<Focus, Focus.Specific> } | { focus: Focus.Specific; id: string }
}
| { type: ActionTypes.GoToOption; focus: Focus.Specific; id: string; trigger?: ActivationTrigger }
| {
type: ActionTypes.GoToOption
@@ -138,11 +142,12 @@ let reducers: {
return {
...state,
activeOptionIndex: null,
pendingFocus: { focus: Focus.Nothing },
listboxState: ListboxStates.Closed,
__demoMode: false,
}
},
[ActionTypes.OpenListbox](state) {
[ActionTypes.OpenListbox](state, action) {
if (state.dataRef.current.disabled) return state
if (state.listboxState === ListboxStates.Open) return state
@@ -155,7 +160,13 @@ let reducers: {
activeOptionIndex = optionIdx
}
return { ...state, listboxState: ListboxStates.Open, activeOptionIndex, __demoMode: false }
return {
...state,
pendingFocus: action.focus,
listboxState: ListboxStates.Open,
activeOptionIndex,
__demoMode: false,
}
},
[ActionTypes.GoToOption](state, action) {
if (state.dataRef.current.disabled) return state
@@ -311,6 +322,14 @@ let reducers: {
let options = state.options.concat(action.options)
let activeOptionIndex = state.activeOptionIndex
if (state.pendingFocus.focus !== Focus.Nothing) {
activeOptionIndex = calculateActiveIndex(state.pendingFocus, {
resolveItems: () => options,
resolveActiveIndex: () => state.activeOptionIndex,
resolveId: (item) => item.id,
resolveDisabled: (item) => item.dataRef.current.disabled,
})
}
// Check if we need to make the newly registered option active.
if (state.activeOptionIndex === null) {
@@ -325,6 +344,7 @@ let reducers: {
...state,
options,
activeOptionIndex,
pendingFocus: { focus: Focus.Nothing },
pendingShouldSort: true,
}
},
@@ -385,6 +405,8 @@ export class ListboxMachine<T> extends Machine<State<T>, Actions<T>> {
activationTrigger: ActivationTrigger.Other,
buttonElement: null,
optionsElement: null,
pendingShouldSort: false,
pendingFocus: { focus: Focus.Nothing },
__demoMode,
})
}
@@ -464,8 +486,10 @@ export class ListboxMachine<T> extends Machine<State<T>, Actions<T>> {
closeListbox: () => {
this.send({ type: ActionTypes.CloseListbox })
},
openListbox: () => {
this.send({ type: ActionTypes.OpenListbox })
openListbox: (
focus: { focus: Exclude<Focus, Focus.Specific> } | { focus: Focus.Specific; id: string }
) => {
this.send({ type: ActionTypes.OpenListbox, focus })
},
selectActiveOption: () => {
if (this.state.activeOptionIndex !== null) {
@@ -404,14 +404,12 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
case Keys.Space:
case Keys.ArrowDown:
event.preventDefault()
flushSync(() => machine.actions.openListbox())
if (!data.value) machine.actions.goToOption({ focus: Focus.First })
machine.actions.openListbox({ focus: data.value ? Focus.Nothing : Focus.First })
break
case Keys.ArrowUp:
event.preventDefault()
flushSync(() => machine.actions.openListbox())
if (!data.value) machine.actions.goToOption({ focus: Focus.Last })
machine.actions.openListbox({ focus: data.value ? Focus.Nothing : Focus.Last })
break
}
})
@@ -435,7 +433,7 @@ function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
machine.state.buttonElement?.focus({ preventScroll: true })
} else {
event.preventDefault()
machine.actions.openListbox()
machine.actions.openListbox({ focus: Focus.Nothing })
}
})