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

Initial Collapse stuck on 'expanding' #29

Closed
marcopixel opened this issue Jun 26, 2024 · 7 comments · Fixed by #30
Closed

Initial Collapse stuck on 'expanding' #29

marcopixel opened this issue Jun 26, 2024 · 7 comments · Fixed by #30
Assignees
Labels
bug Something isn't working

Comments

@marcopixel
Copy link

marcopixel commented Jun 26, 2024

Description

I've made a wrapper component with your library and when setting the parent props to expanded true and if there's an element inside changing size it will freeze at the expanding stage without any errors. I've added as an example TinyMCE but this will happen with other elements changing the size while transitioning.

Reproduction

https://stackblitz.com/edit/vitejs-vite-1wliil?file=src%2FApp.vue,src%2Fcomponents%2FCollapse.vue,src%2Fcomponents%2FEditor.vue

Steps to Reproduce:

  1. Wait for the page to load
  2. The TinyMCE Window at the bottom will be cutoff

Expected: The collapsed content should be extended completly and all contents should be visible.
Actual: The collapsed content is partially cutoff and not completly visible until we reopen the collapse.

Aufzeichnung.2024-06-26.093858.1.mp4
@marcopixel
Copy link
Author

Might be a similar issue to #20.

@smastrom
Copy link
Owner

Hi @marcopixel, thank you for reporting the issue.

I think you pasted the wrong StackBlitz link. However, I was able to find it by browsing your profile space. As far as I can see, there are no issues:

Screenshot 2024-06-26 alle 13 53 44

Maybe the reproduction is still WIP?

@marcopixel
Copy link
Author

Hi @smastrom - thanks for the quick response and feedback, appreciate it 😄

I've corrected the StackBlitz link now, also i've fixed a few minor things which i've noticed in your screenshot (maily TinyMCE not initializing in FF because of a CORS issue) so the issue wouldn't even show up on your end because the fallback is just rendering a normal textarea field instead.

If you can please try it again 🙏

It's pretty consistent on my end and i can trigger it by refreshing the window inside StackBlitz like 9/10 times, seems like it fails to properly go to the end height (since it grows while transitioning) and thus gets stuck at the last height value before change.

You can see it pretty clearly cutoff in the video down below:

Timeline.1.mp4

@smastrom smastrom self-assigned this Jun 27, 2024
@smastrom smastrom added the bug Something isn't working label Jun 27, 2024
@smastrom
Copy link
Owner

Yes, I now understand what's happening. Since TinyMCE loads aynchronously, most of the times it is already loaded before expanding, sometimes it is loaded while transitioning. When that happens, fails to enter the expanded state because the scrollHeight doesn't match the computed height.

Same happens for other scenarios where for whatever reason, a child may change its height while expanding.

I need to reason a little bit about how to work around this as I'd like to find a rock solid solution.

For the moment, some workarounds that comes to my mind are:

  • Load TinyMCE when the expand transition is completed using @expanded callback (maybe by displaying a Spinner in the meantime).
  • Load TinyMCE in a container which has a fixed height
  • Only allow expanding/collapse after TinyMCE is loaded

Thank you again for providing the reproduction, I will keep you posted about any progress in the next hours or days.

@smastrom smastrom linked a pull request Jun 27, 2024 that will close this issue
@marcopixel
Copy link
Author

Just wanted you to know that i've cloned your pull request and i can verify it is indeed working now as expected :)

@smastrom
Copy link
Owner

Yeah, I just need to write a couple of tests and then it will be ready to go. Unfortunately I have been a bit busy at work at the moment but I am confident this will be merged the next week.

@marcopixel
Copy link
Author

@smastrom

No worries, didn't want to stress ya. Just wanted to verify that the issue is fixed with your changes :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants