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

Add onChange callback to search config object. #469

Merged
merged 2 commits into from
Mar 7, 2018

Conversation

cjcenizal
Copy link
Contributor

I added an example in this PR in which the input handler is debounced so that the search is executed once you're done typing, instead of executed as you type. I'd like to improve this UX so that we can bypass the debounce and execute the search immediately if the user hits ENTER.

I think I'll implement this in a separate PR, in which we change EuiFieldSearch to accept an onChange prop (called as you type) and an onSearch prop (called when you hit enter).

@cjcenizal cjcenizal requested review from uboness and nreese March 5, 2018 17:47
@cjcenizal
Copy link
Contributor Author

Fixes #446

@@ -112,6 +113,10 @@ export class EuiInMemoryTable extends React.Component {
}

onQueryChange(query) {
if (this.props.search.onChange) {
return this.props.search.onChange(query);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this should return. There are cases where the consumer will want the table to filter the results like normal. This is just a callback that gives the consumer the option to fetch new results if needed.

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

lgtm

@cjcenizal cjcenizal merged commit ef3738f into elastic:master Mar 7, 2018
@cjcenizal cjcenizal deleted the in-memory-table-search-callback branch March 7, 2018 23:09
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.

2 participants