-
Notifications
You must be signed in to change notification settings - Fork 331
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
feat(selected): Adding context about original event triggering an autocomplete:selected
event
#267
Conversation
autocomplete:selected
event
9335d10
to
b75f4e8
Compare
@s-pace @Jerska @Haroenv @rayrutjes I added you as reviewer because we either discussed the issue together or you were interested in such a feature. I don't need review from all 4 of you, but the more the merrier :) |
could this be a breaking change? |
I don't think so. It's only adding a third parameter to the triggered event, so no change in userland; old event bindings will still work. |
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.
has a test + not breaking = LGTM
This is great @pixelastic 👏 |
@rayrutjes A concrete example of how it will help us is that we no longer have an issue to redirect user from the DocSearch UI. The issue was that we were using handleSelect that was handling both:
We were missing:
We wrap hits element in an tag. This solves the two point raised but the redirection done via the keyboard is not longer working. This is why we need this PR in order to handle the user's interaction in a different way and keep backward compatibility. |
Thanks for your comment @s-pace . I was more wondering if we could showcase a simple example narrowed down to the |
thanks @rayrutjes We will need to do a new release of our DocSearch UI to make it works. Might be a good way to promote this but definitely agree that it should be part of a simple "how to" |
@s-pace Let me know when we want to release a new Autocomplete.js version. Then I can update the Autocomplete.js sandbox template so that you can demo the new feature there. |
Sure we will |
@francoischalifour I tried to release autocomplete but got an error when running @rayrutjes I was planning on writing the relevant documentation once the new version is released. Basically, you will now be able to use a |
This adds a third parameter to the
autocomplete:selected
event, an object that contain information about the context in which it was called. It passed a key namedselectionMethod
that can take any value out ofclick
,enterKey
,tabKey
orblur
.Using this, we should now be able to detect if the event is coming from a click or not in userland. If it's coming from an click (and the template is a link), we could "do nothing" and fallback to the browser behavior, while still doing our own thing for all the other events.