Remove react-virtualized from ComboBox#3173
Remove react-virtualized from ComboBox#3173myasonik merged 8 commits intoelastic:feature/selectable-a11yfrom
Conversation
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3173/ |
|
This change was requested by @chandlerprall but anyone can review |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3173/ |
thompsongl
left a comment
There was a problem hiding this comment.
A couple initial things:
-
Can we remove
react-virtualizedfrom our deps now? -
In the "Virtualized" EuiComboBox example, when I select an option the list is scrolled to the top. Previously, scroll position was maintained after making selections.
src/components/combo_box/combo_box_options_list/combo_box_options_list.tsx
Outdated
Show resolved
Hide resolved
chandlerprall
left a comment
There was a problem hiding this comment.
There's a regression in functionality for the Groups example: clicking on a group label in the dropdown closes the list; compare with the published docs where nothing happens when clicking a group label.
src/components/combo_box/combo_box_options_list/combo_box_options_list.tsx
Outdated
Show resolved
Hide resolved
src/components/combo_box/combo_box_options_list/combo_box_options_list.tsx
Outdated
Show resolved
Hide resolved
386b61e to
3c0c49e
Compare
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3173/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3173/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3173/ |
chandlerprall
left a comment
There was a problem hiding this comment.
Changes LGTM! Tested in the PR's docs build and couldn't hit a snag.
chandlerprall
left a comment
There was a problem hiding this comment.
Meant to hit the Approve button
Summary
Removing
react-virtualizedin favor ofreact-window.EuiSelectableandEuiComboBoxwere the only two consumers ofreact-virtualized. WithEuiSelectablemoving toreact-window, this made sense to do to drop EUI's footprint.react-windowis smaller, faster, and has better accessibility so it's pretty much a win on all fronts.Did the smallest amount of changes to make this happen but they do handle things like scrolling and item rendering a bit differently so it required a bit of code.
Checklist
- [ ] Check against all themes for compatibility in both light and dark modes- [ ] Checked in mobile- [ ] Checked in IE11- [ ] Props have proper autodocs- [ ] Added documentation examples- [ ] Checked for accessibility including keyboard-only and screenreader modes- [ ] A changelog entry exists and is marked appropriately