Prevent toggling loading on tables from re-mounting visible rows#2754
Merged
chandlerprall merged 4 commits intoelastic:masterfrom Jan 13, 2020
Merged
Conversation
…revent re-rendering the whole table when loading is set to true
thompsongl
approved these changes
Jan 13, 2020
Contributor
|
Also, the issue tagged this as urgent for 7.6. Are we looking to get this in with the (likely) backport release coming later this week? |
Contributor
Yes. This was at request from @bmcconaghy and @mikecote. Their UI won't be active for 7.6 but I know they wanted this PR so they didn't need to comment out a bunch of tests. |
This reverts commit 7942eee.
Contributor
Author
|
Example reverted, changelog added. Will merge on green. |
thompsongl
added a commit
that referenced
this pull request
Jan 13, 2020
This was referenced Jan 14, 2020
|
Thanks for getting this done! Sorry I couldn't review as I was out on vacation. This solves the issue on our end 🙏 👍 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2612
Summary
Removed
EuiLoadingTableBody& moved its functionality intoEuiBasicTable. This prevents React from un- and re-mounting visible table rows and their components when theloadingprop changes, as the virtual dom structure remains the same.This also fixes a memory leak where the event listeners added which prevent interactions on a
loadingtable were never removed.For testing, I've modified the first table example (A simple BasicTable) to have a popover on the Nationality column. A
setIsLoadingfunction is added to thewindowobject for toggling the table's state - execute it through the developer console, passingtrueorfalse. I'll revert the changes to this example before merging.Checklist
- [ ] Check against all themes for compatibility in both light and dark modes- [ ] Checked in mobile- [ ] Checked in IE11 and Firefox- [ ] Props have proper autodocs- [ ] Added documentation examples- [ ] Checked for accessibility including keyboard-only and screenreader modes