-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DataGrid] Refactor: row virtualization & rendering #12247
Conversation
Deploy preview: https://deploy-preview-12247--material-ui-x.netlify.app/ |
) { | ||
isRowWithFocusedCellNotInRange = true; | ||
const firstRowToRender = renderContext.firstRowIndex; | ||
const lastRowToRender = Math.min(renderContext.lastRowIndex, rowModels.length); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, in some cases renderContext.lastRowIndex
is greater than rowModels.length
. It's not a new bug, it's just that before this PR the Math.min
operation was hiding as rows.slice(firstRowToRender, lastRowToRender)
.
The root cause is that renderContext
lags behind the rows when the "view" changes (e.g. filter modified, row expanded/collapsed, sorting modifier, etc). I've partially addressed the issue with the useGridApiEventHandler
calls below, but some cases remain. I don't have enough time to hunt them all down in this PR.
NONE, | ||
LEFT, | ||
RIGHT, | ||
VIRTUAL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've introduced the "virtual" term to refer to out-of-viewport focused cells that need to be rendered to keep their DOM state alive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, love the simplifications!
A few performance & refactor changes for the row rendering.
Based on #12238
Changes:
offsets
object to avoid re-renders{ colSpan: 1 }
Very limited performance gains, around 3%, but it's always nice to shave off some CPU time during scrolling.