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

Fixed state not being reset to correct value when transitioning out #628

Merged

Conversation

dardalan
Copy link
Contributor

@dardalan dardalan commented Aug 19, 2021

This is already opened pr on this Issue #623
but i am not sure we should make any changes on this code ember-basic-dropdown, cause on production code - it works good, the only problem with tests, so i made a workaround for testing purpose. Any idea how to fix this issue better, very welcome.

The transitioning--in etc classes don't get reset after the first render. When the dropdown content first renders, it animates correctly, but on subsequent renders there is no animation - cause state not being reset to correct value.

Looks like it was broken here in this commit, so idea to revert back and cover with tests:
1819c01#diff-73297f8c71645544a2a6826e5c3146aff7cb2333d9c17fe968258e5f14dc4a79R164

package-lock.json Outdated Show resolved Hide resolved
@dardalan dardalan force-pushed the bugfix/reset-animation-state branch from 1434264 to f60a8fe Compare August 23, 2021 14:23
@gwak
Copy link
Contributor

gwak commented Dec 5, 2021

@dardalan @alex-konoval This PR seems to correctly fix the issue. Is there something missing before it can be merged ? Can I help ?

@alex-konoval
Copy link

@gwak we are using a fork with this fix for a few months and no issues reported yet.
@dardalan could you please rebase your branch?

@cibernox
Copy link
Owner

cibernox commented Dec 6, 2021

Sorry for neglecting this for so long. If you can rebase it and add an entry to the changelog I'll merge it today.

@dardalan
Copy link
Contributor Author

dardalan commented Dec 6, 2021

Sorry for neglecting this for so long. If you can rebase it and add an entry to the changelog I'll merge it today.

@cibernox no worries, will try to update it asap.

@dardalan
Copy link
Contributor Author

dardalan commented Dec 6, 2021

@cibernox Please check it out, when you have a time.

@dardalan
Copy link
Contributor Author

dardalan commented Dec 7, 2021

Sorry for neglecting this for so long. If you can rebase it and add an entry to the changelog I'll merge it today.

@cibernox can you check please and merge if all is good?

@dardalan
Copy link
Contributor Author

dardalan commented Dec 7, 2021

Sorry for neglecting this for so long. If you can rebase it and add an entry to the changelog I'll merge it today.

@cibernox can you check please and merge if all is good?

@cibernox can you trigger CI again please?

@cibernox cibernox merged commit 78b91ea into cibernox:master Dec 7, 2021
@cibernox
Copy link
Owner

cibernox commented Dec 7, 2021

Thanks! Merged!

@gwak
Copy link
Contributor

gwak commented Dec 7, 2021

Great! 🙏

@dardalan
Copy link
Contributor Author

dardalan commented Dec 7, 2021

@cibernox Thanks!

@krasnoukhov
Copy link
Contributor

@cibernox Would you cut a release? 🙌

@cibernox
Copy link
Owner

Released in 4.0.1

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.

5 participants