Skip to content

Commit

Permalink
Fix crash when toggling between virtual and non-virtual mode in `Co…
Browse files Browse the repository at this point in the history
…mbobox` component (#3236)

* ensure we correctly merge the `virtual` configuration

* use more generic `UpdateVirtualOptions`

This way we can passthrough the `disabled` function as well.

* properly handle re-use of `disabled` function

* use same order in objects

* cleanup `!` and `?` if we already know we are in `virtual` mode

* directly enable virtual mode in state if previously we weren't using virtual mode

* update changelog
  • Loading branch information
RobinMalfait authored May 24, 2024
1 parent b478189 commit b822c8a
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 24 deletions.
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- [internal] Don’t set a focus fallback for Dialog’s in demo mode ([#3194](https://github.com/tailwindlabs/headlessui/pull/3194))
- Ensure page doesn't scroll down when pressing `Escape` to close the `Dialog` component ([#3218](https://github.com/tailwindlabs/headlessui/pull/3218))
- Fix crash when toggling between `virtual` and non-virtual mode in `Combobox` component ([#3236](https://github.com/tailwindlabs/headlessui/pull/3236))

### Deprecated

Expand Down
60 changes: 36 additions & 24 deletions packages/@headlessui-react/src/components/combobox/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ enum ActionTypes {

SetActivationTrigger,

UpdateVirtualOptions,
UpdateVirtualConfiguration,
}

function adjustOrderedState<T>(
Expand Down Expand Up @@ -180,7 +180,11 @@ type Actions<T> =
}
| { type: ActionTypes.UnregisterOption; id: string }
| { type: ActionTypes.SetActivationTrigger; trigger: ActivationTrigger }
| { type: ActionTypes.UpdateVirtualOptions; options: T[] }
| {
type: ActionTypes.UpdateVirtualConfiguration
options: T[]
disabled: ((value: any) => boolean) | null
}

let reducers: {
[P in ActionTypes]: <T>(
Expand Down Expand Up @@ -236,16 +240,15 @@ let reducers: {
}

if (state.virtual) {
let { options, disabled } = state.virtual
let activeOptionIndex =
action.focus === Focus.Specific
? action.idx
: calculateActiveIndex(action, {
resolveItems: () => state.virtual!.options,
resolveItems: () => options,
resolveActiveIndex: () =>
state.activeOptionIndex ??
state.virtual!.options.findIndex((option) => !state.virtual!.disabled(option)) ??
null,
resolveDisabled: state.virtual!.disabled,
state.activeOptionIndex ?? options.findIndex((option) => !disabled(option)) ?? null,
resolveDisabled: disabled,
resolveId() {
throw new Error('Function not implemented.')
},
Expand Down Expand Up @@ -373,14 +376,21 @@ let reducers: {
activationTrigger: action.trigger,
}
},
[ActionTypes.UpdateVirtualOptions]: (state, action) => {
if (state.virtual?.options === action.options) {
[ActionTypes.UpdateVirtualConfiguration]: (state, action) => {
if (state.virtual === null) {
return {
...state,
virtual: { options: action.options, disabled: action.disabled ?? (() => false) },
}
}

if (state.virtual.options === action.options && state.virtual.disabled === action.disabled) {
return state
}

let adjustedActiveOptionIndex = state.activeOptionIndex
if (state.activeOptionIndex !== null) {
let idx = action.options.indexOf(state.virtual!.options[state.activeOptionIndex])
let idx = action.options.indexOf(state.virtual.options[state.activeOptionIndex])
if (idx !== -1) {
adjustedActiveOptionIndex = idx
} else {
Expand All @@ -391,7 +401,7 @@ let reducers: {
return {
...state,
activeOptionIndex: adjustedActiveOptionIndex,
virtual: Object.assign({}, state.virtual, { options: action.options }),
virtual: { options: action.options, disabled: action.disabled ?? (() => false) },
}
},
}
Expand Down Expand Up @@ -425,6 +435,7 @@ function VirtualProvider(props: {
children: (data: { option: unknown; open: boolean }) => React.ReactElement
}) {
let data = useData('VirtualProvider')
let { options } = data.virtual!

let [paddingStart, paddingEnd] = useMemo(() => {
let el = data.optionsRef.current
Expand All @@ -441,7 +452,7 @@ function VirtualProvider(props: {
let virtualizer = useVirtualizer({
scrollPaddingStart: paddingStart,
scrollPaddingEnd: paddingEnd,
count: data.virtual!.options.length,
count: options.length,
estimateSize() {
return 40
},
Expand All @@ -454,7 +465,7 @@ function VirtualProvider(props: {
let [baseKey, setBaseKey] = useState(0)
useIsoMorphicEffect(() => {
setBaseKey((v) => v + 1)
}, [data.virtual?.options])
}, [options])

let items = virtualizer.getVirtualItems()

Expand Down Expand Up @@ -487,10 +498,7 @@ function VirtualProvider(props: {
return
}

if (
data.activeOptionIndex !== null &&
data.virtual!.options.length > data.activeOptionIndex
) {
if (data.activeOptionIndex !== null && options.length > data.activeOptionIndex) {
virtualizer.scrollToIndex(data.activeOptionIndex)
}
}
Expand All @@ -501,13 +509,13 @@ function VirtualProvider(props: {
<Fragment key={item.key}>
{React.cloneElement(
props.children?.({
option: data.virtual!.options[item.index],
option: options[item.index],
open: data.comboboxState === ComboboxState.Open,
}),