From 255fc366687a912b4cd32b416c26faf76666772e Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 7 Jul 2022 18:34:39 +0200 Subject: [PATCH] Ensure `PopoverPanel` can be used inside `` (#1653) * ensure there is an animatable root node This is a bit sad, but it is how Vue works... We used to render just a simple PopoverPanel that resolved to let's say a `
`, that's all good. Because the native `` component requires that there is only 1 DOM child (regardless of the Vue "tree"). This is the sad part, because we simplified focus trapping for the Popover by introducing sibling hidden buttons to capture focus instead of managing this ourselves. Since we can't just return multiple items we wrap them in a `Fragment` component. If you wrap items in a Fragment, then a lot of Vue's magic goes away (automatically adding `class` to the root node). Luckily, Vue has a solution for that, which is `inheritAttrs: false` and then manually spreading the `attrs` onto the correct element. This all works beautiful, but not for the `` component... so... let's move the focus trappable elements inside the actual Panel and update the logic slightly to go to the Next/Previous item instead of the First/Last because the First/Last will now be the actual focus guards. * update changelog * make TypeScript a bit happier * improve `default` slot in `PopoverPanel` --- .../src/test-utils/interactions.ts | 2 +- packages/@headlessui-vue/CHANGELOG.md | 1 + .../src/components/popover/popover.ts | 73 ++++++++++--------- .../src/internal/focus-sentinel.ts | 2 +- .../src/test-utils/interactions.ts | 2 +- 5 files changed, 43 insertions(+), 37 deletions(-) diff --git a/packages/@headlessui-react/src/test-utils/interactions.ts b/packages/@headlessui-react/src/test-utils/interactions.ts index a742a94315..2b7976dae7 100644 --- a/packages/@headlessui-react/src/test-utils/interactions.ts +++ b/packages/@headlessui-react/src/test-utils/interactions.ts @@ -234,7 +234,7 @@ export async function click( ) { try { if (element === null) return expect(element).not.toBe(null) - if (element.disabled) return + if (element instanceof HTMLButtonElement && element.disabled) return let options = { button } diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index 9df9b897fe..916863ec26 100644 --- a/packages/@headlessui-vue/CHANGELOG.md +++ b/packages/@headlessui-vue/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix getting Vue dom elements ([#1610](https://github.com/tailwindlabs/headlessui/pull/1610)) - Ensure `CMD`+`Backspace` works in nullable mode for `Combobox` component ([#1617](https://github.com/tailwindlabs/headlessui/pull/1617)) - Properly merge incoming props with own props ([#1651](https://github.com/tailwindlabs/headlessui/pull/1651)) +- Ensure `PopoverPanel` can be used inside `` ([#1653](https://github.com/tailwindlabs/headlessui/pull/1653)) ## [1.6.5] - 2022-06-20 diff --git a/packages/@headlessui-vue/src/components/popover/popover.ts b/packages/@headlessui-vue/src/components/popover/popover.ts index 595ad99973..226220f4ce 100644 --- a/packages/@headlessui-vue/src/components/popover/popover.ts +++ b/packages/@headlessui-vue/src/components/popover/popover.ts @@ -542,7 +542,7 @@ export let PopoverPanel = defineComponent({ function run() { match(direction.value, { [TabDirection.Forwards]: () => { - focusIn(el, Focus.First) + focusIn(el, Focus.Next) }, [TabDirection.Backwards]: () => { // Coming from the Popover.Panel (which is portalled to somewhere else). Let's redirect @@ -592,7 +592,7 @@ export let PopoverPanel = defineComponent({ focusIn(combined, Focus.First, false) }, - [TabDirection.Backwards]: () => focusIn(el, Focus.Last), + [TabDirection.Backwards]: () => focusIn(el, Focus.Previous), }) } @@ -618,38 +618,43 @@ export let PopoverPanel = defineComponent({ tabIndex: -1, } - return h(Fragment, [ - visible.value && - api.isPortalled.value && - h(Hidden, { - id: beforePanelSentinelId, - ref: api.beforePanelSentinel, - features: HiddenFeatures.Focusable, - as: 'button', - type: 'button', - onFocus: handleBeforeFocus, - }), - render({ - ourProps, - theirProps: { ...attrs, ...props }, - slot, - attrs, - slots, - features: Features.RenderStrategy | Features.Static, - visible: visible.value, - name: 'PopoverPanel', - }), - visible.value && - api.isPortalled.value && - h(Hidden, { - id: afterPanelSentinelId, - ref: api.afterPanelSentinel, - features: HiddenFeatures.Focusable, - as: 'button', - type: 'button', - onFocus: handleAfterFocus, - }), - ]) + return render({ + ourProps, + theirProps: { ...attrs, ...props }, + attrs, + slot, + slots: { + ...slots, + default: (...args) => [ + h(Fragment, [ + visible.value && + api.isPortalled.value && + h(Hidden, { + id: beforePanelSentinelId, + ref: api.beforePanelSentinel, + features: HiddenFeatures.Focusable, + as: 'button', + type: 'button', + onFocus: handleBeforeFocus, + }), + slots.default?.(...args), + visible.value && + api.isPortalled.value && + h(Hidden, { + id: afterPanelSentinelId, + ref: api.afterPanelSentinel, + features: HiddenFeatures.Focusable, + as: 'button', + type: 'button', + onFocus: handleAfterFocus, + }), + ]), + ], + }, + features: Features.RenderStrategy | Features.Static, + visible: visible.value, + name: 'PopoverPanel', + }) } }, }) diff --git a/packages/@headlessui-vue/src/internal/focus-sentinel.ts b/packages/@headlessui-vue/src/internal/focus-sentinel.ts index 40370b491e..ddda4d52b4 100644 --- a/packages/@headlessui-vue/src/internal/focus-sentinel.ts +++ b/packages/@headlessui-vue/src/internal/focus-sentinel.ts @@ -33,7 +33,7 @@ export let FocusSentinel = defineComponent({ // Try to move focus to the correct element. This depends on the implementation // of `onFocus` of course since it would be different for each place we use it in. - if (props.onFocus()) { + if (props.onFocus?.()) { enabled.value = false cancelAnimationFrame(frame) return diff --git a/packages/@headlessui-vue/src/test-utils/interactions.ts b/packages/@headlessui-vue/src/test-utils/interactions.ts index 37f8169d8a..4f99cb731d 100644 --- a/packages/@headlessui-vue/src/test-utils/interactions.ts +++ b/packages/@headlessui-vue/src/test-utils/interactions.ts @@ -232,7 +232,7 @@ export async function click( ) { try { if (element === null) return expect(element).not.toBe(null) - if (element.disabled) return + if (element instanceof HTMLButtonElement && element.disabled) return let options = { button }