Skip to content

Commit

Permalink
Ensure DOM ref is properly handled in the RadioGroup component (#…
Browse files Browse the repository at this point in the history
…2424)

* drop `by` prop

Otherwise it ends up in the DOM which doesn't hurt but isn't ideal
either.

* ensure we are reading the underlying DOM correctly

We assumed that the `optionRef` was `HTMLElement | null`, but if you use
a custom component, then it is exposed as `{ $el: ref }`, this is why we
use the `dom()` helper.

* add test to ensure using a custom `as` prop works as expected

* update changelog
  • Loading branch information
RobinMalfait committed Apr 11, 2023
1 parent 7ec0652 commit 43a71cf
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 4 deletions.
1 change: 1 addition & 0 deletions packages/@headlessui-vue/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Disable `ComboboxInput` when its `Combobox` is disabled ([#2375](https://github.com/tailwindlabs/headlessui/pull/2375))
- Add `FocusTrap` event listeners once document has loaded ([#2389](https://github.com/tailwindlabs/headlessui/pull/2389))
- Don't scroll-lock `<Dialog>` when wrapping transition isn't showing ([#2422](https://github.com/tailwindlabs/headlessui/pull/2422))
- Ensure DOM `ref` is properly handled in the `RadioGroup` component ([#2424](https://github.com/tailwindlabs/headlessui/pull/2424))

### Added

Expand Down
27 changes: 27 additions & 0 deletions packages/@headlessui-vue/src/components/combobox/combobox.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1523,6 +1523,33 @@ describe('Rendering', () => {
expect(handleChange).toHaveBeenNthCalledWith(2, 'bob')
})
})

it(
'should be possible to use a custom component using the `as` prop without crashing',
suppressConsoleLogs(async () => {
let CustomComponent = defineComponent({
template: html`<button><slot /></button>`,
})

renderTemplate({
template: html`
<Combobox name="assignee">
<ComboboxInput />
<ComboboxButton />
<ComboboxOptions>
<ComboboxOption :as="CustomComponent" value="alice">Alice</RadioGroupOption>
<ComboboxOption :as="CustomComponent" value="bob">Bob</RadioGroupOption>
<ComboboxOption :as="CustomComponent" value="charlie">Charlie</RadioGroupOption>
</ComboboxOptions>
</Combobox>
`,
setup: () => ({ CustomComponent }),
})

// Open combobox
await click(getComboboxButton())
})
)
})

describe('Rendering composition', () => {
Expand Down
26 changes: 26 additions & 0 deletions packages/@headlessui-vue/src/components/listbox/listbox.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1270,6 +1270,32 @@ describe('Rendering', () => {
expect(handleChange).toHaveBeenNthCalledWith(2, 'bob')
})
})

it(
'should be possible to use a custom component using the `as` prop without crashing',
suppressConsoleLogs(async () => {
let CustomComponent = defineComponent({
template: html`<button><slot /></button>`,
})

renderTemplate({
template: html`
<Listbox name="assignee">
<ListboxButton />
<ListboxOptions>
<ListboxOption :as="CustomComponent" value="alice">Alice</RadioGroupOption>
<ListboxOption :as="CustomComponent" value="bob">Bob</RadioGroupOption>
<ListboxOption :as="CustomComponent" value="charlie">Charlie</RadioGroupOption>
</ListboxOptions>
</Listbox>
`,
setup: () => ({ CustomComponent }),
})

// Open listbox
await click(getListboxButton())
})
)
})

describe('Rendering composition', () => {
Expand Down
26 changes: 26 additions & 0 deletions packages/@headlessui-vue/src/components/menu/menu.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,32 @@ describe('Rendering', () => {
// Verify that the third menu item is active
assertMenuLinkedWithMenuItem(items[2])
})

it(
'should be possible to use a custom component using the `as` prop without crashing',
suppressConsoleLogs(async () => {
let CustomComponent = defineComponent({
template: `<button><slot /></button>`,
})

renderTemplate({
template: `
<Menu>
<MenuButton />
<MenuOptions>
<MenuOption :as="CustomComponent">Alice</RadioGroupOption>
<MenuOption :as="CustomComponent">Bob</RadioGroupOption>
<MenuOption :as="CustomComponent">Charlie</RadioGroupOption>
</MenuOptions>
</Menu>
`,
setup: () => ({ CustomComponent }),
})

// Open menu
await click(getMenuButton())
})
)
})

describe('Rendering composition', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { nextTick, ref, watch, reactive } from 'vue'
import { nextTick, ref, watch, reactive, defineComponent, defineExpose } from 'vue'
import { createRenderTemplate, render } from '../../test-utils/vue-testing-library'

import { RadioGroup, RadioGroupOption, RadioGroupLabel, RadioGroupDescription } from './radio-group'
Expand Down Expand Up @@ -496,6 +496,26 @@ describe('Rendering', () => {
assertActiveElement(getByText('Option 3'))
})

it(
'should be possible to use a custom component using the `as` prop without crashing',
suppressConsoleLogs(async () => {
let CustomComponent = defineComponent({
template: html`<button><slot /></button>`,
})

renderTemplate({
template: html`
<RadioGroup name="assignee">
<RadioGroupOption :as="CustomComponent" value="alice">Alice</RadioGroupOption>
<RadioGroupOption :as="CustomComponent" value="bob">Bob</RadioGroupOption>
<RadioGroupOption :as="CustomComponent" value="charlie">Charlie</RadioGroupOption>
</RadioGroup>
`,
setup: () => ({ CustomComponent }),
})
})
)

describe('Equality', () => {
let options = [
{ id: 1, name: 'Alice' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ export let RadioGroup = defineComponent({
: []),
render({
ourProps,
theirProps: { ...attrs, ...omit(theirProps, ['modelValue', 'defaultValue']) },
theirProps: { ...attrs, ...omit(theirProps, ['modelValue', 'defaultValue', 'by']) },
slot: {},
attrs,
slots,
Expand Down Expand Up @@ -309,7 +309,8 @@ export let RadioGroupOption = defineComponent({

expose({ el: optionRef, $el: optionRef })

onMounted(() => api.registerOption({ id: props.id, element: optionRef, propsRef }))
let element = computed(() => dom(optionRef))
onMounted(() => api.registerOption({ id: props.id, element, propsRef }))
onUnmounted(() => api.unregisterOption(props.id))

let isFirstOption = computed(() => api.firstOption.value?.id === props.id)
Expand All @@ -326,7 +327,7 @@ export let RadioGroupOption = defineComponent({
if (!api.change(props.value)) return

state.value |= OptionState.Active
optionRef.value?.focus()
dom(optionRef)?.focus()
}

function handleFocus() {
Expand Down

0 comments on commit 43a71cf

Please sign in to comment.