From 3a9fec4f8351e380521fa42dab1f83d776f18e1d Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 20 Jun 2024 14:00:03 +0200 Subject: [PATCH 1/8] add internal `Frozen` component and `useFrozenData` hook --- .../@headlessui-react/src/internal/frozen.tsx | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 packages/@headlessui-react/src/internal/frozen.tsx diff --git a/packages/@headlessui-react/src/internal/frozen.tsx b/packages/@headlessui-react/src/internal/frozen.tsx new file mode 100644 index 0000000000..6deffe31b5 --- /dev/null +++ b/packages/@headlessui-react/src/internal/frozen.tsx @@ -0,0 +1,19 @@ +import React, { useState } from 'react' + +export function Frozen({ children, freeze }: { children: React.ReactNode; freeze: boolean }) { + let contents = useFrozenData(freeze, children) + return <>{contents} +} + +export function useFrozenData(freeze: boolean, data: T) { + let [frozenValue, setFrozenValue] = useState(data) + + // We should keep updating the frozen value, as long as we shouldn't freeze + // the value yet. The moment we should freeze the value we stop updating it + // which allows us to reference the "previous" (thus frozen) value. + if (!freeze && frozenValue !== data) { + setFrozenValue(data) + } + + return freeze ? frozenValue : data +} From 3e5cd1c64992f426675b3880c4fdb3bf8f5cdb6f Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 20 Jun 2024 14:00:43 +0200 Subject: [PATCH 2/8] implement frozen state for the `Combobox` component When the `Combobox` is in a closed state, but still visible (aka transitioning out), then we want to freeze the `children` of the `ComboboxOptions`. This way we still look at the old list while transitioning out and you can safely reset any `state` that filters the options in the `onClose` callback. Note: we want to only freeze the children of the `ComboboxOptions`, not the `ComboboxOptions` itself because we are still applying the necessary data attributes to make the transition happen. Similarly, if you are using the `virtual` prop, then we only freeze the `virtual.options` and render the _old_ list while transitioning out. --- .../src/components/combobox/combobox.tsx | 51 ++++++++++++++----- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 6927a4e7f1..3adce816c6 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -54,6 +54,7 @@ import { type AnchorProps, } from '../../internal/floating' import { FormFields } from '../../internal/form-fields' +import { Frozen, useFrozenData } from '../../internal/frozen' import { useProvidedId } from '../../internal/id' import { OpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' import type { EnsureArray, Props } from '../../types' @@ -1707,23 +1708,39 @@ function OptionsFn( onMouseDown: handleMouseDown, }) + // We should freeze when the `visible` state is true and if the `visible` + // state and the `comboboxState` are not in sync. This means that a transition + // is happening and the component is still visible (for the transition effect) + // but closed from a functionality perspective. + let shouldFreeze = visible && data.comboboxState === ComboboxState.Closed + + let options = useFrozenData(shouldFreeze, data.virtual?.options) + // Map the children in a scrollable container when virtualization is enabled - if (data.virtual && visible) { + if (data.virtual) { + if (options === undefined) throw new Error('Missing `options` in virtual mode') + Object.assign(theirProps, { - // @ts-expect-error The `children` prop now is a callback function that receives `{ option }`. - children: {theirProps.children}, + children: ( + + {/* @ts-expect-error The `children` prop now is a callback function that receives `{option}` */} + {theirProps.children} + + ), }) } // Frozen state, the selected value will only update visually when the user re-opens the - let [frozenValue, setFrozenValue] = useState(data.value) - if ( - data.value !== frozenValue && - data.comboboxState === ComboboxState.Open && - data.mode !== ValueMode.Multi - ) { - setFrozenValue(data.value) - } + let frozenValue = useFrozenData( + !(data.comboboxState === ComboboxState.Open && data.mode !== ValueMode.Multi), + data.value + ) let isSelected = useEvent((compareValue: unknown) => { return data.compare(frozenValue, compareValue) @@ -1736,7 +1753,17 @@ function OptionsFn( > {render({ ourProps, - theirProps, + theirProps: { + ...theirProps, + children: ( + + {typeof theirProps.children === 'function' + ? // @ts-expect-error The `children` prop now is a callback function + theirProps.children?.(slot) + : theirProps.children} + + ), + }, slot, defaultTag: DEFAULT_OPTIONS_TAG, features: OptionsRenderFeatures, From d1fc211b2266023412c97b039a7e2792df1d2149 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 20 Jun 2024 14:10:06 +0200 Subject: [PATCH 3/8] use `useFrozenData` in `Listbox` component --- .../src/components/listbox/listbox.tsx | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/@headlessui-react/src/components/listbox/listbox.tsx b/packages/@headlessui-react/src/components/listbox/listbox.tsx index 3cf106953f..d88c671de4 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.tsx @@ -12,7 +12,6 @@ import React, { useMemo, useReducer, useRef, - useState, type CSSProperties, type ElementType, type MutableRefObject, @@ -54,6 +53,7 @@ import { type AnchorPropsWithSelection, } from '../../internal/floating' import { FormFields } from '../../internal/form-fields' +import { useFrozenData } from '../../internal/frozen' import { useProvidedId } from '../../internal/id' import { OpenClosedProvider, State, useOpenClosed } from '../../internal/open-closed' import type { EnsureArray, Props } from '../../types' @@ -1116,14 +1116,11 @@ function OptionsFn( }) // Frozen state, the selected value will only update visually when the user re-opens the - let [frozenValue, setFrozenValue] = useState(data.value) - if ( - data.value !== frozenValue && - data.listboxState === ListboxStates.Open && - data.mode !== ValueMode.Multi - ) { - setFrozenValue(data.value) - } + let frozenValue = useFrozenData( + !(data.listboxState === ListboxStates.Open && data.mode !== ValueMode.Multi), + data.value + ) + let isSelected = useEvent((compareValue: unknown) => { return data.compare(frozenValue, compareValue) }) From 19260aee3bb53d12320b783eed46bad8aaee4296 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 20 Jun 2024 14:11:03 +0200 Subject: [PATCH 4/8] use `data-*` attributes and `transition` prop to simplify playgrounds --- .../pages/combobox/combobox-countries.tsx | 82 +++++---- .../pages/combobox/combobox-virtualized.tsx | 174 +++++++++--------- .../listbox/listbox-with-pure-tailwind.tsx | 6 +- 3 files changed, 135 insertions(+), 127 deletions(-) diff --git a/playgrounds/react/pages/combobox/combobox-countries.tsx b/playgrounds/react/pages/combobox/combobox-countries.tsx index 0ae9a813e1..02a7a0b5ad 100644 --- a/playgrounds/react/pages/combobox/combobox-countries.tsx +++ b/playgrounds/react/pages/combobox/combobox-countries.tsx @@ -72,51 +72,53 @@ export default function Home() { -
- - {countries.map((country) => ( - { - return classNames( - 'relative cursor-default select-none py-2 pl-3 pr-9 focus:outline-none', - active ? 'bg-indigo-600 text-white' : 'text-gray-900' - ) - }} - > - {({ active, selected }) => ( - <> + + {countries.map((country) => ( + { + return classNames( + 'relative cursor-default select-none py-2 pl-3 pr-9 focus:outline-none', + active ? 'bg-indigo-600 text-white' : 'text-gray-900' + ) + }} + > + {({ active, selected }) => ( + <> + + {country} + + {selected && ( - {country} + + + - {selected && ( - - - - - - )} - - )} - - ))} - -
+ )} + + )} + + ))} + diff --git a/playgrounds/react/pages/combobox/combobox-virtualized.tsx b/playgrounds/react/pages/combobox/combobox-virtualized.tsx index 700749eb19..9873ff5034 100644 --- a/playgrounds/react/pages/combobox/combobox-virtualized.tsx +++ b/playgrounds/react/pages/combobox/combobox-virtualized.tsx @@ -104,101 +104,107 @@ function Example({ virtual = true, data, initial }: { virtual?: boolean; data; i -
- {virtual ? ( - - {({ option }) => { - return ( - { - return classNames( - 'relative w-full cursor-default select-none py-2 pl-3 pr-9 focus:outline-none', - active ? 'bg-indigo-600 text-white' : 'text-gray-900' - ) - }} - > - {({ active, selected }) => ( - <> + {virtual ? ( + + {({ option }) => { + return ( + { + return classNames( + 'relative w-full cursor-default select-none py-2 pl-3 pr-9 focus:outline-none', + active ? 'bg-indigo-600 text-white' : 'text-gray-900' + ) + }} + > + {({ active, selected }) => ( + <> + + {option as any} + + {selected && ( - {option as any} + + + - {selected && ( - - - - - + )} + + )} + + ) + }} + + ) : ( + + {timezones.map((timezone, idx) => { + return ( + { + return classNames( + 'relative w-full cursor-default select-none py-2 pl-3 pr-9 focus:outline-none', + active ? 'bg-indigo-600 text-white' : 'text-gray-900' + ) + }} + > + {({ active, selected }) => ( + <> + - )} - - ) - }} - - ) : ( - - {timezones.map((timezone, idx) => { - return ( - { - return classNames( - 'relative w-full cursor-default select-none py-2 pl-3 pr-9 focus:outline-none', - active ? 'bg-indigo-600 text-white' : 'text-gray-900' - ) - }} - > - {({ active, selected }) => ( - <> + > + {timezone} + + {selected && ( - {timezone} + + + - {selected && ( - - - - - - )} - - )} - - ) - })} - - )} -
+ )} + + )} + + ) + })} + + )} diff --git a/playgrounds/react/pages/listbox/listbox-with-pure-tailwind.tsx b/playgrounds/react/pages/listbox/listbox-with-pure-tailwind.tsx index 7ee9ecbfe3..0d845b098a 100644 --- a/playgrounds/react/pages/listbox/listbox-with-pure-tailwind.tsx +++ b/playgrounds/react/pages/listbox/listbox-with-pure-tailwind.tsx @@ -73,12 +73,12 @@ export default function Home() { - + {name} - + Date: Thu, 20 Jun 2024 14:29:54 +0200 Subject: [PATCH 5/8] update changelog --- packages/@headlessui-react/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index f8c3cc473d..4b7fe6d887 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Ensure `ComboboxInput` does not sync with current value while typing ([#3259](https://github.com/tailwindlabs/headlessui/pull/3259)) - Cancel outside click behavior on touch devices when scrolling ([#3266](https://github.com/tailwindlabs/headlessui/pull/3266)) - Correctly apply conditional classses when using `` and `` components ([#3303](https://github.com/tailwindlabs/headlessui/pull/3303)) +- Improve UX by freezing `ComboboxOptions` while closing ([#3304](https://github.com/tailwindlabs/headlessui/pull/3304)) ### Changed From d5378727b233c6070ae179422dcc1255e7f3bbf7 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 20 Jun 2024 15:50:50 +0200 Subject: [PATCH 6/8] improve comment --- .../@headlessui-react/src/components/combobox/combobox.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 3adce816c6..379890c2de 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -1708,10 +1708,9 @@ function OptionsFn( onMouseDown: handleMouseDown, }) - // We should freeze when the `visible` state is true and if the `visible` - // state and the `comboboxState` are not in sync. This means that a transition - // is happening and the component is still visible (for the transition effect) - // but closed from a functionality perspective. + // We should freeze when the combobox is visible but "closed". This means that + // a transition is currently happening and the component is still visible (for + // the transition) but closed from a functionality perspective. let shouldFreeze = visible && data.comboboxState === ComboboxState.Closed let options = useFrozenData(shouldFreeze, data.virtual?.options) From 6b8b28aafa26d48bc9c07b494209f27a49e649e0 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 20 Jun 2024 15:59:26 +0200 Subject: [PATCH 7/8] simplify frozen conditions --- .../src/components/combobox/combobox.tsx | 15 +++++---------- .../src/components/listbox/listbox.tsx | 14 +++++++------- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 379890c2de..0981d0d0df 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -1715,6 +1715,11 @@ function OptionsFn( let options = useFrozenData(shouldFreeze, data.virtual?.options) + // Frozen state, the selected value will only update visually when the user re-opens the + let frozenValue = useFrozenData(shouldFreeze, data.value) + + let isSelected = useEvent((compareValue) => data.compare(frozenValue, compareValue)) + // Map the children in a scrollable container when virtualization is enabled if (data.virtual) { if (options === undefined) throw new Error('Missing `options` in virtual mode') @@ -1735,16 +1740,6 @@ function OptionsFn( }) } - // Frozen state, the selected value will only update visually when the user re-opens the - let frozenValue = useFrozenData( - !(data.comboboxState === ComboboxState.Open && data.mode !== ValueMode.Multi), - data.value - ) - - let isSelected = useEvent((compareValue: unknown) => { - return data.compare(frozenValue, compareValue) - }) - return ( ( } as CSSProperties, }) + // We should freeze when the listbox is visible but "closed". This means that + // a transition is currently happening and the component is still visible (for + // the transition) but closed from a functionality perspective. + let shouldFreeze = visible && data.listboxState === ListboxStates.Closed + // Frozen state, the selected value will only update visually when the user re-opens the - let frozenValue = useFrozenData( - !(data.listboxState === ListboxStates.Open && data.mode !== ValueMode.Multi), - data.value - ) + let frozenValue = useFrozenData(shouldFreeze, data.value) - let isSelected = useEvent((compareValue: unknown) => { - return data.compare(frozenValue, compareValue) - }) + let isSelected = useEvent((compareValue: unknown) => data.compare(frozenValue, compareValue)) return ( From 28a5ee6bf0b28c91b92a7d53e27bf242bbfb12ff Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 20 Jun 2024 16:03:44 +0200 Subject: [PATCH 8/8] use existing variable for frozen state --- packages/@headlessui-react/src/components/combobox/combobox.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 0981d0d0df..f89e5140a8 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -1750,7 +1750,7 @@ function OptionsFn( theirProps: { ...theirProps, children: ( - + {typeof theirProps.children === 'function' ? // @ts-expect-error The `children` prop now is a callback function theirProps.children?.(slot)