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

move on-suggested to an event #58

Closed
Haroenv opened this issue Sep 12, 2018 · 1 comment
Closed

move on-suggested to an event #58

Haroenv opened this issue Sep 12, 2018 · 1 comment

Comments

@Haroenv
Copy link
Contributor

Haroenv commented Sep 12, 2018

  • vue-autosuggest version: 1.5.0

Relevant code or config

<vue-autosuggest
  :suggestions="indicesToSuggestions(indices)"
  :on-selected="selectHandler"
  :input-props="{
    style: 'width: 100%',
    onInputChange: refine,
  }"
/>

Problem description:

_onSelected seems to be a fine abstraction, but it's more of a React pattern than a Vue pattern. For that I'd change that to this.$emit('selected', ...data). I'd do a PR but wasn't completely sure what the code all meant :)

Suggested solution:

change to an event for "selected" (with backwards compatibility, like for the other events)

<vue-autosuggest
  :suggestions="indicesToSuggestions(indices)"
  @selected="selectHandler"
  :input-props="{
    style: 'width: 100%',
    onInputChange: refine,
  }"
/>
@darrenjennings
Copy link
Owner

I had originally left off custom listeners because I usually see events passed as callback props, and @ left for native DOM events. I also borrowed much of my inspiration from react-autosuggest. However, I like this API, as it makes the component easier to tell which props are events. Should be an easy fix with backwards compatibility as you say. Thanks!

darrenjennings added a commit that referenced this issue Oct 3, 2018
- chore(eslint) add eslint vue/recommended configuration and have
autosuggest lib conform
- chore(rollup) uglify esm module for smaller lib size
- chore(storybook) add story for suggestion event
- test(events) add tests for suggestion event/deprecation warnings

Fixes #58 From #62
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants