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

Allow overriding of 'hidden' #16

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

Conversation

Tonksthebear
Copy link

I'm not sure if this adds too much complexity to the library, but, for my uses, I didn't want a 'hidden' class added to the end of every leave transition. This would allow the library to be used for simple animations that don't result in visibility changes. I went through a couple iterations with the goal of preserving the same invocation syntax, except now it checks for two additional data attributes.

This PR adds data-transition-enter-final and data-transition-leave-final. These classes get added at the end of their respective directions, and get removed at the beginning of their opposition directions. It preserves the hidden default by adding a default data-transition-leave-final attribute of hidden.

enter() and leave() now track their last status by settings a data-transitioned attribute, rather than the presence of css classes. With the presence of potential multiple transition-leave-final classes, checking the transitioned status would be complicated. I thought leveraging another data attribute would be more precise and consistent.

I understand if this is more complicated than you want to make the library, but I wanted to at least make a PR in case you found it valuable to add this extra flexibility. Javascript is not my primary language so also completely happy to make syntax changes if you like the functionality but not the look!

This allows overriding of 'hidden' class
@Tonksthebear
Copy link
Author

@mmccall10 is this something you would consider? I'm about to pull my fork into another project and it reminded me of this PR so I figured I'd see if there were any updates

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.

None yet

1 participant