-
Notifications
You must be signed in to change notification settings - Fork 64
Enabling Transitions: always dispatch the toggle event #61
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
Conversation
src/autocomplete.mjs
Outdated
this.element.setAttribute("aria-expanded", "true") | ||
this.element.dispatchEvent( | ||
new CustomEvent("toggle", { | ||
detail: { input: this.input, results: this.results } | ||
detail: { action: 'open', input: this.input, results: this.results } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I'm missing something, this.input
and this.results
are undefined 100% of the time... and it looks like it was always this way.
If the intention was for these to be this.inputTarget
and this.resultsTarget
, we could change those. But I wanted to keep my PR diff as small as possible :).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair. You have my blessing in broadly fix external events since you're looking at them/care about them. This PR or another is good by me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful then :). I've updated these to be the two targets, which I think could be useful :).
I don't have an ETA of when I will review this. Will try to do it this week. |
Thanks @drnic! To make it (hopefully) MUCH easier for you (I know getting a simple test project setup can be annoying... especially to merge someone else's feature), I've created a SUPER simple project you can use to test: https://github.com/weaverryan/stimulus-autocomplete-events In 5 commands (all in the README) you can test my PR in a real project and also see how an example usage performs (where a custom controller listens to the Please let me know if there is anything else I can do to make your job easier :). I'm hoping to feature this (including the transitions) in a screencast over on SymfonyCasts.com. Thanks! |
Hi! A friendly ping... and sorry to be annoying :/. The reason is that I'm hoping to show this library in the last video of a tutorial that's being released this week and next. I created a repository that will hopefully make testing this as painless as possible :) https://github.com/weaverryan/stimulus-autocomplete-events Thank you! |
Sorry i've been busy/away. Thanks for trying to make it easier for me; I'll
try to look soon.
|
Love this PR |
Would love to see this merged. |
Thanks for reminding me. Merged. |
Enabling Transitions: always dispatch the toggle event
Hi!
This closes #59.
I've created 3 clean commits to show the incremental progress - I have tried to change as little as possible:
The controller already has
open()
andclose()
methods, but they were not consistently called. The first commit always calls these methods when opening or closing the element. This means that thetoggle
event is now dispatched in open/close situations when it wasn't before, but I think this is a bug fix :). The docs state that thetoggle
event "fires when the results element is shown or hidden". I also added a newaction
key to the event set toopen
orclose
so that a listener can determine what's happening.Added a
this.isHidden
property to the controller to track the "is hidden" state, instead of relying onthis.resultsTarget.hidden
.Added a new
skipHiddenProperty
value that can be set if you want to avoidthis.resultsTarget.hidden
from being set (because you want to handle hiding/showing yourself).The end result of this is that a user can now leverage use-transition to add, for example, a fade-in/out for this auto-complete. I'll include that example code below in case anyone is interested :).
Thanks!
Transitions with stimulus-use
A) The HTML:
B) Custom autocomplete-transition controller:
C) Some fade out / fade in CSS