Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix displayValue syncing problem #1755

Merged
merged 11 commits into from
Aug 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
110 changes: 76 additions & 34 deletions packages/@headlessui-react/src/components/combobox/combobox.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -427,62 +427,52 @@ describe('Rendering', () => {
suppressConsoleLogs(async () => {
function Example() {
let [value, setValue] = useState(null)
let [suffix, setSuffix] = useState(false)

return (
<>
<Combobox value={value} onChange={setValue} nullable>
{({ open }) => {
if (!open) {
return (
<>
<Combobox.Input
onChange={NOOP}
displayValue={(str?: string) => `${str?.toUpperCase() ?? ''} closed`}
/>
<Combobox.Button>Trigger</Combobox.Button>
</>
)
<Combobox.Input
onChange={NOOP}
displayValue={(str?: string) =>
`${str?.toUpperCase() ?? ''} ${suffix ? 'with suffix' : 'no suffix'}`
}

return (
<>
<Combobox.Input
onChange={NOOP}
displayValue={(str?: string) => `${str?.toUpperCase() ?? ''} open`}
/>
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
<Combobox.Option value="a">Option A</Combobox.Option>
<Combobox.Option value="b">Option B</Combobox.Option>
<Combobox.Option value="c">Option C</Combobox.Option>
</Combobox.Options>
</>
)
}}
/>
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
<Combobox.Option value="a">Option A</Combobox.Option>
<Combobox.Option value="b">Option B</Combobox.Option>
<Combobox.Option value="c">Option C</Combobox.Option>
</Combobox.Options>
<button onClick={() => setSuffix((v) => !v)}>Toggle suffix</button>
</Combobox>
</>
)
}

render(<Example />)

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')
})
)

Expand Down Expand Up @@ -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 (
<Combobox value={value} onChange={setValue}>
{({ open }) => (
<>
<Combobox.Input
onChange={(event) => setQuery(event.target.value)}
displayValue={(person) => `${person ?? ''} - ${open ? 'open' : 'closed'}`}
/>

<Combobox.Button />

<Combobox.Options>
<Combobox.Option value="alice">alice</Combobox.Option>
<Combobox.Option value="bob">bob</Combobox.Option>
<Combobox.Option value="charlie">charlie</Combobox.Option>
</Combobox.Options>
</>
)}
</Combobox>
)
}

render(<Example />)

// 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', () => {
Expand Down
55 changes: 24 additions & 31 deletions packages/@headlessui-react/src/components/combobox/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -382,7 +379,6 @@ let ComboboxRoot = forwardRefWithAs(function Combobox<
() => ({
...state,
optionsPropsRef,
inputPropsRef,
labelRef,
inputRef,
buttonRef,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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<HTMLInputElement>) => {
switch (event.key) {
Expand Down
11 changes: 8 additions & 3 deletions packages/@headlessui-react/src/hooks/use-watch.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
import { useEffect, useRef } from 'react'
import { useEvent } from './use-event'

export function useWatch<T>(cb: (values: T[]) => void | (() => void), dependencies: T[]) {
let track = useRef<typeof dependencies>([])
export function useWatch<T extends any[]>(
cb: (newValues: [...T], oldValues: [...T]) => void | (() => void),
dependencies: [...T]
) {
let track = useRef<typeof dependencies>([] 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
}
Expand Down
2 changes: 1 addition & 1 deletion packages/@headlessui-vue/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Loading