[DataGrid] Fix grid dimensions calculation with autoHeight=true
#7489
Conversation
cherniavskii
added
bug 🐛
| @@ -79,6 +78,7 @@ export function useGridDimensions( | |||
| const rootElement = apiRef.current.rootElementRef?.current; | |||
| const columnsTotalWidth = gridColumnsTotalWidthSelector(apiRef); | |||
| const pinnedRowsHeight = calculatePinnedRowsHeight(apiRef); | |||
| const rowsMeta = gridRowsMetaSelector(apiRef.current.state); | |||
There was a problem hiding this comment.
The rowsMeta.currentPageTotalHeight value was outdated here, so I moved rowsMeta inside the callback.
For example, when filtering the rows and there were no rows matching the filtering criteria, updateGridDimensionsRef was called with the outdated value of rowsMeta.currentPageTotalHeight before applying filters.
These are the results for the performance tests:
|
cherniavskii
changed the title
cherniavskii
changed the title
autoHeight=true
github-actions
bot
added
the
PR: out-of-date
This comment was marked as outdated.
This comment was marked as outdated.
github-actions
bot
removed
the
PR: out-of-date
| rowsMeta.currentPageTotalHeight, | ||
| totalHeaderHeight, | ||
| ]); | ||
| }, [apiRef, props.scrollbarSize, props.autoHeight, totalHeaderHeight]); |
There was a problem hiding this comment.
Removing rowsMeta.currentPageTotalHeight caused another bug. Go to https://deploy-preview-7489--material-ui-x.netlify.app/x/react-data-grid/layout/#auto-height and remove all rows. The label is misaligned because its height is from when there was still one row left.
rowsMeta.currentPageTotalHeight as dependency ensured that whenever the total height changed (which may occur not only by filtering but also by removing rows), the dimensions would be updated.
I solved this problem in #6979 by checking currentPage.rows.length === 0 instead of the height.
There was a problem hiding this comment.
Good catch!
What do you think about the direction taken in this PR?
It's not fully ready yet (I've noticed that it doesn't always work), but I think it's still a problem with dimensions calculations rather than overlays layout.
Does it make sense to put more effort into it?
There was a problem hiding this comment.
If there's a bug that makes the returned value of apiRef.current.getRootDimensions inconsistent, then we should fix it, so +1 for pushing this forward. For the specific problem of the "No results" overlay, I think we shouldn't even have this problem in the first place if the overlay weren't rendered on top of the content. In #6979 I delegated to CSS to expand the overlay to fill the available height, it's one less point of failure now. If we consider #7489 as the fix for #6970, I would prefer to still merge my PR as an improvement to how the overlays are rendered.
There was a problem hiding this comment.
Alright, sounds good to me!
This PR might be a potential fix for the same issue on the master branch since we won't change the overlays layout in v5.
github-actions
bot
added
the
PR: out-of-date
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
Closing this PR, because the underlying issue was fixed by #8091 |
Fixes #6970
Before: https://codesandbox.io/s/beautiful-silence-24ue6i?file=/demo.tsx
After: https://codesandbox.io/s/great-microservice-r98k1d?file=/demo.tsx
Steps to reproduce:
TODO:
masterbranch