Skip to content
This repository was archived by the owner on Jul 19, 2019. It is now read-only.

Trigger event on enter #25 #83

Closed

Conversation

emilianosanti
Copy link

The Autocomplete component will receive a function as a propType that will be called when the following events occurs.

  • The user enter an element that not exists in the Autocomplete source list.
  • The user press enter key.

The function receives a parameter which is the value entered on the input. Thus, who instantiate the Autocomplete component could make any action with that value. For this specific issue, we need to make a service call with this value.

By implementing this improvement with this approach we reach the next benefits:

  • This feature is totally optional, in the case we wont add this functionality the component will have the default behavior.
  • According to the issue description, we need to make a service call, but if the business rules change (for instance, show an error message highlighting that the search doesn't match with the list) we only need to change the implementation of the function we receive by propType, so the component is totally flexible and adaptable.

@@ -10,6 +10,7 @@ let Autocomplete = React.createClass({
initialValue: React.PropTypes.any,
onChange: React.PropTypes.func,
onSelect: React.PropTypes.func,
onSearch: React.PropTypes.func,
Copy link
Author

Choose a reason for hiding this comment

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

Feel free to suggest another name for this props.

@CMTegner
Copy link
Collaborator

What would you do as a result of onSearch()? Fetch more items? Perhaps this functionality would be better suited in a onSubmit() (form) handler.

@emilianosanti
Copy link
Author

What would you do as a result of onSearch()? Fetch more items?

You can do whatever you need inside onSearch function. For instance, it's a improvement referenced to the issue #25, in this function you can submit queries that aren't in the autocomplete options..
I think onSubmit() name would it be better.. I will update that

@loverajoel
Copy link

  • Keep the focus of the PR (add an event after Enter key press), if you think that examples requires a refactor please send another PR for that and don't add noise to this PR. cc @emilianosanti
  • I think that this event can be useful in order to set the value from outside the module. ex:
onSearch => (currentValue, setValueCallback) {
    setValueCallback(currentValue.split(',').reverse().join());
}

@emilianosanti
Copy link
Author

Keep the focus of the PR

You're right with that, I'll revert the commit 👍

Regarding to the event, it's a great idea, I'll update

@CMTegner
Copy link
Collaborator

Let's take a step back and review this proposal:

  1. Add an API that lets you provide a callback which should be invoked when the user hits return in the input when no suggestion is highlighted.
  2. Provide said callback with a way of updating state.value.

I'd argue that the first point is the responsibility of the surrounding form, not the input element. Remember, there's already a default behaviour attached to hitting return in an input element, which is to submit the form (naming the callback onSubmit makes things even more confusing). Forcing your users to learn a new behaviour for one specific input field probably won't lead to great UX.

As for the second point, there's already work in progress (#65) on removing state.value and making Autocomplete align with how value is treated in regular <input> elements (i.e. you control it 100% by passing it as a prop).

I still don't understand the need for this API. No one has offered an example where this proposed callback would be any better than simply using onChange. Perhaps I'm missing some important piece of insight into this problem area?

@emilianosanti
Copy link
Author

I agree with several thing you mentioned, this new API has emerged from an issue requested by cleercode (#25). Do you really think we need this functionality?. I have in mind to decline this pull request

@CMTegner
Copy link
Collaborator

CMTegner commented Apr 4, 2016

I honestly don't think it's needed, but I'm not the correct person to argue for its existence since I don't know the problem well enough.

@garrettmaring
Copy link
Contributor

garrettmaring commented May 20, 2016

I don't see how this is easily implemented outside of the library. To restate the goal, there are two desired actions:

  1. "enter" when an item is highlighted -> trigger onSelect function
  2. "enter" when no item is highlighted -> trigger some other function

@CMTegner you mentioned that it is the job of the surronding form, suggesting that should be wrapped in a form? But that along with other options (assigning a listener to the search event if making input type search, or listening for an enter key press) are difficult because the focus remains on the input field while using the arrow keys to navigate through the suggested results.

How can we tell whether any item is currently highlighted? Or how can we watch for an event that will only fire if no item is highlighted given that "onSubmit" events will fire with or without something selected?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants