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 resize on update children components & simplified #310

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kir4ik
Copy link

@kir4ik kir4ik commented Jul 5, 2022

if child components are redrawn with a new height, this is not always tracked in getSnapshotBeforeUpdate. Therefore, I moved the height setting logic to onResize. In addition, getSnapshotBeforeUpdate assumes the use of previous states, but here it is not necessary, onResize has everything you need

@nkbt
Copy link
Owner

nkbt commented Jul 6, 2022

If child component is updated, this is not supposed to be tracked in snapshot.
but it is covered in timer-based check. Then new hight is calculated and applied.
What problem are you solving that prompted this PR?

@kir4ik
Copy link
Author

kir4ik commented Jul 6, 2022

If child component is updated, this is not supposed to be tracked in snapshot. but it is covered in timer-based check. Then new hight is calculated and applied. What problem are you solving that prompted this PR?

https://codepen.io/Kira4/pen/bGvEgRJ?editors=0010

This is a rough example, but still it seems to me that even this should work.
Just enter a character into the input and you will see that the height breaks. The animation end event, respectively, also cannot be processed because the height of the content is different from the height of the container

@nkbt
Copy link
Owner

nkbt commented Jul 6, 2022

I don't think it is a good example with timeout 0

More realistically it would be smth else like this
https://codepen.io/nkbt/pen/rNdxjoy?editors=0010

As long as timeout is not 0 - no problems anymore. Or if there is not timeout - same no problem.

@kir4ik
Copy link
Author

kir4ik commented Jul 6, 2022

I don't think it is a good example with timeout 0

More realistically it would be smth else like this https://codepen.io/nkbt/pen/rNdxjoy?editors=0010

As long as timeout is not 0 - no problems anymore. Or if there is not timeout - same no problem.

https://codepen.io/Kira4/pen/ExEPWKQ?editors=0010

It's not about the timeout. Just a promise that ended quickly

For example, there is a validator interface, the validation method is asynchronous, and it doesn’t matter how it is implemented, there may be a request to the server, there may not be any requests and immediately validate on the client

@nkbt
Copy link
Owner

nkbt commented Jul 8, 2022

Yeah makes sense. Need to check all the edge-cases with this update. There certainly was a reason to put logic in snapshot before update, but I do not remember why anymore.

@yehhork
Copy link

yehhork commented May 10, 2023

Hello!
Thank you for a such great lib!

Unfortunately, I am experiencing the same problem so I would appreciate any updates on this.

Do you mind moving this PR forward?

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.

3 participants