From d58ac5648267150bfc6baf00d59acccfb10f6d14 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 11 Apr 2023 16:47:53 +0200 Subject: [PATCH 1/4] drop `by` prop Otherwise it ends up in the DOM which doesn't hurt but isn't ideal either. --- .../@headlessui-vue/src/components/radio-group/radio-group.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@headlessui-vue/src/components/radio-group/radio-group.ts b/packages/@headlessui-vue/src/components/radio-group/radio-group.ts index a198d1997..67824ddee 100644 --- a/packages/@headlessui-vue/src/components/radio-group/radio-group.ts +++ b/packages/@headlessui-vue/src/components/radio-group/radio-group.ts @@ -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, From 608733aae3112235cf572c94269fb3c97b81dce3 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 11 Apr 2023 16:48:23 +0200 Subject: [PATCH 2/4] 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. --- .../src/components/radio-group/radio-group.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/@headlessui-vue/src/components/radio-group/radio-group.ts b/packages/@headlessui-vue/src/components/radio-group/radio-group.ts index 67824ddee..e320a5461 100644 --- a/packages/@headlessui-vue/src/components/radio-group/radio-group.ts +++ b/packages/@headlessui-vue/src/components/radio-group/radio-group.ts @@ -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) @@ -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() { From c51b114fb1c5c0829cd86e71a0a68a4515673dff Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 11 Apr 2023 16:53:01 +0200 Subject: [PATCH 3/4] add test to ensure using a custom `as` prop works as expected --- .../src/components/combobox/combobox.test.ts | 27 +++++++++++++++++++ .../src/components/listbox/listbox.test.tsx | 26 ++++++++++++++++++ .../src/components/menu/menu.test.tsx | 26 ++++++++++++++++++ .../radio-group/radio-group.test.ts | 22 ++++++++++++++- 4 files changed, 100 insertions(+), 1 deletion(-) diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts index 4d39fbd29..3c066fb37 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts @@ -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``, + }) + + renderTemplate({ + template: html` + + + + + Alice + Bob + Charlie + + + `, + setup: () => ({ CustomComponent }), + }) + + // Open combobox + await click(getComboboxButton()) + }) + ) }) describe('Rendering composition', () => { diff --git a/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx b/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx index a40bbd134..56a8ab8b0 100644 --- a/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx +++ b/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx @@ -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``, + }) + + renderTemplate({ + template: html` + + + + Alice + Bob + Charlie + + + `, + setup: () => ({ CustomComponent }), + }) + + // Open listbox + await click(getListboxButton()) + }) + ) }) describe('Rendering composition', () => { diff --git a/packages/@headlessui-vue/src/components/menu/menu.test.tsx b/packages/@headlessui-vue/src/components/menu/menu.test.tsx index 8f87562c1..9b1e9bcc1 100644 --- a/packages/@headlessui-vue/src/components/menu/menu.test.tsx +++ b/packages/@headlessui-vue/src/components/menu/menu.test.tsx @@ -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: ``, + }) + + renderTemplate({ + template: ` + + + + Alice + Bob + Charlie + + + `, + setup: () => ({ CustomComponent }), + }) + + // Open menu + await click(getMenuButton()) + }) + ) }) describe('Rendering composition', () => { diff --git a/packages/@headlessui-vue/src/components/radio-group/radio-group.test.ts b/packages/@headlessui-vue/src/components/radio-group/radio-group.test.ts index 6cb0e4ce5..1a4cc83b1 100644 --- a/packages/@headlessui-vue/src/components/radio-group/radio-group.test.ts +++ b/packages/@headlessui-vue/src/components/radio-group/radio-group.test.ts @@ -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' @@ -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``, + }) + + renderTemplate({ + template: html` + + Alice + Bob + Charlie + + `, + setup: () => ({ CustomComponent }), + }) + }) + ) + describe('Equality', () => { let options = [ { id: 1, name: 'Alice' }, From 5373779ddbc8b16b9ff975e4f131e986483f6fc0 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Tue, 11 Apr 2023 16:55:26 +0200 Subject: [PATCH 4/4] update changelog --- packages/@headlessui-vue/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index 4dd6dfa72..087363428 100644 --- a/packages/@headlessui-vue/CHANGELOG.md +++ b/packages/@headlessui-vue/CHANGELOG.md @@ -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 `` 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