[EuiDataGrid] Auto-fit rows to content by VladLasitsa · Pull Request #4958 · elastic/eui

I'm curious if you've tried a resize observer or similar to respond to changes in cell contents, instead of only during mount or onCellLoaded is called?

src/components/datagrid/data_grid_cell.tsx

src/components/datagrid/row_height_utils.ts

VladLasitsa · 2021-07-26T20:56:43Z

I'm curious if you've tried a resize observer or similar to respond to changes in cell contents, instead of only during mount or onCellLoaded is called?

You mean like this but for all cellContents?

I think mount, onCellLoaded and update height in shouldComponentUpdate cover all needs. But I can try to use ResizeObserver.

UPDATE: Start using ResizeObserver - work fine, also it allow to remove window.fonts.dowload method and now we setHeight in cache only here and in didUpdate method (needed because of sorting)

kibanamachine · 2021-07-26T22:21:26Z

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

kibanamachine · 2021-07-27T14:18:00Z

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

chandlerprall

With the resize observer, these changes are easier to follow & my experience playing with the result seems to be better & more stable 👍

The scroll bar acts a little weird while scrolling. Without knowing all of the row heights, I don't think this behaviour is avoidable when scrolling down, but it seems that the grid loses its knowledge of row heights while scrolling back up.

datagrid_scrolling.mov

src/components/datagrid/data_grid_body.tsx

src/components/datagrid/data_grid_cell.tsx

src/components/datagrid/row_height_utils.ts

VladLasitsa · 2021-07-28T15:25:11Z

but it seems that the grid loses its knowledge of row heights while scrolling back up.

Could you please provide a video or gif for this?

kibanamachine · 2021-07-28T15:47:10Z

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

…grid

kibanamachine · 2021-07-28T16:35:32Z

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

elizabetdev · 2021-09-16T14:12:44Z

When we change the density to the smaller size the last row on the screen gets overlapped by the footer (in fullscreen mode):

Screen.Recording.2021-09-16.at.03.06.PM.mov

But it seems this also happens in #/tabular-content/data-grid-virtualization:

kibanamachine · 2021-09-16T14:31:46Z

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

VladLasitsa · 2021-09-16T14:48:59Z

So IMO we should set the toolbarVisibility={{ showStyleSelector: false }} for the examples we're providing. But it makes sense to have the Density aka toolbarVisibility={{ showStyleSelector: true }} for other use cases that uses auto. So we shouldn't disable it by default when auto is enabled.

Makes sense to me, but let's also be sure to explain that in the docs as well. Basically a sentence about when to keep/hide that density selector.

@VladLasitsa Can you push you visibility fix so we can test? It's hard to tell just by your screencast.

I pushed changes. @cchaos could you please take a look?

kibanamachine · 2021-09-16T15:13:45Z

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

chandlerprall · 2021-09-16T16:52:07Z

I tried the following solution: change visibility property in style prop - visibility: hidden when we don't have correct heights and visibility: visible when we reset grid after heights calculation. And now I have the following:

Data.grid.row.height.options.-.Elastic.UI.Framework.mp4
Is it okay?

If we try to fix in this PR (I'm good with Elizabet's recommendation to solve it separately),

@VladLasitsa Cell from data_grid_body.tsx line 52 receives an isScrolling prop that we could probably base that on:

Adds an additional isScrolling parameter to the children render function. This parameter can be used to show a placeholder row or column while the list is being scrolled.

Note that using this parameter will result in an additional render call after scrolling has stopped (when isScrolling changes from true to false).

https://react-window.vercel.app/#/api/FixedSizeGrid

kibanamachine · 2021-09-16T17:20:08Z

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

src/components/datagrid/body/data_grid_body.tsx

kibanamachine · 2021-09-16T18:49:17Z

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

chandlerprall · 2021-09-16T20:10:26Z

Let's resolve the flashing content in a separate PR; I think it'd be good to do some more investigation into the issue & solutions first, and don't want to hold auto-fitting up for that.

This reverts commit d1d4305.

kibanamachine · 2021-09-16T20:47:50Z

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

kibanamachine · 2021-09-16T23:07:10Z

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

…grid

kibanamachine · 2021-09-20T17:11:21Z

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

kibanamachine · 2021-09-20T18:31:30Z

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

elizabetdev · 2021-09-20T18:33:21Z

@cchaos, @VladLasitsa, and @chandlerprall,

I updated both height demos and the density is now working. To make it work I used relative font sizes in EuiText and EuiMarkdownFormat.

I also tried to explain better how the font size overriding works. Feel free o suggest a better explanation.

elizabetdev

I finished the design review. Tested in Chrome, Safari, Edge and Firefox, and LGTM. 🎉

The only issue I can still find is this one: #4958 (comment). I was not able to solve it.

And thanks @VladLasitsa for all the effort and patient. This feature looks amazing! 👍🏽

chandlerprall

Thank you @VladLasitsa for so much effort on this project, and everyone else's fantastic attention to detail in reviews & additional contributions. Y'all are awesome ❤️

This looks ready to merge, I'll make a new issue to track #4958 (comment) as it exists on master separate from these changes.

Add auto height possibility to row

83bb0c3

VladLasitsa requested a review from alexwizp July 16, 2021 13:16

Fix eslint

39bfc86

Fix some problems with hiding column

40c2cf0

VladLasitsa added 3 commits July 19, 2021 18:29

Fix example

e76129d

Fix example

c2de611

Merge branch 'master' into auto_height_data_grid

d06b9fa

fix calculation height so that work for images and fix problems with …

428860f

…font

Fix tests

05c52a8

fix eslint

eca4c2b

chandlerprall reviewed Jul 26, 2021

View reviewed changes

src/components/datagrid/data_grid_cell.tsx Outdated Show resolved Hide resolved

src/components/datagrid/row_height_utils.ts Outdated Show resolved Hide resolved

fix some problems

82f698a

use resizeObserver instead of handling height in didMount method

253f16a

chandlerprall reviewed Jul 27, 2021

View reviewed changes

src/components/datagrid/data_grid_body.tsx Outdated Show resolved Hide resolved

src/components/datagrid/data_grid_cell.tsx Outdated Show resolved Hide resolved

src/components/datagrid/row_height_utils.ts Outdated Show resolved Hide resolved

Fix some remarks

9fe0270

Merge remote-tracking branch 'upstream/master' into auto_height_data_…

8027a88

…grid

fix some problems with shrinking

2664080

make visibility property depends on calculating cell height

d1d4305

Merge branch 'master' into auto_height_data_grid

110b903

make lint happy

5d589cc

cee-chen reviewed Sep 16, 2021

View reviewed changes

src/components/datagrid/body/data_grid_body.tsx Show resolved Hide resolved

Merge branch 'master' into auto_height_data_grid

675ebad

Revert "make visibility property depends on calculating cell height"

9441e39

This reverts commit d1d4305.

snapshot

9e3fc6d

elizabetdev added 2 commits September 20, 2021 17:40

Updating example to make density work

dbe8cf6

Merge remote-tracking branch 'upstream/master' into auto_height_data_…

0dff342

…grid

Better gridStyle font size explanation

a82612d

elizabetdev approved these changes Sep 20, 2021

View reviewed changes

chandlerprall approved these changes Sep 21, 2021

View reviewed changes

chandlerprall mentioned this pull request Sep 21, 2021

[EuiDataGrid] Last row overlaps with footer in high-density setup #5203

Closed

VladLasitsa merged commit 8f8d0b5 into elastic:master Sep 21, 2021

cee-chen mentioned this pull request Sep 21, 2021

[EuiDataGrid] Auto-fit rows to content #4634

Closed

breehall mentioned this pull request Oct 1, 2021

Bumping EUI to 39.0.0 elastic/kibana#113633

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

Conversation

VladLasitsa commented Jul 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

kibanamachine commented Jul 16, 2021

Uh oh!

kibanamachine commented Jul 16, 2021

Uh oh!

kibanamachine commented Jul 19, 2021

Uh oh!

kibanamachine commented Jul 19, 2021

Uh oh!

kibanamachine commented Jul 22, 2021

Uh oh!

kibanamachine commented Jul 23, 2021

Uh oh!

kibanamachine commented Jul 23, 2021

Uh oh!

chandlerprall left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

VladLasitsa commented Jul 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kibanamachine commented Jul 26, 2021

Uh oh!

kibanamachine commented Jul 27, 2021

Uh oh!

chandlerprall left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

VladLasitsa commented Jul 28, 2021

Uh oh!

kibanamachine commented Jul 28, 2021

Uh oh!

kibanamachine commented Jul 28, 2021

Uh oh!

elizabetdev commented Sep 16, 2021

Uh oh!

kibanamachine commented Sep 16, 2021

Uh oh!

VladLasitsa commented Sep 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kibanamachine commented Sep 16, 2021

Uh oh!

chandlerprall commented Sep 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kibanamachine commented Sep 16, 2021

Uh oh!

Uh oh!

kibanamachine commented Sep 16, 2021

Uh oh!

chandlerprall commented Sep 16, 2021

Uh oh!

kibanamachine commented Sep 16, 2021

Uh oh!

kibanamachine commented Sep 16, 2021

Uh oh!

kibanamachine commented Sep 20, 2021

Uh oh!

kibanamachine commented Sep 20, 2021

Uh oh!

elizabetdev commented Sep 20, 2021

Uh oh!

elizabetdev left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chandlerprall left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

VladLasitsa commented Jul 16, 2021 •

edited

Loading

VladLasitsa commented Jul 26, 2021 •

edited

Loading

VladLasitsa commented Sep 16, 2021 •

edited

Loading

chandlerprall commented Sep 16, 2021 •

edited

Loading

elizabetdev left a comment •

edited

Loading