diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 1127e958b0..98caeccb71 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -19,7 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Don’t close dialog when drag ends outside dialog ([#1667](https://github.com/tailwindlabs/headlessui/pull/1667)) - Fix outside clicks to close dialog when nested, unopened dialogs are present ([#1667](https://github.com/tailwindlabs/headlessui/pull/1667)) - Close `Menu` component when using `tab` key ([#1673](https://github.com/tailwindlabs/headlessui/pull/1673)) -- Resync input when display value changes ([#1679](https://github.com/tailwindlabs/headlessui/pull/1679)) +- Resync input when display value changes ([#1679](https://github.com/tailwindlabs/headlessui/pull/1679), [#1755](https://github.com/tailwindlabs/headlessui/pull/1755)) - Ensure controlled `Tabs` don't change automagically ([#1680](https://github.com/tailwindlabs/headlessui/pull/1680)) - Don't scroll lock when a Transition + Dialog is mounted but hidden ([#1681](https://github.com/tailwindlabs/headlessui/pull/1681)) - Improve outside click on Safari iOS ([#1712](https://github.com/tailwindlabs/headlessui/pull/1712)) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx index 5ef29f9263..2d11699c3c 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx @@ -427,38 +427,24 @@ describe('Rendering', () => { suppressConsoleLogs(async () => { function Example() { let [value, setValue] = useState(null) + let [suffix, setSuffix] = useState(false) return ( <> - {({ open }) => { - if (!open) { - return ( - <> - `${str?.toUpperCase() ?? ''} closed`} - /> - Trigger - - ) + + `${str?.toUpperCase() ?? ''} ${suffix ? 'with suffix' : 'no suffix'}` } - - return ( - <> - `${str?.toUpperCase() ?? ''} open`} - /> - Trigger - - Option A - Option B - Option C - - - ) - }} + /> + Trigger + + Option A + Option B + Option C + + ) @@ -466,23 +452,27 @@ describe('Rendering', () => { render() - expect(getComboboxInput()).toHaveValue(' closed') + expect(getComboboxInput()).toHaveValue(' no suffix') await click(getComboboxButton()) - assertComboboxList({ state: ComboboxState.Visible }) - - expect(getComboboxInput()).toHaveValue(' open') + expect(getComboboxInput()).toHaveValue(' no suffix') await click(getComboboxOptions()[1]) - expect(getComboboxInput()).toHaveValue('B closed') + expect(getComboboxInput()).toHaveValue('B no suffix') + + await click(getByText('Toggle suffix')) + + expect(getComboboxInput()).toHaveValue('B no suffix') // No re-sync yet await click(getComboboxButton()) - assertComboboxList({ state: ComboboxState.Visible }) + expect(getComboboxInput()).toHaveValue('B no suffix') // No re-sync yet - expect(getComboboxInput()).toHaveValue('B open') + await click(getComboboxOptions()[0]) + + expect(getComboboxInput()).toHaveValue('A with suffix') }) ) @@ -510,6 +500,58 @@ describe('Rendering', () => { expect(getComboboxInput()).toHaveAttribute('type', 'search') }) ) + + xit( + 'should reflect the value in the input when the value changes and when you are typing', + suppressConsoleLogs(async () => { + function Example() { + let [value, setValue] = useState('bob') + let [_query, setQuery] = useState('') + + return ( + + {({ open }) => ( + <> + setQuery(event.target.value)} + displayValue={(person) => `${person ?? ''} - ${open ? 'open' : 'closed'}`} + /> + + + + + alice + bob + charlie + + + )} + + ) + } + + render() + + // Check for proper state sync + expect(getComboboxInput()).toHaveValue('bob - closed') + await click(getComboboxButton()) + expect(getComboboxInput()).toHaveValue('bob - open') + await click(getComboboxButton()) + expect(getComboboxInput()).toHaveValue('bob - closed') + + // Check if we can still edit the input + for (let _ of Array(' - closed'.length)) { + await press(Keys.Backspace, getComboboxInput()) + } + getComboboxInput()?.select() + await type(word('alice'), getComboboxInput()) + expect(getComboboxInput()).toHaveValue('alice') + + // Open the combobox and choose an option + await click(getComboboxOptions()[2]) + expect(getComboboxInput()).toHaveValue('charlie - closed') + }) + ) }) describe('Combobox.Label', () => { diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 6f67109fc2..120b6f7c5c 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -41,6 +41,7 @@ import { useOpenClosed, State, OpenClosedProvider } from '../../internal/open-cl import { Keys } from '../keyboard' import { useControllable } from '../../hooks/use-controllable' +import { useWatch } from '../../hooks/use-watch' enum ComboboxState { Open, @@ -262,9 +263,6 @@ let ComboboxDataContext = createContext< isSelected(value: unknown): boolean __demoMode: boolean - inputPropsRef: MutableRefObject<{ - displayValue?(item: unknown): string - }> optionsPropsRef: MutableRefObject<{ static: boolean hold: boolean @@ -352,7 +350,6 @@ let ComboboxRoot = forwardRefWithAs(function Combobox< let defaultToFirstOption = useRef(false) let optionsPropsRef = useRef<_Data['optionsPropsRef']['current']>({ static: false, hold: false }) - let inputPropsRef = useRef<_Data['inputPropsRef']['current']>({ displayValue: undefined }) let labelRef = useRef<_Data['labelRef']['current']>(null) let inputRef = useRef<_Data['inputRef']['current']>(null) @@ -382,7 +379,6 @@ let ComboboxRoot = forwardRefWithAs(function Combobox< () => ({ ...state, optionsPropsRef, - inputPropsRef, labelRef, inputRef, buttonRef, @@ -440,32 +436,17 @@ let ComboboxRoot = forwardRefWithAs(function Combobox< [data, disabled, value] ) - let syncInputValue = useCallback(() => { - if (!data.inputRef.current) return - let displayValue = inputPropsRef.current.displayValue - - if (typeof displayValue === 'function') { - data.inputRef.current.value = displayValue(value) ?? '' - } else if (typeof value === 'string') { - data.inputRef.current.value = value - } else { - data.inputRef.current.value = '' - } - }, [value, data.inputRef, inputPropsRef.current?.displayValue]) - let selectOption = useEvent((id: string) => { let option = data.options.find((item) => item.id === id) if (!option) return onChange(option.dataRef.current.value) - syncInputValue() }) let selectActiveOption = useEvent(() => { if (data.activeOptionIndex !== null) { let { dataRef, id } = data.options[data.activeOptionIndex] onChange(dataRef.current.value) - syncInputValue() // It could happen that the `activeOptionIndex` stored in state is actually null, // but we are getting the fallback active option back instead. @@ -531,13 +512,6 @@ let ComboboxRoot = forwardRefWithAs(function Combobox< [] ) - useIsoMorphicEffect(() => { - if (data.comboboxState !== ComboboxState.Closed) return - syncInputValue() - }, [syncInputValue, data.comboboxState]) - - // Ensure that we update the inputRef if the value changes - useIsoMorphicEffect(syncInputValue, [syncInputValue]) let ourProps = ref === null ? {} : { ref } return ( @@ -612,14 +586,33 @@ let Input = forwardRefWithAs(function Input< let actions = useActions('Combobox.Input') let inputRef = useSyncRefs(data.inputRef, ref) - let inputPropsRef = data.inputPropsRef let id = `headlessui-combobox-input-${useId()}` let d = useDisposables() - useIsoMorphicEffect(() => { - inputPropsRef.current.displayValue = displayValue - }, [displayValue, inputPropsRef]) + let currentValue = useMemo(() => { + if (typeof displayValue === 'function') { + return displayValue(data.value as unknown as TType) ?? '' + } else if (typeof data.value === 'string') { + return data.value + } else { + return '' + } + + // displayValue is intentionally left out + }, [data.value]) + + useWatch( + ([currentValue, state], [oldCurrentValue, oldState]) => { + if (!data.inputRef.current) return + if (oldState === ComboboxState.Open && state === ComboboxState.Closed) { + data.inputRef.current.value = currentValue + } else if (currentValue !== oldCurrentValue) { + data.inputRef.current.value = currentValue + } + }, + [currentValue, data.comboboxState] + ) let handleKeyDown = useEvent((event: ReactKeyboardEvent) => { switch (event.key) { diff --git a/packages/@headlessui-react/src/hooks/use-watch.ts b/packages/@headlessui-react/src/hooks/use-watch.ts index 18f5917411..ccdc16786e 100644 --- a/packages/@headlessui-react/src/hooks/use-watch.ts +++ b/packages/@headlessui-react/src/hooks/use-watch.ts @@ -1,15 +1,20 @@ import { useEffect, useRef } from 'react' import { useEvent } from './use-event' -export function useWatch(cb: (values: T[]) => void | (() => void), dependencies: T[]) { - let track = useRef([]) +export function useWatch( + cb: (newValues: [...T], oldValues: [...T]) => void | (() => void), + dependencies: [...T] +) { + let track = useRef([] as unknown as [...T]) let action = useEvent(cb) useEffect(() => { + let oldValues = [...track.current] as unknown as [...T] + for (let [idx, value] of dependencies.entries()) { if (track.current[idx] !== value) { // At least 1 item changed - let returnValue = action(dependencies) + let returnValue = action(dependencies, oldValues) track.current = dependencies return returnValue } diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index 98a12fb45a..7702b1549e 100644 --- a/packages/@headlessui-vue/CHANGELOG.md +++ b/packages/@headlessui-vue/CHANGELOG.md @@ -19,7 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Don’t close dialog when drag ends outside dialog ([#1667](https://github.com/tailwindlabs/headlessui/pull/1667)) - Fix outside clicks to close dialog when nested, unopened dialogs are present ([#1667](https://github.com/tailwindlabs/headlessui/pull/1667)) - Close `Menu` component when using `tab` key ([#1673](https://github.com/tailwindlabs/headlessui/pull/1673)) -- Resync input when display value changes ([#1679](https://github.com/tailwindlabs/headlessui/pull/1679)) +- Resync input when display value changes ([#1679](https://github.com/tailwindlabs/headlessui/pull/1679), [#1755](https://github.com/tailwindlabs/headlessui/pull/1755)) - Ensure controlled `Tabs` don't change automagically ([#1680](https://github.com/tailwindlabs/headlessui/pull/1680)) - Improve outside click on Safari iOS ([#1712](https://github.com/tailwindlabs/headlessui/pull/1712)) - Improve event handler merging ([#1715](https://github.com/tailwindlabs/headlessui/pull/1715)) diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts index 0430539431..2c1585eebd 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts @@ -389,7 +389,7 @@ describe('Rendering', () => { }) }) - describe('Combobox.Input', () => { + describe('ComboboxInput', () => { it( 'selecting an option puts the value into Combobox.Input when displayValue is not provided', suppressConsoleLogs(async () => { @@ -454,53 +454,51 @@ describe('Rendering', () => { }) ) - it( - 'conditionally rendering the input should allow changing the display value', - suppressConsoleLogs(async () => { - let Example = defineComponent({ - template: html` - - - - - `, - setup: () => ({ value: ref(null) }), - }) + it('conditionally rendering the input should allow changing the display value', async () => { + let Example = defineComponent({ + template: html` + + + Trigger + + Option A + Option B + Option C + + + + `, + setup: () => ({ value: ref(null), suffix: ref(false) }), + }) - renderTemplate(Example) + renderTemplate(Example) - await nextFrame() + await nextFrame() - expect(getComboboxInput()).toHaveValue(' closed') + expect(getComboboxInput()).toHaveValue(' no suffix') - await click(getComboboxButton()) + await click(getComboboxButton()) - assertComboboxList({ state: ComboboxState.Visible }) + expect(getComboboxInput()).toHaveValue(' no suffix') - expect(getComboboxInput()).toHaveValue(' open') + await click(getComboboxOptions()[1]) - await click(getComboboxOptions()[1]) + expect(getComboboxInput()).toHaveValue('B no suffix') - expect(getComboboxInput()).toHaveValue('B closed') + await click(getByText('Toggle suffix')) - await click(getComboboxButton()) + expect(getComboboxInput()).toHaveValue('B no suffix') // No re-sync yet - assertComboboxList({ state: ComboboxState.Visible }) + await click(getComboboxButton()) - expect(getComboboxInput()).toHaveValue('B open') - }) - ) + expect(getComboboxInput()).toHaveValue('B no suffix') // No re-sync yet + + await click(getComboboxOptions()[0]) + + expect(getComboboxInput()).toHaveValue('A with suffix') + }) it( 'should be possible to override the `type` on the input', @@ -525,6 +523,54 @@ describe('Rendering', () => { expect(getComboboxInput()).toHaveAttribute('type', 'search') }) ) + + xit( + 'should reflect the value in the input when the value changes and when you are typing', + suppressConsoleLogs(async () => { + renderTemplate({ + template: html` + + + + + + + alice + bob + charlie + + + `, + setup: () => ({ + value: ref('bob'), + displayValue(person: string, open: boolean) { + return `${person ?? ''} - ${open ? 'open' : 'closed'}` + }, + }), + }) + + await nextFrame() + + // Check for proper state sync + expect(getComboboxInput()).toHaveValue('bob - closed') + await click(getComboboxButton()) + expect(getComboboxInput()).toHaveValue('bob - open') + await click(getComboboxButton()) + expect(getComboboxInput()).toHaveValue('bob - closed') + + // Check if we can still edit the input + for (let _ of Array(' - closed'.length)) { + await press(Keys.Backspace, getComboboxInput()) + } + getComboboxInput()?.select() + await type(word('alice'), getComboboxInput()) + expect(getComboboxInput()).toHaveValue('alice') + + // Open the combobox and choose an option + await click(getComboboxOptions()[2]) + expect(getComboboxInput()).toHaveValue('charlie - closed') + }) + ) }) describe('ComboboxLabel', () => { diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts index 5e2c10dc39..8cfa1ce7f0 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.ts @@ -70,7 +70,6 @@ type StateDefinition = { compare: (a: unknown, z: unknown) => boolean - inputPropsRef: Ref<{ displayValue?: (item: unknown) => string }> optionsPropsRef: Ref<{ static: boolean; hold: boolean }> labelRef: Ref @@ -217,7 +216,6 @@ export let Combobox = defineComponent({ return activeOptionIndex.value }), activationTrigger, - inputPropsRef: ref({ displayValue: undefined }), optionsPropsRef, closeCombobox() { defaultToFirstOption.value = false @@ -296,19 +294,6 @@ export let Combobox = defineComponent({ activationTrigger.value = trigger ?? ActivationTrigger.Other options.value = adjustedState.options }, - syncInputValue() { - let value = api.value.value - if (!dom(api.inputRef)) return - let displayValue = api.inputPropsRef.value.displayValue - - if (typeof displayValue === 'function') { - api.inputRef!.value!.value = displayValue(value) ?? '' - } else if (typeof value === 'string') { - api.inputRef!.value!.value = value - } else { - api.inputRef!.value!.value = '' - } - }, selectOption(id: string) { let option = options.value.find((item) => item.id === id) if (!option) return @@ -332,7 +317,6 @@ export let Combobox = defineComponent({ }, }) ) - api.syncInputValue() }, selectActiveOption() { if (api.activeOptionIndex.value === null) return @@ -356,7 +340,6 @@ export let Combobox = defineComponent({ }, }) ) - api.syncInputValue() // It could happen that the `activeOptionIndex` stored in state is actually null, // but we are getting the fallback active option back instead. @@ -406,26 +389,6 @@ export let Combobox = defineComponent({ computed(() => comboboxState.value === ComboboxStates.Open) ) - watch([api.value, api.inputRef, api.inputPropsRef], () => api.syncInputValue(), { - immediate: true, - deep: true, - }) - - // Only sync the input value on close as typing into the input will trigger it to open - // causing a resync of the input value with the currently stored, stale value that is - // one character behind since the input's value has just been updated by the browser - watch( - api.comboboxState, - (state) => { - if (state === ComboboxStates.Closed) { - api.syncInputValue() - } - }, - { - immediate: true, - } - ) - // @ts-expect-error Types of property 'dataRef' are incompatible. provide(ComboboxContext, api) useOpenClosedProvider( @@ -647,12 +610,44 @@ export let ComboboxInput = defineComponent({ let api = useComboboxContext('ComboboxInput') let id = `headlessui-combobox-input-${useId()}` - watchEffect(() => { - api.inputPropsRef.value = props - }) - expose({ el: api.inputRef, $el: api.inputRef }) + let currentValue = ref(api.value.value as unknown as string) + + let getCurrentValue = () => { + let value = api.value.value + if (!dom(api.inputRef)) return '' + + if (typeof props.displayValue !== 'undefined') { + return props.displayValue(value as unknown) ?? '' + } else if (typeof value === 'string') { + return value + } else { + return '' + } + } + + onMounted(() => { + watch([api.value], () => (currentValue.value = getCurrentValue()), { + flush: 'sync', + immediate: true, + }) + + watch( + [currentValue, api.comboboxState], + ([currentValue, state], [oldCurrentValue, oldState]) => { + let input = dom(api.inputRef) + if (!input) return + if (oldState === ComboboxStates.Open && state === ComboboxStates.Closed) { + input.value = currentValue + } else if (currentValue !== oldCurrentValue) { + input.value = currentValue + } + }, + { immediate: true } + ) + }) + function handleKeyDown(event: KeyboardEvent) { switch (event.key) { // Ref: https://www.w3.org/TR/wai-aria-practices-1.2/#keyboard-interaction-12 diff --git a/packages/@headlessui-vue/src/hooks/use-controllable.ts b/packages/@headlessui-vue/src/hooks/use-controllable.ts index 566caccdd6..239671c159 100644 --- a/packages/@headlessui-vue/src/hooks/use-controllable.ts +++ b/packages/@headlessui-vue/src/hooks/use-controllable.ts @@ -1,4 +1,4 @@ -import { computed, ComputedRef, ref } from 'vue' +import { computed, ComputedRef, UnwrapRef, ref } from 'vue' export function useControllable( controlledValue: ComputedRef, @@ -14,7 +14,7 @@ export function useControllable( if (isControlled.value) { return onChange?.(value as T) } else { - internalValue.value = value as T + internalValue.value = value as UnwrapRef return onChange?.(value as T) } }, diff --git a/packages/@headlessui-vue/src/internal/stack-context.ts b/packages/@headlessui-vue/src/internal/stack-context.ts index 075bc925cd..419762426f 100644 --- a/packages/@headlessui-vue/src/internal/stack-context.ts +++ b/packages/@headlessui-vue/src/internal/stack-context.ts @@ -8,8 +8,6 @@ import { InjectionKey, Ref, watch, - ref, - onBeforeUnmount, } from 'vue' type OnUpdate = (message: StackMessage, type: string, element: Ref) => void