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

feat(ctrl-click): Enable Ctrl-Click on suggestions #542

Merged
merged 2 commits into from Nov 21, 2018
Merged

Conversation

pixelastic
Copy link
Contributor

@pixelastic pixelastic commented Nov 20, 2018

This updates the underlying autocomplete.js to a version that passes
the selection context (click or keyboard) to the
autocomplete:selected event.

With the default handleSelected, and in the case of a click, it will
do nothing (as each suggestion already being a <a href>, it will
already follow the link), but manually change the location for
keyboard navigation.

When a custom handleSelected is set though, default browser clicks
on suggestions are disabled and only handleSelected is called. This
is to keep backward compatibility with all the live websites that
might already use handleSelected.

This updates the underlying autocomplete.js to a version that passes
the selection context (click or keyboard) to the
`autocomplete:selected` event.

With the default `handleSelected, and in the case of a click, it will
do nothing (as each suggestion already being a `<a href>`, it will
already follow the link), but manually change the location for
keyboard navigation.

When a custom `handleSelected` is set though, default browser clicks
on suggestions are disabled and only `handleSelected` is called. This
is to keep backward compatibility with all the live websites that
might already use `handleSelected`.
Copy link

@s-pace s-pace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I haven't run it but looks to do what we need

- `suggestion`, the object representing the current selection. Contains a `.url`
key representing the destination.

- `datasetNumber`, this should always equal `1` as DocSearch is searching
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO we should not document this input if it is not useful with our UI which is already tight with one index only.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is that it's the 3rd parameter out of 4, that was already there before. If I don't document it, it make it look like there are only 3 parameters. And we need to keep it to not break backward compatibility, so I unfortunately have to keep it that way :/

@s-pace
Copy link

s-pace commented Nov 20, 2018

@pixelastic can you release it with a new version please?

@Haroenv Haroenv removed their request for review November 21, 2018 08:22
@pixelastic pixelastic merged commit a470c48 into master Nov 21, 2018
@pixelastic pixelastic deleted the fix/ctrl-click branch November 21, 2018 14:47
pixelastic added a commit that referenced this pull request Nov 22, 2018
### Features

* **ctrl-click:** Enable Ctrl-Click on suggestions ([#542](#542)) ([a470c48](a470c48))
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

2 participants