Skip to content

Fix transition and focus prop combination for PopoverPanel component#3361

Merged
RobinMalfait merged 2 commits intomainfrom
fix/issue-3358
Jul 4, 2024
Merged

Fix transition and focus prop combination for PopoverPanel component#3361
RobinMalfait merged 2 commits intomainfrom
fix/issue-3358

Conversation

@RobinMalfait
Copy link
Copy Markdown
Member

@RobinMalfait RobinMalfait commented Jul 4, 2024

This PR fixes an issue where the combination of the focus and transition prop on the PopoverPanel component didn't work as expected.

The issue is that we moved focus inside the PopoverPanel in an effect, and we relied on a useRef to get access to the PopoverPanel DOM node. However, due to the transition, this ref wasn't filled in. The moment the ref is filled in, it's too late and the effect doesn't re-run anymore.

Instead, we rely on the DOM element that's coming from state. This one is updated and will re-render the component because we update the state when the ref is available.

Fixes: #3358

@vercel
Copy link
Copy Markdown

vercel Bot commented Jul 4, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
headlessui-react ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 4, 2024 10:33am
headlessui-vue ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 4, 2024 10:33am

Comment thread packages/@headlessui-react/src/components/popover/popover.tsx
Comment thread packages/@headlessui-react/src/components/popover/popover.tsx
@RobinMalfait RobinMalfait merged commit d8f44e0 into main Jul 4, 2024
@RobinMalfait RobinMalfait deleted the fix/issue-3358 branch July 4, 2024 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PopoverPanel focus prop doesn't work when transition prop is set

2 participants