[EuiDataGrid] Virtualization #4170

chandlerprall · 2020-10-22T15:50:48Z

Summary

( Fixes #4365 in 5328347 )

Added a new docs page, Data grid virtualization, to demonstrate the effect of virtualization & how to configure a grid's dimensions through props or DOM layout.

Need to write actual documentation for using/configuring the virtualization, everything else is ready for review.

Issues needing a fix:

Control column cells are too tall
Columns can have a gap between them
Unit tests are not happy with the virtualized setup
verify (and fix?) detected height resets after a new rowCount is provided (00ed696)
Header / row height needs to be calculated dynamically through an observer (8b4ed63 52798b3)
Double click issue on row header popover (5328347)
Footer cells are not getting widths (check version with styled footer border) (458d25f)
Cell widths are not updating on observable or on column width change from user

Items for follow up PRs:

[EuiDataGrid] Provide a way to highlight a row #4401 [EUIDataGrid] rowHover in gridStyle has no effect after 31.4.0 #4483 Highlight css has no class. Hovering a row, needs to hover sibling cells
[EuiDataGrid] Scroll cells into view via keyboard #4456 Cell keyboard navigation does not always scroll cells into view ([EuiDataGrid] Virtualization #4170 (review))
[EuiDataGrid] Unnecessary scroll bars when constrained #4457 When height is restrained it adds an unnecessary horizontal scrollbar ([EuiDataGrid] Virtualization #4170 (comment))
[EuiDataGrid] UX Cleanups #4458 UX cleanups ([EuiDataGrid] Virtualization #4170 (review))
[EuiDataGrid] a11y roles not valid (gridcell without row) #4474 role=gridcell not valid without role=row wrapper

Props

Added optional height and width props that accept valid CSS values.

Constrained layout

Checklist

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

…ed-datagrid

snide · 2020-12-02T16:55:43Z

Pushed some fixes. Still have the following todo:

need to figure out first/last columns in "row" so i can apply border styling
need to get height for header / footer from JS rather than a hard coded size
need to figure out why the width for the grid body cells is not updating
add a class to cell for striping

chandlerprall · 2021-01-21T19:57:47Z

flaky network, jenkins test this

kibanamachine · 2021-01-21T20:20:06Z

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

cchaos

Well, I accidentally loaded the old DataGrid and was testing that one, and it immediately kicked my Chrome up to 100% CPU... Now that I got this one running, my fans are quiet as a mouse. 🐭 So fantastic job there! 🎉

The only thing that I wish was a bit snappier is when changing any sort of client-side display of the datagrid like sorting, or changing of the rows per page, or the density (at a high number of rows). It's less that the grid itself takes a long time to make that change, but that the selection itself does. For instance the button group button doesn't show selected until the data grid has completely re-rendered. Not sure really what can be done there.

Also it's actually quite sluggish in Full Screen mode when scrolling 100 rows. This may have to do with the fact that it's no longer using the browser's page scroll but an internal overflow.

Some other things I noticed:

1. The density adjustments no longer seem to be changing the overall height of the cells.
There seems to be some calculations going that always come out to make the cells 34px high.

2. In full screen mode, the initial calculation doesn't take the scrollbar into account

3. Visually I think we need more borders especially around the scrollbars

4. It would be great if while scrolling with virtualization which shows a white background while the cells load, we could have either add in some light gray like $euiPageBackgroundColor, or even some sort of pattern indication "loading".

5. It looks like we also need a smarter pagination display on skinny screens/containers.

6. In full screen, the bottom row is still getting cutoff by the fixed pagination and it's hiding the horizontal scrollbar.

A few of these items could be quick follow ups. Just wasn't sure what is known.

cchaos · 2021-01-21T19:59:06Z

src-docs/src/views/datagrid/datagrid.js

@@ -191,7 +192,7 @@ const trailingControlColumns = [
            ownFocus={true}>
            <EuiPopoverTitle>Actions</EuiPopoverTitle>
            <div style={{ width: 150 }}>
-              <button onClick={() => {}} component="span">
+              <button onClick={() => {}}>


This has been bugging me a bunch in our docs. Why do we have a button (icon) within a <button> instead of just using a context menu?

There's also a whole bunch of console error because of the nested <button>s.

snide · 2021-01-21T21:04:33Z

@chandlerprall I can do a pass on some of that feedback. The only one I might skip @cchaos is the border one. Usually we have some sort of container in these cases that requires extra css (for example, discover). I think it's better to leave it more neutral and leave the css layer as additive based on the implementation.

cchaos · 2021-01-21T21:54:08Z

The only one I might skip @cchaos is the border one.

I still think it's important to add borders to the scrollbars or something. The white space is jarring when it just cuts off the content or looks like odd spacing between in and the pagination.

kibanamachine · 2021-01-26T18:27:17Z

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

chandlerprall · 2021-01-26T20:10:45Z

I'll let Dave take a pass at those items, but I'll add that performance is largely addressed by a PR Joe put up against my branch. It moves a number of things around which makes reviewing the virtualization effort harder, ~~so I've opted to keep it separate - though it is ready to follow this one quickly.~~ EDIT: that PR has been merged into this branch

Less wrapper elements

chandlerprall · 2021-01-26T21:28:11Z

Since the main virtualization changes have been reviewed, as has @flash1293's awesome performance changes, I've merged those additional optimizations in to ease testing & further review/conversation.

kibanamachine · 2021-01-26T21:48:31Z

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

kibanamachine · 2021-01-27T21:20:03Z

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

kibanamachine · 2021-01-28T17:06:28Z

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

thompsongl

This is great 🙌
Confirmed the upgrade path in Kibana (aside from unit tests)
The noted follow-up tasks cover the remaining functional issues

kibanamachine · 2021-01-28T19:32:49Z

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

kibanamachine · 2021-01-28T21:58:13Z

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

in-progress virtualization

b0d616f

chandlerprall force-pushed the feature/virtualized-datagrid branch from 787da9f to b0d616f Compare October 22, 2020 16:12

chandlerprall mentioned this pull request Oct 28, 2020

[EuiSwitch] Not re-rendering on toggle in EuiDataGrid #4175

Closed

chandlerprall added 2 commits November 2, 2020 10:54

Merge branch 'master' into feature/virtualized-datagrid

8125fe2

Fix leading/trailing column focus management

3795c28

myasonik mentioned this pull request Nov 3, 2020

[Data Grid] prevent off-page rendering #4217

Closed

chandlerprall added 7 commits November 3, 2020 09:08

Re-enabled datagrid header focus

d4060e5

Fix cell tabIndex enable/disabling

37c6146

Re-enabled initial grid focus when tabbing into it

70cec09

Merge branch 'master' into feature/virtualized-datagrid

3d2ac34

Merge branch 'master' into feature/virtualized-datagrid

89ee454

removed outdated code

639f3bb

restructure cell focus update calls to avoid render side effect

653a313

chandlerprall force-pushed the feature/virtualized-datagrid branch from 721cc29 to 653a313 Compare November 6, 2020 18:07

chandlerprall added 6 commits November 6, 2020 12:54

respond to changes in cell height

3889cc7

Merge branch 'master' into feature/virtualized-datagrid

d574ee3

Add optional height&width props to EuiDataGrid

9571bc0

Merge branch 'master' into feature/virtualized-datagrid

f588432

Respect height/width props and DOM constraints

e81c56d

updated snapshots

b61114a

chandlerprall requested review from snide and thompsongl November 12, 2020 21:02

chandlerprall marked this pull request as ready for review November 12, 2020 21:03

kertal mentioned this pull request Nov 16, 2020

[Discover] EuiDataGrid Implementation elastic/kibana#67259

Merged

13 tasks

This was referenced Nov 20, 2020

[EuiDataGrid] Document what props cause rows to re-render #3518

Closed

[Meta] [EuiDataGrid] data grid performance #4295

Closed

snide added 2 commits December 2, 2020 08:47

grid cells styling fixes

bef5073

Merge remote-tracking branch 'upstream/master' into feature/virtualiz…

1c2a571

…ed-datagrid

fix borders and stripes

1449d02

cchaos reviewed Jan 21, 2021

View reviewed changes

Merge branch 'master' into feature/virtualized-datagrid

0f532df

Merge pull request #18 from flash1293/flash1293/less-wrappers

80915fd

Less wrapper elements

chandlerprall changed the title ~~[WIP] datagrid virtualization~~ [EuiDataGrid] Virtualization Jan 26, 2021

Documentation

fa04eb1

Fix a unit test setup

1e345a1

thompsongl approved these changes Jan 28, 2021

View reviewed changes

chandlerprall added 2 commits January 28, 2021 12:10

chanagelog

ca01966

Merge branch 'master' into feature/virtualized-datagrid

d02a6b3

chandlerprall merged commit 1176832 into elastic:master Jan 28, 2021

chandlerprall deleted the feature/virtualized-datagrid branch January 28, 2021 19:17

This was referenced Jan 28, 2021

[EuiDataGrid] Scroll cells into view via keyboard #4456

Closed

[EuiDataGrid] Unnecessary scroll bars when constrained #4457

Closed

[EuiDataGrid] UX Cleanups #4458

Closed

chandlerprall mentioned this pull request Jan 28, 2021

Upgrade EUI to v31.4.0 elastic/kibana#89648

Merged

chandlerprall added the data grid label Feb 5, 2021

snide mentioned this pull request Feb 15, 2021

Upgrade EUI to v31.7.0 elastic/kibana#91210

Merged

cee-chen mentioned this pull request Feb 10, 2022

[EuiDataGrid] Fix open cell popovers causing auto height rows to bug out + misc cleanup #5622

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiDataGrid] Virtualization #4170

[EuiDataGrid] Virtualization #4170

chandlerprall commented Oct 22, 2020 •

edited

Loading

snide commented Dec 2, 2020 •

edited by chandlerprall

Loading

chandlerprall commented Jan 21, 2021

kibanamachine commented Jan 21, 2021

cchaos left a comment

cchaos Jan 21, 2021

cchaos Jan 21, 2021

snide commented Jan 21, 2021

cchaos commented Jan 21, 2021

kibanamachine commented Jan 26, 2021

chandlerprall commented Jan 26, 2021 •

edited

Loading

chandlerprall commented Jan 26, 2021

kibanamachine commented Jan 26, 2021

kibanamachine commented Jan 27, 2021

kibanamachine commented Jan 28, 2021

thompsongl left a comment

kibanamachine commented Jan 28, 2021

kibanamachine commented Jan 28, 2021

[EuiDataGrid] Virtualization #4170

[EuiDataGrid] Virtualization #4170

Conversation

chandlerprall commented Oct 22, 2020 • edited Loading

Summary

Items for follow up PRs:

Props

Constrained layout

Checklist

snide commented Dec 2, 2020 • edited by chandlerprall Loading

chandlerprall commented Jan 21, 2021

kibanamachine commented Jan 21, 2021

cchaos left a comment

Choose a reason for hiding this comment

cchaos Jan 21, 2021

Choose a reason for hiding this comment

cchaos Jan 21, 2021

Choose a reason for hiding this comment

snide commented Jan 21, 2021

cchaos commented Jan 21, 2021

kibanamachine commented Jan 26, 2021

chandlerprall commented Jan 26, 2021 • edited Loading

chandlerprall commented Jan 26, 2021

kibanamachine commented Jan 26, 2021

kibanamachine commented Jan 27, 2021

kibanamachine commented Jan 28, 2021

thompsongl left a comment

Choose a reason for hiding this comment

kibanamachine commented Jan 28, 2021

kibanamachine commented Jan 28, 2021

chandlerprall commented Oct 22, 2020 •

edited

Loading

snide commented Dec 2, 2020 •

edited by chandlerprall

Loading

chandlerprall commented Jan 26, 2021 •

edited

Loading