[DataGrid] Add 'gridReady' event to notify the user when the grid has been mounted #6568

yaredtsy · 2022-10-20T07:01:53Z

Problem

we can not use apiRef.current in useEffect. because apiRef.current.windowRef is null in the begging and we can not tell when it is ready to use. so the only solution was to use it in setTimout.

Solution

i have created an event called gridReady which will get triggered when the state initialization is finished. user can subscribe to it to get notified when the grid is ready to interact.

mui-bot · 2022-10-20T07:07:32Z

Netlify deploy preview

Netlify deploy preview: https://deploy-preview-6568--material-ui-x.netlify.app/

Updated pages

No updates.

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	749.5	1,475.7	749.5	1,042.06	291.541
Sort 100k rows	ms	874.9	1,496.8	1,358.8	1,268.18	208.753
Select 100k rows	ms	295.6	566.2	400.7	393.24	98.209
Deselect 100k rows	ms	198.8	380.9	289.8	279.42	68.346

Generated by 🚫 dangerJS against cfe8eba

DanailH

The destination branch should be next. As far as I can see this is not a critical issue. We can actually even consider it as an enhancement.

yaredtsy · 2022-10-20T20:17:44Z

@DanailH done, and I have accidentally removed other reviewers.

m4theushw

Thanks for the contribution! Could you fix the pipeline and provide a CodeSandbox showing that this PR solves the use case in #6411. I'm not very sure it will fix because the scrollTOp is reset in an effect when the grid is mounted:

mui-x/packages/grid/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx

Lines 208 to 216 in 7a7977f

 React.useEffect(() => { 

 if (disableVirtualization) { 

 renderZoneRef.current!.style.transform = `translate3d(0px, 0px, 0px)`; 

 } else { 

 // TODO a scroll reset should not be necessary 

 rootRef.current!.scrollLeft = 0; 

 rootRef.current!.scrollTop = 0; 

 } 

 }, [disableVirtualization]);

I was expecting something like

diff --git a/packages/grid/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx b/packages/grid/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx
index 2b773e035..f16e2999a 100644
--- a/packages/grid/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx
+++ b/packages/grid/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx
@@ -205,7 +205,7 @@ export const useGridVirtualScroller = (props: UseGridVirtualScrollerProps) => {
     containerWidth,
   ]);
 
-  React.useEffect(() => {
+  React.useLayoutEffect(() => {
     if (disableVirtualization) {
       renderZoneRef.current!.style.transform = `translate3d(0px, 0px, 0px)`;
     } else {
@@ -213,6 +213,10 @@ export const useGridVirtualScroller = (props: UseGridVirtualScrollerProps) => {
       rootRef.current!.scrollLeft = 0;
       rootRef.current!.scrollTop = 0;
     }
+
+    if (onReady) {
+      onReady();
+    }
   }, [disableVirtualization]);
 
   React.useEffect(() => {

yaredtsy · 2022-10-28T07:46:58Z

@m4theushw in setState I have modified the equality check. and it breaks 1 test.

diff --git a/packages/grid/x-data-grid/src/hooks/core/useGridStateInitialization.ts b/packages/grid/x-data-grid/src/hooks/core/useGridStateInitialization.ts
index 1662575f..debcf7a3 100644
--- a/packages/grid/x-data-grid/src/hooks/core/useGridStateInitialization.ts
+++ b/packages/grid/x-data-grid/src/hooks/core/useGridStateInitialization.ts
@@ -36,7 +36,7 @@ export const useGridStateInitialization = <Api extends GridApiCommon>(
         newState = state;
       }

-      if (apiRef.current.state === newState) {
+      if (isDeepEqual(apiRef.current.state, newState)) {
         return false;
       }

Demo: https://codesandbox.io/s/admiring-scott-chcls3?file=/demo.tsx

yaredtsy · 2022-10-31T22:06:42Z

Hey @m4theushw
I found the problem that was breaking the test. It was columnGrouping tests.

 it('should update headers when `columnGroupingModel` is modified', () => {
    ...
      setProps({ columnGroupingModel: [{ groupId: 'col2', children: [{ field: 'col2' }] }] });
      expect(screen.queryAllByRole('row')).to.have.length(3);
}

the problem was when the states change columnGrouping was not updating in the first round.

Demo: https://codesandbox.io/s/silly-hypatia-xgs712?file=/demo.tsx you have to click the update button twice to update it.

fix

--- a/packages/grid/x-data-grid/src/hooks/features/columnGrouping/useGridColumnGrouping.ts
+++ b/packages/grid/x-data-grid/src/hooks/features/columnGrouping/useGridColumnGrouping.ts
@@ -174,6 +174,7 @@ export const useGridColumnGrouping = (
         },
       };
     });
+    apiRef.current.forceUpdate();
   }, [
     apiRef,
     columnFields,

this seemed to fix it should I create another PR for this
Demo: https://codesandbox.io/s/elegant-jang-dnctgf

cherniavskii · 2022-11-01T16:06:51Z

packages/grid/x-data-grid/src/components/base/GridBody.tsx

+ apiRef.current.windowRef = { current: node };
+ const unsubscribe = apiRef.current.subscribeEvent('stateChange', () => {
+ setTimeout(() => {
+ apiRef.current.publishEvent('gridReady');


It doesn't seem like it fixes the issue: https://codesandbox.io/s/confident-forest-6tizwv?file=/src/App.tsx

does it not scroll to the index for you?

I add setTimeout because some states like rowMeta might not finish initializing after row data is set. I am experimenting with a way to check if all the states have finished before gridReady is triggered.

const unsubscribe = apiRef.current.subscribeEvent('stateChange', (newState) => { if (newState.rowsMeta.currentPageTotalHeight > 0) { setTimeout(() => { apiRef.current.publishEvent('gridReady'); }, 0); unsubscribe(); } }); }

cherniavskii · 2022-11-01T16:20:35Z

packages/grid/x-data-grid/src/components/base/GridBody.tsx

+ const unsubscribe = apiRef.current.subscribeEvent('stateChange', () => {
+ setTimeout(() => {
+ apiRef.current.publishEvent('gridReady');
+ }, 5);


Regarding the original issue #6411 specifically, would gridReady solve it if the grid is rendered in a loading state while waiting for rows data to load?

yes, that's why I add for 'stateChange' listener. if the rows data did not load it won't be fired. the downside is if you create an empty DataGrid, 'gridReady' won't be triggered.

here is an example. https://codesandbox.io/s/laughing-noyce-rm95xy?file=/index.tsx

If the grid is empty or in the loading state it's still "ready".

github-actions · 2022-11-07T07:54:46Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions · 2022-11-15T15:03:47Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

packages/grid/x-data-grid/src/components/base/GridBody.tsx

m4theushw · 2022-11-15T15:19:20Z

packages/grid/x-data-grid/src/components/base/GridBody.tsx

+ (node: HTMLDivElement) => {
+ if (node && apiRef.current.windowRef?.current == null) {
+ apiRef.current.windowRef = { current: node };
+ const triggerGridReadyEvent = debounce(() => apiRef.current.publishEvent('gridReady'), 100);


The debounce added is fighting against

mui-x/packages/grid/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx

Lines 212 to 214 in 4d77507

// TODO a scroll reset should not be necessary

rootRef.current!.scrollLeft = 0;

rootRef.current!.scrollTop = 0;

Why not try #6568 (review) that doesn't require setTimeout? We should try to avoid setTimeout.

Why not try #6568 (review) that doesn't require setTimeout? We should try to avoid setTimeout

I have tried it, but 'gridReady' is getting triggered multiple times. I could not get it to be triggered only 1 time.

diff --git a/packages/grid/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx b/packages/grid/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx index 782d3db54..b5e7ccf87 100644 --- a/packages/grid/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx +++ b/packages/grid/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import * as ReactDOM from 'react-dom'; import { unstable_useForkRef as useForkRef } from '@mui/utils'; +import { debounce } from '@mui/material/utils'; import { useGridApiContext } from '../../utils/useGridApiContext'; import { useGridRootProps } from '../../utils/useGridRootProps'; import { useGridSelector } from '../../utils/useGridSelector'; @@ -97,6 +98,10 @@ export const useGridVirtualScroller = (props: UseGridVirtualScrollerProps) => { getRowProps, } = props; + const triggerGridReadyEvent = React.useMemo( + () => debounce(() => apiRef.current.publishEvent('gridReady'), 300), + [apiRef], + ); const columnPositions = useGridSelector(apiRef, gridColumnPositionsSelector); const columnsTotalWidth = useGridSelector(apiRef, gridColumnsTotalWidthSelector); const cellFocus = useGridSelector(apiRef, gridFocusCellSelector); @@ -205,7 +210,7 @@ export const useGridVirtualScroller = (props: UseGridVirtualScrollerProps) => { containerWidth, ]); - React.useEffect(() => { + React.useLayoutEffect(() => { if (disableVirtualization) { renderZoneRef.current!.style.transform = `translate3d(0px, 0px, 0px)`; } else { @@ -213,7 +218,13 @@ export const useGridVirtualScroller = (props: UseGridVirtualScrollerProps) => { rootRef.current!.scrollLeft = 0; rootRef.current!.scrollTop = 0; } - }, [disableVirtualization]); + + apiRef.current.subscribeEvent('stateChange', (newState) => { + if (newState.rowsMeta.currentPageTotalHeight > 0) { + triggerGridReadyEvent(); + } + }); + }, [disableVirtualization, triggerGridReadyEvent, apiRef]);

this is how I tried to implement it.

hey @m4theushw

i was looking at

mui-x/packages/grid/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx

Lines 212 to 214 in 4d77507

// TODO a scroll reset should not be necessary

rootRef.current!.scrollLeft = 0;

rootRef.current!.scrollTop = 0;

and I think they are getting called too early before the rows are populated, I do not think they have any effect. I have tried rootRef.current!.scrollTop = 100. and it does not do anything. also when I logged rowMeta its currentPageTotalHeight is 0. so scrolling at that point has no effect.

Without this code, if you scroll to the middle of the viewport, disable virtualization with disableVirtualization=true, then re-enable it again, the scroll position will change a bit. Resetting the scroll was better than having this small scroll.

hey @m4theushw, I have managed useEffect to be called only once. and I have put the code in useGridVirtualScroller.

DanailH · 2022-11-16T08:44:34Z

@yaredtsy can we add tests to see the track of the number of times the gridReady is called? We probably need a few to be sure it's working as expected.

simple grid with a couple of rows
loading grid without any rows
grid with a lot of rows, enough to trigger the virtualization
a grid with rows and after some time an interaction happens that needs to result in a state update which rerenders the grid
grid with a predefined initial state

These are off the top of my head. The first 4 IMO are 100% needed so we know what we want to achieve.

yaredtsy · 2022-11-19T01:08:35Z

hey @DanailH
I have added the tests.

github-actions · 2022-12-14T22:05:57Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

packages/grid/x-data-grid-pro/src/tests/events.DataGridPro.test.tsx

github-actions · 2023-01-18T12:32:13Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Signed-off-by: yared tsegaye <[email protected]>

github-actions · 2023-09-22T14:45:55Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

salos1982 · 2024-04-04T10:38:11Z

@yaredtsy Hi, I have the same issue that is described in the pull request. Could you please at least add gridReady event?

flaviendelangle · 2024-04-04T11:07:26Z

@cherniavskii we should probably either close this PR or continue working on it to avoid giving false hope 👍

DanailH changed the title ~~[DataGrid] The 'gridReady' event has been added to notify the user when the grid has been mounted.~~ [DataGrid] Add 'gridReady' event to notify the user when the grid has been mounted Oct 20, 2022

DanailH requested review from cherniavskii, m4theushw, DanailH and MBilalShafi October 20, 2022 07:07

DanailH assigned DanailH and yaredtsy and unassigned DanailH Oct 20, 2022

DanailH added bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! labels Oct 20, 2022

DanailH requested changes Oct 20, 2022

View reviewed changes

DanailH added enhancement This is not a bug, nor a new feature and removed bug 🐛 Something doesn't work labels Oct 20, 2022

yaredtsy force-pushed the 6411_fix branch from c801377 to 3efd50e Compare October 20, 2022 11:39

yaredtsy changed the base branch from master to next October 20, 2022 11:45

yaredtsy requested review from DanailH and removed request for MBilalShafi, cherniavskii and m4theushw October 20, 2022 17:14

m4theushw self-requested a review October 26, 2022 12:51

m4theushw reviewed Oct 27, 2022

View reviewed changes

yaredtsy force-pushed the 6411_fix branch from 4e6412c to 7e0c811 Compare October 28, 2022 09:00

cherniavskii reviewed Nov 1, 2022

View reviewed changes

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 7, 2022

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 7, 2022

yaredtsy force-pushed the 6411_fix branch from 2535600 to 8f052d5 Compare November 15, 2022 15:15

m4theushw reviewed Nov 15, 2022

View reviewed changes

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 16, 2022

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 14, 2022

yaredtsy force-pushed the 6411_fix branch from 11887f7 to ca575bf Compare December 30, 2022 09:06

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 30, 2022

DanailH reviewed Jan 6, 2023

View reviewed changes

packages/grid/x-data-grid-pro/src/tests/events.DataGridPro.test.tsx Outdated Show resolved Hide resolved

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 18, 2023

yaredtsy added 7 commits February 15, 2023 10:26

feat: gridyReady event added

2c7cb7d

test: test for gridReady added

dc3e41a

fix: docs built for gridready

209395f

refa : test and comment refactored

95439a9

refa: added useEffectOnce

775017f

fix: Infinite loop fixed

eaaf4cc

refa: test moved from DataGridPro to DataGrid

7ddcac6

yaredtsy force-pushed the 6411_fix branch from aff4461 to 7ddcac6 Compare February 15, 2023 07:30

Merge branch 'next' into 6411_fix

789c29b

Signed-off-by: yared tsegaye <[email protected]>

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 15, 2023

Merge branch 'mui:master' into 6411_fix

cfe8eba

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 22, 2023

MBilalShafi changed the base branch from master to v6.x March 21, 2024 02:48

MBilalShafi added the v6.x label Mar 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Add 'gridReady' event to notify the user when the grid has been mounted #6568

[DataGrid] Add 'gridReady' event to notify the user when the grid has been mounted #6568

yaredtsy commented Oct 20, 2022 •

edited

Loading

mui-bot commented Oct 20, 2022 •

edited

Loading

DanailH left a comment •

edited

Loading

yaredtsy commented Oct 20, 2022

m4theushw left a comment

yaredtsy commented Oct 28, 2022

yaredtsy commented Oct 31, 2022

cherniavskii Nov 1, 2022

yaredtsy Nov 1, 2022

cherniavskii Nov 1, 2022

yaredtsy Nov 1, 2022

yaredtsy Nov 2, 2022

m4theushw Nov 15, 2022

github-actions bot commented Nov 7, 2022

github-actions bot commented Nov 15, 2022

m4theushw Nov 15, 2022

yaredtsy Nov 16, 2022

yaredtsy Nov 18, 2022 •

edited

Loading

m4theushw Nov 18, 2022

yaredtsy Dec 30, 2022

DanailH commented Nov 16, 2022

yaredtsy commented Nov 19, 2022

github-actions bot commented Dec 14, 2022

github-actions bot commented Jan 18, 2023

github-actions bot commented Sep 22, 2023

salos1982 commented Apr 4, 2024

flaviendelangle commented Apr 4, 2024

	React.useEffect(() => {
	if (disableVirtualization) {
	renderZoneRef.current!.style.transform = `translate3d(0px, 0px, 0px)`;
	} else {
	// TODO a scroll reset should not be necessary
	rootRef.current!.scrollLeft = 0;
	rootRef.current!.scrollTop = 0;
	}
	}, [disableVirtualization]);

[DataGrid] Add 'gridReady' event to notify the user when the grid has been mounted #6568

Are you sure you want to change the base?

[DataGrid] Add 'gridReady' event to notify the user when the grid has been mounted #6568

Conversation

yaredtsy commented Oct 20, 2022 • edited Loading

mui-bot commented Oct 20, 2022 • edited Loading

Netlify deploy preview

Updated pages

DanailH left a comment • edited Loading

Choose a reason for hiding this comment

yaredtsy commented Oct 20, 2022

m4theushw left a comment

Choose a reason for hiding this comment

yaredtsy commented Oct 28, 2022

yaredtsy commented Oct 31, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 7, 2022

github-actions bot commented Nov 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yaredtsy Nov 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanailH commented Nov 16, 2022

yaredtsy commented Nov 19, 2022

github-actions bot commented Dec 14, 2022

github-actions bot commented Jan 18, 2023

github-actions bot commented Sep 22, 2023

salos1982 commented Apr 4, 2024

flaviendelangle commented Apr 4, 2024

yaredtsy commented Oct 20, 2022 •

edited

Loading

mui-bot commented Oct 20, 2022 •

edited

Loading

DanailH left a comment •

edited

Loading

yaredtsy Nov 18, 2022 •

edited

Loading