stimulus-components / stimulus-reveal-controller

A Stimulus controller to toggle a class on one or multiple items to show or hide them.
https://www.stimulus-components.com/docs/stimulus-reveal-controller/
27 stars 5 forks source link

Added transition animation support #3

Closed blafri closed 1 year ago

blafri commented 3 years ago

You can now have transition effects when revealing/hiding items by adding data attributes to the item telling the controller what classes to add to the item for the transition to happen. Got the information about how to do this from the great articles and repos linked below so all credit to the authors for their great work.

Sebastian De Deyne https://sebastiandedeyne.com/javascript-framework-diet/enter-leave-transitions/

Mike McCall article on dev.to https://dev.to/mmccall10/tailwind-enter-leave-transition-effects-with-stimulus-js-5hl7

The el-transition package by Mike McCall https://github.com/mmccall10/el-transition

And a pull request from tomclose on the el-transition repo to fix an issue with original classes being overwritten https://github.com/mmccall10/el-transition/pull/4

guillaumebriday commented 3 years ago

Hey! Thanks for the awesome work 🙏

At this point I wanted to use https://github.com/stimulus-use/stimulus-use/blob/master/docs/use-transition.md when it will be released. Because I don't want to add this complex work in the codebase. And I will use it in other packages.

Do you think you can adapt your PR to make it work with stimulus-use? Thanks @blafri

blafri commented 3 years ago

Hey @guillaumebriday I would like to try to get it to work with stimulus-use!!

I took a look at use-transition in the stimulus-use package but, unless I am missing something, it seems that right now they only support having a single target for the transition.

If you take a look at this line https://github.com/stimulus-use/stimulus-use/blob/c2e956cff6369dec3ffed7facc8a3ada17d4fd82/src/use-transition/use-transition.ts#L41 you will see where they get the target for the transition and it does not seem to allow multiple targets.

In your reveal controller you allow supporting multiple items to show/hide/toggle on each show()/hide()/toggle() call. How would you like to handle this in your controller?

adrienpoly commented 3 years ago

Hey

Just to let you know that I finally got time to update useTransition, mostly fixing a few bugs.

As you said currently only one target can be specified. This is really something that I had in mind to support multiple targets.

I need to make the tests suite a bit more exhaustive and will do a refactor to handle properly multiple targets. If it is ok with you I ll ping you here once it is available and it would be great to widen the tests. It is clearly a subject will lots of small edge cases.

blafri commented 3 years ago

@adrienpoly thats sounds great. Looking forward to trying this out. Thanks for all of your great work guys!!

guillaumebriday commented 3 years ago

@blafri Maybe you could update your PR with stimulus-use/use-transition? 🙏

blafri commented 3 years ago

Hey @guillaumebriday, I updated the pull request to use stimulus-use/use-transition as requested. I used the beta version of the stimulus-use package (0.24.0-1).

The problem with this implementation is that it does not support multiple targets like the previous version did, as currently the stimulus-use package only supports a single target to transition.

Let me know how you want to handle this as this change may break peoples code that have multiple targets setup for the controller. I added section in the index.html with multiple targets so you can see what happens if multiple targets are defined.

guillaumebriday commented 3 years ago

Thanks a lot @blafri 🙏

Will review it this week!

iainbeeston commented 1 year ago

Is there any chance this could get merged? Transitions would be useful