Skip to content

Conversation

@RobinMalfait
Copy link
Member

We had an issue where an open Dialog got hidden by css didn't properly unmount because the Transition never "finished". We fixed this by checking if the node was hidden by using getBoundingClientRect.

Today I learned that just reading those values (aka call node.getBoundingClientRect()) it for whatever reason completely stops the transition. This causes the enter transitions to completely stop working.

Instead, we move this code so that we only check the existence of the Node when we try to transition out because this means that the Node is definitely there, just have to check its bounding rect.

Fixes: #1503

We had an issue where an open Dialog got hidden by css didn't properly
unmount because the Transition never "finished". We fixed this by
checking if the node was hidden by using `getBoundingClientRect`.

Today I learned that just *reading* those values (aka call
`node.getBoundingClientRect()`) it for whatever reason completely stops
the transition. This causes the enter transitions to completely stop
working.

Instead, we move this code so that we only check the existence of the
Node when we try to transition out because this means that the Node is
definitely there, just have to check its bounding rect.
@vercel
Copy link

vercel bot commented May 28, 2022

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

Name Status Preview Updated
headlessui-react ✅ Ready (Inspect) Visit Preview May 28, 2022 at 9:25PM (UTC)
headlessui-vue ✅ Ready (Inspect) Visit Preview May 28, 2022 at 9:25PM (UTC)

@RobinMalfait RobinMalfait merged commit ce12406 into main May 28, 2022
@RobinMalfait RobinMalfait deleted the fix/issue-1503 branch May 28, 2022 21:28
RobinMalfait added a commit that referenced this pull request Jul 8, 2024
This was originally introduced in
#1519 to fix an issue
where some enter transitions where broken: #1503

However, since we refactored the `Transition` component to make use of
the `useTransition` hook, I can't seem to reproduce this issue anymore.

In fact, removing this code fixes an issue.

The bigger issue here is that when the component becomes hidden, that we
always set the state to hidden as well. But if it becomes visible again,
we don't show it again.

Right now, since I couldn't reproduce any of the other issue, I opted to
just removing the code entirely. But if we ever need it again, we
probably have to make sure that it becomes visible in the other scenario
as well.

Fixes: #3328
RobinMalfait added a commit that referenced this pull request Jul 10, 2024
…dden (#3372)

* do not change visibility of `Transition` component

This was originally introduced in
#1519 to fix an issue
where some enter transitions where broken: #1503

However, since we refactored the `Transition` component to make use of
the `useTransition` hook, I can't seem to reproduce this issue anymore.

In fact, removing this code fixes an issue.

The bigger issue here is that when the component becomes hidden, that we
always set the state to hidden as well. But if it becomes visible again,
we don't show it again.

Right now, since I couldn't reproduce any of the other issue, I opted to
just removing the code entirely. But if we ever need it again, we
probably have to make sure that it becomes visible in the other scenario
as well.

Fixes: #3328

* update changelog
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.

Transition enterFrom classes not working as expected

2 participants