[EuiSelectable] Clear Searchbar Bug Fix #6317

breehall · 2022-10-18T20:51:56Z

Summary

This PR resolves a bug in EuiSelectable that caused the options list to remain in a filtered state after the search bar contents have been removed. This came about as a consume created a function to clear the search bar with an external function. After clearing the contents of the bar, the options list did not reappear.
This was caused by a race condition inside of EuiSelectable that checked tried to match the previous contents of the search bar with options in the list instead of reloading the list (because the search bar was empty).

Previous Behavior
When clicking on the "no results found <p>, the search bar is cleared by setting searchProps.value to ''(empty string). When the search bar contents contain an empty string, the full list should reappear.

ED48BD12-FFB9-48E4-89B4-0C3746DA2CF3.mov

Expected Behavior
Setting searchProps.value to '' (empty string) returns the full list

98023640-5F53-4342-8EA7-9856ED8B20DC.mov

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

~~[ ] Checked in both light and dark modes~~
[ ] Checked in mobile
Checked in Chrome, Safari, Edge, and Firefox
[ ] Props have proper autodocs and playground toggles
[ ] Added documentation
~~[ ] Checked Code Sandbox works for any docs examples~~
Added or updated jest and cypress tests
Checked for breaking changes and labeled appropriately
~~[ ] Checked for accessibility including keyboard-only and screenreader modes~~
~~[ ] Updated the Figma library counterpart~~
A changelog entry exists and is marked appropriately

…t from reappearing when the search bar was alter or emptied via an external function. Added a new test case to catch this instance

kibanamachine · 2022-10-18T21:21:51Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6317/

cee-chen · 2022-10-24T17:51:42Z

src/components/selectable/selectable.spec.tsx

+          expect(cy.get('p')).to.exist;
+        });
+
+      cy.get('input').clear();


I'm not convinced this is a valid regression test of the actual bug/behavior because this already works in production. From the Slack convo describing the bug, this has nothing to do with clearing the search bar via browser/UI, this bug occurs when the consuming application is controlling the input via searchProps.value and sets it to null/undefined.

To correctly reproduce this buggy/broken behavior, I'm guessing we would need to write the test for that in Jest rather than Cypress.

cee-chen · 2022-10-24T17:55:53Z

src/components/selectable/selectable.tsx

    const stateUpdate: Partial<EuiSelectableState<T>> = {
-      visibleOptions: matchingOptions,
+      searchValue,
      activeOptionIndex,
    };

+    if (searchProps?.value != null && searchProps.value !== searchValue) {
+      stateUpdate.searchValue = searchProps.value;
+    }
+
+    stateUpdate.visibleOptions = getMatchingOptions<T>(
+      options,
+      stateUpdate.searchValue ?? '',
+      isPreFiltered
+    );
+
    if (
      activeOptionIndex != null &&
-      activeOptionIndex >= matchingOptions.length
+      activeOptionIndex >= stateUpdate.visibleOptions.length


LGTM / no issues with the code order of operations change/moves - my main blocker with this PR is that I don't see a valid reproduction of the bug that this is fixing to QA

upcoming_changelogs/6317.md

…aused search results not to appear when the search bar value is controlled by searchProps.value. Removed Cypress test in lieu of a more accurate Jest test.

…able-clear-searchbar Pulling code from main

Co-authored-by: Constance <constancecchen@users.noreply.github.com>

…eehall/eui into euiSelectable-clear-searchbar Pulling commits made in Github UI

cee-chen

Awesome job!! This test looks great and I can confirm it fails as expected on main/prod but passes in this branch 🎉

kibanamachine · 2022-11-02T17:24:11Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6317/

## Summary `eui@67.1.8` ⏩ `eui@70.2.2` ⚠️ Note: This upgrade contains breaking changes to `EuiFlexGroup` and `EuiFlexGrid`, primarily around switching margins and negative margins to `gap`. Please do a quick QA pass of your app to scan for any issues. We're happy to help resolve minor fixes, or potentially follow up after PR merges. You can find us over in #eui! ## [`70.2.4`](https://github.com/elastic/eui/tree/v70.2.4) **Bug fixes** - Fixed visual bug in nested `EuiFlexGroup`s, where the parent `EuiFlexGroup` is responsive but a child `EuiFlexGroup` is not ([#6381](elastic/eui#6381)) ## [`70.2.3`](https://github.com/elastic/eui/tree/v70.2.3) **Bug fixes** - Fixed incorrect margins in `EuiSuperDatePicker` caused by `EuiFlex` CSS gap change ([#6380](elastic/eui#6380)) ## [`70.2.2`](https://github.com/elastic/eui/tree/v70.2.2) - `EuiButton` now accepts `minWidth={false}` ([#6373](elastic/eui#6373)) **Bug fixes** - `EuiButton` no longer outputs unnecessary inline styles for `minWidth={0}` or `minWidth={false}` ([#6373](elastic/eui#6373)) - `EuiFacetButton` no longer reports type issues when passing props accepted by `EuiButton` ([#6373](elastic/eui#6373)) - Fixed the shadow sizes of `.eui-yScrollWithShadows` and `.eui-xScrollWithShadows` ([#6374](elastic/eui#6374)) ## [`70.2.1`](https://github.com/elastic/eui/tree/v70.2.1) **Bug fixes** - Re-fixed `EuiPageSection` not correctly merging `contentProps.css` ([#6365](elastic/eui#6365)) - Fixed `EuiTab` not defaulting to size `m` ([#6366](elastic/eui#6366)) ## [`70.2.0`](https://github.com/elastic/eui/tree/v70.2.0) - Added a keyboard shortcuts popover to `EuiDataGrid`'s toolbar. This can be visually hidden via `toolbarVisibility.showKeyboardShortcuts`, but will always remain accessible to keyboard and screen reader users. ([#6036](elastic/eui#6036)) - `EuiScreenReaderOnly`'s `showOnFocus` prop now also shows on focus within its children ([#6036](elastic/eui#6036)) - Added `onFocus` prop callback to `EuiSuperDatePicker` ([#6320](elastic/eui#6320)) **Bug fixes** - Fixed `EuiSelectable` to ensure the full options list is re-displayed when the search bar is controlled and cleared using `searchProps.value` ([#6317](elastic/eui#6317)) - Fixed incorrect padding on `xl`-sized `EuiTabs` ([#6336](elastic/eui#6336)) - Fixed `EuiCard` not correctly merging `css` on its child `icon`s ([#6341](elastic/eui#6341)) - Fixed `EuiCheckableCard` not setting `css` on the correct DOM node ([#6341](elastic/eui#6341)) - Fixed a webkit rendering issue with `EuiModal`s containing `EuiBasicTable`s tall enough to scroll ([#6343](elastic/eui#6343)) - Fixed bug in `to_initials` that truncates custom initials ([#6346](elastic/eui#6346)) - Fix bug in `EuiCard` where layout breaks when `horizontal` and `selectable` are both passed ([#6348](elastic/eui#6348)) ## [`70.1.0`](https://github.com/elastic/eui/tree/v70.1.0) - Added the `hint` prop to the `<EuiSearchBar />`. This prop lets the consumer render a hint below the search bar that will be displayed on focus. ([#6319](elastic/eui#6319)) - Added the `hasDragDrop` prop to `EuiPopover`. Use this prop if your popover contains `EuiDragDropContext`. ([#6329](elastic/eui#6329)) **Bug fixes** - Fixed `EuiButton`'s cursor style when the button is disabled ([#6323](elastic/eui#6323)) - Fixed `EuiPageTemplate` not recognizing child `EuiPageSidebar`s/`EuiPageTemplate.Sidebar`s with `css` props ([#6324](elastic/eui#6324)) - Fixed `EuiBetaBadge` to always respect its `anchorProps` values, including when there is no tooltip content ([#6326](elastic/eui#6326)) - Temporarily patched `EuiModal` to not cause scroll-jumping issues on modal open ([#6327](elastic/eui#6327)) - Fixed buggy drag & drop behavior within `EuiDataGrid`'s columns & sorting toolbar popovers ([#6329](elastic/eui#6329)) - Fixed `EuiButton` not correctly passing `textProps` for children inside fragments or i18n components ([#6332](elastic/eui#6332)) - Fixed `EuiButton` not correctly respecting `minWidth={0}` ([#6332](elastic/eui#6332)) **CSS-in-JS conversions** - Converted `EuiTabs` to Emotion ([#6311](elastic/eui#6311)) ## [`70.0.0`](https://github.com/elastic/eui/tree/v70.0.0) - Added the `enabled` option to the `<EuiInMemoryTable />` `executeQueryOptions` prop. This option prevents the Query from being executed when controlled by the consumer. ([#6284](elastic/eui#6284)) **Bug fixes** - Fixed `EuiOverlayMask` to set a `[data-relative-to-header=above|below]` attribute to replace the `--aboveHeader` and `--belowHeader` classNames removed in its Emotion conversion ([#6289](elastic/eui#6289)) - Fixed `EuiHeader` CSS using removed `EuiOverlayMask` class modifiers ([#6293](elastic/eui#6293)) - Fixed `EuiToolTip` not respecting reduced motion preferences ([#6295](elastic/eui#6295)) - Fixed a bug with `EuiTour` where passing any `panelProps` would cause the beacon to disappear ([#6298](elastic/eui#6298)) **Breaking changes** - `@emotion/css` is now a required peer dependency, alongside `@emotion/react` ([#6288](elastic/eui#6288)) - `@emotion/cache` is no longer required peer dependency, although your project must still use it if setting custom cache/injection locations ([#6288](elastic/eui#6288)) **CSS-in-JS conversions** - Converted `EuiCode` and `EuiCodeBlock` to Emotion; Removed `euiCodeSyntaxTokens` Sass mixin and `$euiCodeBlockPaddingModifiers`; ([#6263](elastic/eui#6263)) - Converted `EuiResizableContainer` and `EuiResizablePanel` to Emotion ([#6287](elastic/eui#6287)) ## [`69.0.0`](https://github.com/elastic/eui/tree/v69.0.0) - Added support for `fullWidth` prop on EuiForm, which will be the default for all rows/controls within ([#6229](elastic/eui#6229)) - Added support for `onResizeStart` and `onResizeEnd` callbacks to `EuiResizableContainer` ([#6236](elastic/eui#6236)) - Added optional case sensitive option matching to `EuiComboBox` with the `isCaseSensitive` prop ([#6268](elastic/eui#6268)) - `EuiFlexItem` now supports `grow={0}` ([#6270](elastic/eui#6270)) - Added the `alignItems` prop to `EuiFlexGrid` ([#6281](elastic/eui#6281)) - Added `filter`, `filterExclude`, `filterIgnore`, `filterInclude`, `indexTemporary`, `infinity`, `sortAscending`, and `sortDescending` glyphs to `EuiIcon` ([#6282](elastic/eui#6282)) **Bug fixes** - Fixed `EuiTextProps` to show the `color` type option `inherit` as default ([#6267](elastic/eui#6267)) - `EuiFlexGroup` now correctly respects `gutterSize` when responsive ([#6270](elastic/eui#6270)) - Fixed the last breadcrumb in `EuiBreadcrumbs`'s `breadcrumbs` array not respecting `truncate` overrides ([#6280](elastic/eui#6280)) **Breaking changes** - `EuiFlexGrid` no longer supports `columns={0}`. Use `EuiFlexGroup` instead for normal flex display ([#6270](elastic/eui#6270)) - `EuiFlexGrid` now uses modern `display: grid` CSS ([#6270](elastic/eui#6270)) - `EuiFlexGroup`, `EuiFlexGrid`, and `EuiFlexItem` now use modern `gap` CSS instead of margins and negative margins ([#6270](elastic/eui#6270)) - `EuiFlexGroup` no longer applies responsive styles to `column` or `columnReverse` directions ([#6270](elastic/eui#6270)) **CSS-in-JS conversions** - Converted `EuiFlexGroup`, `EuiFlexGrid`, and `EuiFlexItem` to Emotion ([#6270](elastic/eui#6270)) ## [`68.0.0`](https://github.com/elastic/eui/tree/v68.0.0) - Added `beta` glyph to `EuiIcon` ([#6250](elastic/eui#6250)) - Added `launch` and `spaces` glyphs to `EuiIcon` ([#6260](elastic/eui#6260)) - Added the `fallbackDestination` prop to `EuiSkipLink`, which accepts a string of query selectors to fall back to if the `destinationId` does not have a valid target. Defaults to `main` ([#6261](elastic/eui#6261)) - `EuiSkipLink` is now always an `a` tag to ensure that it is always placed within screen reader link menus. ([#6261](elastic/eui#6261)) **Bug fixes** - Fixed `EuiSuperDatePicker` not correctly merging passed `className`s ([#6253](elastic/eui#6253)) - Fixed `EuiColorStops` not correctly merging in passed `data-test-subj`s, `style`s, or `...rest` ([#6255](elastic/eui#6255)) - Fixed `EuiResizablePanel` incorrectly passing `style` to the wrapper instead of the panel. Use `wrapperProps.style` to pass styles to the wrapper. ([#6255](elastic/eui#6255)) - Fixed custom `onClick`s passed to `EuiSkipLink` overriding `overrideLinkBehavior` ([#6261](elastic/eui#6261)) **Breaking changes** - Removed `inherit` and `ghost` color from `EuiListGroupItem` ([#6207](elastic/eui#6207)) - Changed default color to `text` instead of `inherit` ([#6207](elastic/eui#6207)) **CSS-in-JS conversions** - Converted `EuiListGroup` and `EuiListGroupItem` to Emotion; Removed `$euiListGroupGutterTypes`, `$euiListGroupItemColorTypes` and `$euiListGroupItemSizeTypes`; ([#6207](elastic/eui#6207)) - Converted `EuiBadgeGroup` to Emotion ([#6258](elastic/eui#6258)) - Converted `EuiBetaBadge` to Emotion ([#6258](elastic/eui#6258)) - Converted `EuiNotificationBadge` to Emotion ([#6258](elastic/eui#6258)) Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

breehall added 2 commits October 18, 2022 16:34

Resolved a bug inside of EuiSelectable that prevented the options lis…

4f1fdfc

…t from reappearing when the search bar was alter or emptied via an external function. Added a new test case to catch this instance

Changelog

148e5a3

breehall marked this pull request as ready for review October 18, 2022 21:48

cee-chen reviewed Oct 24, 2022

View reviewed changes

upcoming_changelogs/6317.md Outdated Show resolved Hide resolved

breehall and others added 4 commits November 2, 2022 12:54

Created Jest test case to validate fix for EuiSelectable issue that c…

4c9983e

…aused search results not to appear when the search bar value is controlled by searchProps.value. Removed Cypress test in lieu of a more accurate Jest test.

Merge branch 'main' of https://github.com/breehall/eui into euiSelect…

6d60479

…able-clear-searchbar Pulling code from main

Update upcoming_changelogs/6317.md

e7044ea

Co-authored-by: Constance <constancecchen@users.noreply.github.com>

Merge branch 'euiSelectable-clear-searchbar' of https://github.com/br…

ff31963

…eehall/eui into euiSelectable-clear-searchbar Pulling commits made in Github UI

cee-chen approved these changes Nov 2, 2022

View reviewed changes

breehall merged commit 60e59d5 into elastic:main Nov 2, 2022

cee-chen mentioned this pull request Nov 8, 2022

Upgrade EUI to v70.2.4 elastic/kibana#144845

Merged

breehall deleted the euiSelectable-clear-searchbar branch September 18, 2023 19:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiSelectable] Clear Searchbar Bug Fix #6317

[EuiSelectable] Clear Searchbar Bug Fix #6317

breehall commented Oct 18, 2022 •

edited

Loading

kibanamachine commented Oct 18, 2022

cee-chen Oct 24, 2022

cee-chen Oct 24, 2022

cee-chen left a comment

kibanamachine commented Nov 2, 2022

[EuiSelectable] Clear Searchbar Bug Fix #6317

[EuiSelectable] Clear Searchbar Bug Fix #6317

Conversation

breehall commented Oct 18, 2022 • edited Loading

Summary

QA

General checklist

kibanamachine commented Oct 18, 2022

cee-chen Oct 24, 2022

Choose a reason for hiding this comment

cee-chen Oct 24, 2022

Choose a reason for hiding this comment

cee-chen left a comment

Choose a reason for hiding this comment

kibanamachine commented Nov 2, 2022

breehall commented Oct 18, 2022 •

edited

Loading