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.

Sign up for GitHub

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

Jump to bottom

Transitioned slideovers don't animate correctly #345

Closed
kesne opened this issue Apr 14, 2021 · 4 comments
Closed

Transitioned slideovers don't animate correctly #345

kesne opened this issue Apr 14, 2021 · 4 comments
Assignees
@RobinMalfait

Comments

@kesne
Copy link

kesne commented Apr 14, 2021 •
edited
Loading

What package within Headless UI are you using?

@headlessui/react

What version of that package are you using?

1.0

What browser are you using?

Chrome

Reproduction repository

Non-public

It looks like slideovers that are transitioned and implemented with Dialog don't animate correctly. It looks like this is an issue with the default focus behavior, mixing with browser behavior. The browser will scroll the element into view, despite it transitioning into view. It seems like the focus trap in the modal should use the NoScroll functionality.

I tested this using the examples provided in TailwindUI.
@reczy
Copy link

reczy commented Apr 15, 2021

I'm also having an issue with slide-over animation - doesn't sound like I'm seeing exactly the same thing but maybe it's related? Also using @headlessui/react 1.0, Chrome and Firefox (didn't test other browsers). If it matters, I'm using the postcss7-compat version of tailwind.

To help you reproduce the issue: The below gif is basically just using the react Tailwind UI code (Slide-overs -> With background overlay), plus a button to open.

Tailwind - Headless ui Slide-over animation

Also - Congrats on the release!

RobinMalfait added a commit that referenced this issue Apr 15, 2021
@RobinMalfait
enable NoScroll feature for the initial useFocusTrap hook
0ea8272 
Once you are using Tab and Shift+Tab it does the scrolling.

Fixes: #345
@RobinMalfait RobinMalfait mentioned this issue Apr 15, 2021
Merged
RobinMalfait added a commit that referenced this issue Apr 15, 2021
@RobinMalfait
Enable NoScroll feature for the initial useFocusTrap hook (#356)
19590b0 
* enable NoScroll feature for the initial useFocusTrap hook

Once you are using Tab and Shift+Tab it does the scrolling.

Fixes: #345

* update changelog
@RobinMalfait
Copy link
Member

@reczy can you open a separate issue for this?

@RobinMalfait RobinMalfait self-assigned this Apr 15, 2021
@reczy
Copy link

reczy commented Apr 15, 2021

@RobinMalfait absolutely - #357.

Thanks!

@RobinMalfait
Copy link
Member

@kesne Hey! Thank you for your bug report!
Much appreciated! 🙏

While the NoScroll is a solution it is not a good solution. We've updated the examples on https://tailwindui.com to fix these issues. Checkout #357 (comment) for the quick fix.

Reasons why the NoScroll is not a good enough option for now:

  • When you have a slide-over that contains an input in a scrollable section, then the transition would work. But the input would be focused but not visible to the end user.
  • Sadly, the preventScroll: true doesn't work on Safari

We are investigating a better solution that fixes this issue in a more robuust way.
Going to close it for now since the fixed applied to the examples in https://tailwindui.com seem to have fixed the main issue for this component.

RobinMalfait added a commit that referenced this issue Apr 19, 2021
@RobinMalfait
Enable NoScroll feature for the initial useFocusTrap hook (#356)
7a829a5 
* enable NoScroll feature for the initial useFocusTrap hook

Once you are using Tab and Shift+Tab it does the scrolling.

Fixes: #345

* update changelog
RobinMalfait added a commit that referenced this issue Apr 20, 2021
@RobinMalfait
Enable NoScroll feature for the initial useFocusTrap hook (#356)
5ad0f07 
* enable NoScroll feature for the initial useFocusTrap hook

Once you are using Tab and Shift+Tab it does the scrolling.

Fixes: #345

* update changelog
RobinMalfait added a commit that referenced this issue Apr 26, 2021
@RobinMalfait @shuvroroy
@alexnault @shoeyn @MichaelDeBoey
Next release (#431)
ce23ede 
* Fixed typos (#350)

* chore: Fix typo in render.ts (#347)

* Better vue link (#353)

* Better vue link

* add better React link

Co-authored-by: Robin Malfait <[email protected]>

* Enable NoScroll feature for the initial useFocusTrap hook (#356)

* enable NoScroll feature for the initial useFocusTrap hook

Once you are using Tab and Shift+Tab it does the scrolling.

Fixes: #345

* update changelog

* Revert "Enable NoScroll feature for the initial useFocusTrap hook (#356)"

This reverts commit 19590b0.

Solution is not 100% correct, so will revert for now!

* Improve search (#385)

* make search case insensitive for the listbox

* make search case insensitive for the menu

* update changelog

* add `disabled` prop to RadioGroup and RadioGroup Option (#401)

* add `disabled` prop to RadioGroup and RadioGroup Option

Also did some general cleanup which in turn fixed an issue where the
RadioGroup is unreachable when a value is used that doesn't exist in the
list of options.

Fixes: #378

* update changelog

* Fix type of `RadioGroupOption` (#400)

Match RadioGroupOption value types to match modelValue allowed types for RadioGroup

* update changelog

* fix typo's

* chore(CI): update main workflow (#395)

* chore(CI): update main workflow

* Update main.yml

* fix dialog event propagation (#422)

* re-export the `screen` utility for quick debugging purposes

* stop event propagation when clicking inside a Dialog

Fixes: #414

* improve dialog escape (#430)

* Make sure that `Escape` only closes the top most Dialog

* update changelog

* add defaultOpen prop to Disclosure component (#447)

* add defaultOpen prop to Disclosure component

* update changelog

Co-authored-by: Shuvro Roy <[email protected]>
Co-authored-by: Alex Nault <[email protected]>
Co-authored-by: Eugene Kopich <[email protected]>
Co-authored-by: Nathan Shoemark <[email protected]>
Co-authored-by: Michaël De Boey <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@RobinMalfait RobinMalfait

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@kesne @reczy @RobinMalfait