Skip to content

apply onRest style when work was started #301

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

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

Conversation

palortoff
Copy link

When Collapse is opened, then closed quickly before animation has started (height is still 0), rest will not apply the final closed style leaving the component in onWork style for open.
This results in the Collapse displayed as open even though it should be closed.

Similar behavior in reverse case.

By removing the hasOpened / hasClosed check in onRest we assure that the final style is always applied.

@nkbt
Copy link
Owner

nkbt commented Jun 8, 2021

Checks for hasOpened and hasOpened are necessary. There are cases when component is added, removed or updated dynamically on the page, or when we do server-side rendering. We do not want component to start expanding on the initial load, or collapsing after load. And so on. Every single bit in that code came out of need and cannot be easily removed like that.

Have you checked every single case from example page (I wish I had time to put e2e UI tests for it...)?

PS: I do not remember exact reason why I put those checks in place (it was few years ago after all), but there was a serious reason to do so. Every refactor I've started from the simplest possible solution and made sure it covers all the necessary cases (see above). At some iteration I definitely had the code as simple as you suggest, but it was not working in some situations.

@palortoff palortoff changed the title force rest even when animation appears to be done apply onRest style when work was started Jun 8, 2021
@palortoff
Copy link
Author

Changed the PR to not skip the check but ensure that the onRest style will be applied after the onWork style had been applied.

This should not trigger onRest when not wanted, but force it after onWork ran.

@palortoff
Copy link
Author

@nkbt : any news on this?

@nkbt
Copy link
Owner

nkbt commented Jun 28, 2021

My life is a bit hectic right now, didn't have a chance to look. Sorry about that. Would be much easier if I had e2e ui tests but you never have enough time for opensource :(

@AlexandrPristupa
Copy link

@palortoff I have the same problem )

@palortoff
Copy link
Author

@nkbt : sorry to bother you again.
Is there any chance for this PR to be merged anytime soon?

I need it for a project that is about to be released and I would like to avoid using a fork of react-collapse

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