Added isClearable prop to EuiFieldSearch#2723
Conversation
cchaos
left a comment
There was a problem hiding this comment.
Code looks great and functionally works. You might want to consider the existence of this button in disabled and readOnly states. I would assume that if the input were in these states (user can't alter the contents) that they should also not be able to clear the contents.
I am also wondering if the default should be true instead of false. My consideration is that this being a search style of input, it usually means users may type in it several different times and it doesn't actually "save" the content. Removing (some) browser defaults of this clear button may be considered a regression.
@cchaos thanks for catching that! I disabled |
src/components/form/field_search/__snapshots__/field_search.test.js.snap
Show resolved
Hide resolved
| } | ||
|
|
||
| onClear = () => { | ||
| this.props.onChange(''); |
There was a problem hiding this comment.
This change does not respect the contract of the onChange handler that expects a React.ChangeEvent
It caused a regression bug on our side (elastic/kibana#54810), we can quickly fix it but it might have caused a regression on other consumers so it would be better to change it here. @andreadelrio
Summary
Adds
isClearableprop toEuiFieldSearch. Chrome and IE add a clear button (x) by default which I've hidden using CSS to avoid inconsistent behaviour whenisClearableis set to false.Fixes #1769
EuiFieldSearchwithisClearableset to trueChecklist
- [ ] Check against all themes for compatability in both light and dark modes- [ ] Checked for breaking changes and labeled appropriately- [ ] Checked for accessibility including keyboard-only and screenreader modes