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

[Bug] Transition only applying first class #266

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Hugo-Le-Goff
Copy link

Hi !

I just discovered this repo and I was trying the dropdown controller. When I tried to change de transitions style, I realised that it was not working anymore and only applying the first class of the list.

So I'm proposing a quick fix for it, maybe there is others controllers to fix. It's late and I don't have time for this right now.

Thanks for your job and I hope my fix proposal is good.

@jaywhy
Copy link
Contributor

jaywhy commented Nov 16, 2024

@Hugo-Le-Goff looks good to me.

You're right. The singular version only returns the first class.

https://stimulus.hotwired.dev/reference/css-classes#properties

@Hugo-Le-Goff
Copy link
Author

Hugo-Le-Goff commented Nov 16, 2024

I updated the doc for the transition.js, with :

  • .join(' ')
  • added the "static classes" to be able to access the classes variables
  • changed the data classes in the html to end with "-class" which is mandatory to be included as class variable

I also updated the Dropdown docs, so users can be aware of this functionality.

Also, I'm wondering why this transition feature is natively applied to Dropdown only and not to others components. I don't know if it is relevant for others, I'm still discovering tailwind, stimulus and this repo.

Edit : In the beforeCache function I replaced "hidden" by the transitionOptions.toggleClass value, I thought it was more elegant.

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.

2 participants