[Discover] [Unified Data Table] Conditionally use overscanRowCount when Document column isn't visible
#192332
Conversation
This file contains 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
davismcphee
added
release_note:skip
|
/ci |
davismcphee
force-pushed
the
discover-conditional-row-overscan
branch
from
6a2e18a
to
3bb08e0
Compare
|
/ci |
|
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Async chunks
To update your PR or re-run it, just comment with: cc @davismcphee |
jughosta
approved these changes
Sep 9, 2024
1 task
gergoabraham
pushed a commit
to gergoabraham/kibana
that referenced
this pull request
Sep 13, 2024
…when Document column isn't visible (elastic#192332) ## Summary Part of elastic#191249 included using the `overscanRowCount` property of EUI data grid to render some additional data grid rows off screen, which greatly reduces pop in when scrolling through the grid. While the performance cost of this low in most cases, it has an impact in certain situations such as when documents have many fields and the Document column is visible (e.g. Discover's `many_fields` performance journey). This is because the Document column can render _many_ DOM elements in each of its cells, which EUI data grid struggles to handle. This PR updates Unified Data Table to only use `overscanRowCount` when the Document column isn't visible, so we'll still benefit from the scrolling improvements when columns are selected, but won't take the performance hit for the Document column. Perf journey run: https://buildkite.com/elastic/kibana-single-user-performance/builds/14355. <img width="842" alt="Screenshot 2024-09-09 at 14 07 46" src="https://github.com/user-attachments/assets/b074fd7a-ef8a-4917-ac5e-0675b0e60b7d"> ### Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Part of #191249 included using the
overscanRowCountproperty of EUI data grid to render some additional data grid rows off screen, which greatly reduces pop in when scrolling through the grid. While the performance cost of this low in most cases, it has an impact in certain situations such as when documents have many fields and the Document column is visible (e.g. Discover'smany_fieldsperformance journey). This is because the Document column can render many DOM elements in each of its cells, which EUI data grid struggles to handle.This PR updates Unified Data Table to only use
overscanRowCountwhen the Document column isn't visible, so we'll still benefit from the scrolling improvements when columns are selected, but won't take the performance hit for the Document column.Perf journey run: https://buildkite.com/elastic/kibana-single-user-performance/builds/14355.
Checklist
For maintainers