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

Reinstate combobox onblur pill creation in #1353 and fix bug in initial implementation #1364

Merged
merged 5 commits into from
Dec 11, 2018

Conversation

cjcenizal
Copy link
Contributor

combo-box-fix

There were actually two bugs in the original bug report from Kibana, one on the EUI side and one on the Kibana side.

The EUI bug was caused by the internal onBlur callback also being called when the combo box is focused. I'm not sure why this is the case, but we can work around it by ensuring the input doesn't have focus before trying to create an option. I've added a test which would have caught this the first time around.

On the Kibana side, a bug exists in which empty string input isn't being validated and ignored correctly. I suspect this might be a common mistake when using the combo box, so I've added a note to check for this in the release notes. CC @elastic/kibana-security.

@chandlerprall
Copy link
Contributor

I would think EuiComboBox shouldn't call onCreateOption() at all if the input value is empty (I thought this was already the case).

@cjcenizal
Copy link
Contributor Author

Good idea @chandlerprall. I've made the change, could you please take another look?

Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Changes LGTM; pulled locally and played with the combobox docs

@cjcenizal cjcenizal merged commit ee9da6c into elastic:master Dec 11, 2018
@cjcenizal cjcenizal deleted the combo-box-blur-fix branch December 11, 2018 23:14
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