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

Ensure by comparison is used in multiple mode #1717

Merged
merged 4 commits into from Jul 26, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/@headlessui-react/CHANGELOG.md
Expand Up @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

- Add `by` prop for `Listbox`, `Combobox` and `RadioGroup` ([#1482](https://github.com/tailwindlabs/headlessui/pull/1482))
- Add `by` prop for `Listbox`, `Combobox` and `RadioGroup` ([#1482](https://github.com/tailwindlabs/headlessui/pull/1482), [#1717](https://github.com/tailwindlabs/headlessui/pull/1717))
- Add `@headlessui/tailwindcss` plugin ([#1487](https://github.com/tailwindlabs/headlessui/pull/1487))

### Fixed
Expand Down
Expand Up @@ -272,6 +272,84 @@ describe('Rendering', () => {
)
})
)

it(
'should be possible to use completely new objects while rendering (single mode)',
suppressConsoleLogs(async () => {
function Example() {
let [value, setValue] = useState({ id: 2, name: 'Bob' })

return (
<Combobox value={value} onChange={setValue} by="id">
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
<Combobox.Option value={{ id: 1, name: 'alice' }}>alice</Combobox.Option>
<Combobox.Option value={{ id: 2, name: 'bob' }}>bob</Combobox.Option>
<Combobox.Option value={{ id: 3, name: 'charlie' }}>charlie</Combobox.Option>
</Combobox.Options>
</Combobox>
)
}

render(<Example />)

await click(getComboboxButton())
let [alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')

await click(getComboboxOptions()[2])
await click(getComboboxButton())
;[alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).not.toHaveAttribute('aria-selected')
expect(charlie).toHaveAttribute('aria-selected', 'true')

await click(getComboboxOptions()[1])
await click(getComboboxButton())
;[alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')
})
)

it(
'should be possible to use completely new objects while rendering (multiple mode)',
suppressConsoleLogs(async () => {
function Example() {
let [value, setValue] = useState([{ id: 2, name: 'Bob' }])

return (
<Combobox value={value} onChange={setValue} by="id" multiple>
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
<Combobox.Option value={{ id: 1, name: 'alice' }}>alice</Combobox.Option>
<Combobox.Option value={{ id: 2, name: 'bob' }}>bob</Combobox.Option>
<Combobox.Option value={{ id: 3, name: 'charlie' }}>charlie</Combobox.Option>
</Combobox.Options>
</Combobox>
)
}

render(<Example />)

await click(getComboboxButton())

await click(getComboboxOptions()[2])
let [alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).toHaveAttribute('aria-selected', 'true')

await click(getComboboxOptions()[2])
;[alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')
})
)
})
})

Expand Down
14 changes: 7 additions & 7 deletions packages/@headlessui-react/src/components/combobox/combobox.tsx
Expand Up @@ -315,7 +315,7 @@ let ComboboxRoot = forwardRefWithAs(function Combobox<
> & {
value: TType
onChange(value: TType): void
by?: (keyof TType & string) | ((a: TType, z: TType) => boolean)
by?: (keyof TActualType & string) | ((a: TActualType, z: TActualType) => boolean)
disabled?: boolean
__demoMode?: boolean
name?: string
Expand Down Expand Up @@ -356,19 +356,19 @@ let ComboboxRoot = forwardRefWithAs(function Combobox<

let compare = useEvent(
typeof by === 'string'
? (a: TType, z: TType) => {
let property = by as unknown as keyof TType
? (a: TActualType, z: TActualType) => {
let property = by as unknown as keyof TActualType
return a[property] === z[property]
}
: by
)

let isSelected: (value: TType) => boolean = useCallback(
let isSelected: (value: TActualType) => boolean = useCallback(
(compareValue) =>
match(data.mode, {
[ValueMode.Multi]: () =>
(value as unknown as TType[]).some((option) => compare(option, compareValue)),
[ValueMode.Single]: () => compare(value, compareValue),
(value as unknown as TActualType[]).some((option) => compare(option, compareValue)),
[ValueMode.Single]: () => compare(value as unknown as TActualType, compareValue),
}),
[value]
)
Expand Down Expand Up @@ -500,7 +500,7 @@ let ComboboxRoot = forwardRefWithAs(function Combobox<
[ValueMode.Multi]() {
let copy = (data.value as TActualType[]).slice()

let idx = copy.indexOf(value as TActualType)
let idx = copy.findIndex((item) => compare(item, value as TActualType))
if (idx === -1) {
copy.push(value as TActualType)
} else {
Expand Down
78 changes: 78 additions & 0 deletions packages/@headlessui-react/src/components/listbox/listbox.test.tsx
Expand Up @@ -263,6 +263,84 @@ describe('Rendering', () => {
)
})
)

it(
'should be possible to use completely new objects while rendering (single mode)',
suppressConsoleLogs(async () => {
function Example() {
let [value, setValue] = useState({ id: 2, name: 'Bob' })

return (
<Listbox value={value} onChange={setValue} by="id">
<Listbox.Button>Trigger</Listbox.Button>
<Listbox.Options>
<Listbox.Option value={{ id: 1, name: 'alice' }}>alice</Listbox.Option>
<Listbox.Option value={{ id: 2, name: 'bob' }}>bob</Listbox.Option>
<Listbox.Option value={{ id: 3, name: 'charlie' }}>charlie</Listbox.Option>
</Listbox.Options>
</Listbox>
)
}

render(<Example />)

await click(getListboxButton())
let [alice, bob, charlie] = getListboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')

await click(getListboxOptions()[2])
await click(getListboxButton())
;[alice, bob, charlie] = getListboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).not.toHaveAttribute('aria-selected')
expect(charlie).toHaveAttribute('aria-selected', 'true')

await click(getListboxOptions()[1])
await click(getListboxButton())
;[alice, bob, charlie] = getListboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')
})
)

it(
'should be possible to use completely new objects while rendering (multiple mode)',
suppressConsoleLogs(async () => {
function Example() {
let [value, setValue] = useState([{ id: 2, name: 'Bob' }])

return (
<Listbox value={value} onChange={setValue} by="id" multiple>
<Listbox.Button>Trigger</Listbox.Button>
<Listbox.Options>
<Listbox.Option value={{ id: 1, name: 'alice' }}>alice</Listbox.Option>
<Listbox.Option value={{ id: 2, name: 'bob' }}>bob</Listbox.Option>
<Listbox.Option value={{ id: 3, name: 'charlie' }}>charlie</Listbox.Option>
</Listbox.Options>
</Listbox>
)
}

render(<Example />)

await click(getListboxButton())

await click(getListboxOptions()[2])
let [alice, bob, charlie] = getListboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).toHaveAttribute('aria-selected', 'true')

await click(getListboxOptions()[2])
;[alice, bob, charlie] = getListboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')
})
)
})
})

Expand Down
11 changes: 7 additions & 4 deletions packages/@headlessui-react/src/components/listbox/listbox.tsx
Expand Up @@ -315,7 +315,7 @@ let ListboxRoot = forwardRefWithAs(function Listbox<
> & {
value: TType
onChange(value: TType): void
by?: (keyof TType & string) | ((a: TType, z: TType) => boolean)
by?: (keyof TActualType & string) | ((a: TActualType, z: TActualType) => boolean)
disabled?: boolean
horizontal?: boolean
name?: string
Expand Down Expand Up @@ -345,8 +345,8 @@ let ListboxRoot = forwardRefWithAs(function Listbox<
mode: multiple ? ValueMode.Multi : ValueMode.Single,
compare: useEvent(
typeof by === 'string'
? (a: TType, z: TType) => {
let property = by as unknown as keyof TType
? (a: TActualType, z: TActualType) => {
let property = by as unknown as keyof TActualType
return a[property] === z[property]
}
: by
Expand Down Expand Up @@ -377,7 +377,10 @@ let ListboxRoot = forwardRefWithAs(function Listbox<
[ValueMode.Multi]() {
let copy = (propsRef.current.value as TActualType[]).slice()

let idx = copy.indexOf(value as TActualType)
let { compare } = propsRef.current
let idx = copy.findIndex((item) =>
compare(item as unknown as TActualType, value as TActualType)
)
if (idx === -1) {
copy.push(value as TActualType)
} else {
Expand Down
2 changes: 1 addition & 1 deletion packages/@headlessui-vue/CHANGELOG.md
Expand Up @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

- Add `by` prop for `Listbox`, `Combobox` and `RadioGroup` ([#1482](https://github.com/tailwindlabs/headlessui/pull/1482))
- Add `by` prop for `Listbox`, `Combobox` and `RadioGroup` ([#1482](https://github.com/tailwindlabs/headlessui/pull/1482), [#1717](https://github.com/tailwindlabs/headlessui/pull/1717))
- Add `@headlessui/tailwindcss` plugin ([#1487](https://github.com/tailwindlabs/headlessui/pull/1487))

### Fixed
Expand Down
78 changes: 78 additions & 0 deletions packages/@headlessui-vue/src/components/combobox/combobox.test.ts
Expand Up @@ -308,6 +308,84 @@ describe('Rendering', () => {
)
})
)

it(
'should be possible to use completely new objects while rendering (single mode)',
suppressConsoleLogs(async () => {
renderTemplate({
template: html`
<Combobox v-model="value" by="id">
<ComboboxButton>Trigger</ComboboxButton>
<ComboboxOptions>
<ComboboxOption :value="{ id: 1, name: 'alice' }">alice</ComboboxOption>
<ComboboxOption :value="{ id: 2, name: 'bob' }">bob</ComboboxOption>
<ComboboxOption :value="{ id: 3, name: 'charlie' }">charlie</ComboboxOption>
</ComboboxOptions>
</Combobox>
`,
setup: () => {
let value = ref({ id: 2, name: 'Bob' })
return { options, value }
},
})

await click(getComboboxButton())
let [alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')

await click(getComboboxOptions()[2])
await click(getComboboxButton())
;[alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).not.toHaveAttribute('aria-selected')
expect(charlie).toHaveAttribute('aria-selected', 'true')

await click(getComboboxOptions()[1])
await click(getComboboxButton())
;[alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')
})
)

it(
'should be possible to use completely new objects while rendering (multiple mode)',
suppressConsoleLogs(async () => {
renderTemplate({
template: html`
<Combobox v-model="value" by="id" multiple>
<ComboboxButton>Trigger</ComboboxButton>
<ComboboxOptions>
<ComboboxOption :value="{ id: 1, name: 'alice' }">alice</ComboboxOption>
<ComboboxOption :value="{ id: 2, name: 'bob' }">bob</ComboboxOption>
<ComboboxOption :value="{ id: 3, name: 'charlie' }">charlie</ComboboxOption>
</ComboboxOptions>
</Combobox>
`,
setup: () => {
let value = ref([{ id: 2, name: 'Bob' }])
return { options, value }
},
})

await click(getComboboxButton())

await click(getComboboxOptions()[2])
let [alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).toHaveAttribute('aria-selected', 'true')

await click(getComboboxOptions()[2])
;[alice, bob, charlie] = getComboboxOptions()
expect(alice).not.toHaveAttribute('aria-selected')
expect(bob).toHaveAttribute('aria-selected', 'true')
expect(charlie).not.toHaveAttribute('aria-selected')
})
)
})
})

Expand Down
4 changes: 2 additions & 2 deletions packages/@headlessui-vue/src/components/combobox/combobox.ts
Expand Up @@ -316,7 +316,7 @@ export let Combobox = defineComponent({
let copy = toRaw(api.value.value as unknown[]).slice()
let raw = toRaw(dataRef.value)

let idx = copy.indexOf(raw)
let idx = copy.findIndex((value) => api.compare(raw, toRaw(value)))
if (idx === -1) {
copy.push(raw)
} else {
Expand All @@ -341,7 +341,7 @@ export let Combobox = defineComponent({
let copy = toRaw(api.value.value as unknown[]).slice()
let raw = toRaw(dataRef.value)

let idx = copy.indexOf(raw)
let idx = copy.findIndex((value) => api.compare(raw, toRaw(value)))
if (idx === -1) {
copy.push(raw)
} else {
Expand Down