Skip to content

Commit

Permalink
Ensure PopoverPanel can be used inside <transition> (#1653)
Browse files Browse the repository at this point in the history
* 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 `<div>`, that's all good. Because the native `<transition>` 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 `<transition>` 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`
  • Loading branch information
RobinMalfait authored Jul 7, 2022
1 parent 0260afa commit 255fc36
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 37 deletions.
2 changes: 1 addition & 1 deletion packages/@headlessui-react/src/test-utils/interactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down
1 change: 1 addition & 0 deletions packages/@headlessui-vue/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<transition>` ([#1653](https://github.com/tailwindlabs/headlessui/pull/1653))

## [1.6.5] - 2022-06-20

Expand Down
73 changes: 39 additions & 34 deletions packages/@headlessui-vue/src/components/popover/popover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
})
}

Expand All @@ -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',
})
}
},
})
Expand Down
2 changes: 1 addition & 1 deletion packages/@headlessui-vue/src/internal/focus-sentinel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/@headlessui-vue/src/test-utils/interactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down

0 comments on commit 255fc36

Please sign in to comment.