Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect scrolling to the bottom when opening a Dialog #1716

Merged
merged 4 commits into from Jul 26, 2022

Conversation

RobinMalfait
Copy link
Collaborator

This PR fixes an issue where the page is scrolled down before opening a Dialog.

This is because we tried to focus the first focusable element within the dialog before any
transition happens or whatsoever, this means that technically the dialog is still at the very bottom
of the page (because we render it at the end of the document.body via a Portal).

Focusing the element causes the browser to scroll the page down.

With the queueMicroTask we can delay that just enough to let the browser do its job.

Fixes: #1708

This will render the component as-is without the wrapper.
This will delay the initial focus and makes it consistent between React
and Vue.

Some explanation from within the code why this is happening:

   Delaying the focus to the next microtask ensures that a few
   conditions are true:

   - The container is rendered
   - Transitions could be started

   If we don't do this, then focusing an element will immediately cancel
   any transitions. This is not ideal because transitions will look
   broken. There is an additional issue with doing this immediately. The
   FocusTrap is used inside a Dialog, the Dialog is rendered inside of a
   Portal and the Portal is rendered at the end of the `document.body`.
   This means that the moment we call focus, the browser immediately
   tries to focus the element, which will still be at the bodem
   resulting in the page to scroll down. Delaying this will prevent the
   page to scroll down entirely.
Now that we are triggering the initial focus inside a `queueMicroTask`
we have to make sure that our tests wait a frame so that the micro task
could run, otherwise we will have incorrect results.

Also make the implementation similar in React and Vue
@vercel
Copy link

vercel bot commented Jul 26, 2022

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

Name Status Preview Updated
headlessui-react ✅ Ready (Inspect) Visit Preview Jul 26, 2022 at 1:27PM (UTC)
headlessui-vue ✅ Ready (Inspect) Visit Preview Jul 26, 2022 at 1:27PM (UTC)

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.

Unexpected scroll to bottom of page when re-opening dialog
1 participant