-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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] Fix first row flickering with autoHeight enabled #14235
Changes from all commits
30fe934
b6555c2
7640bba
fa4b9cf
9b14b51
9b61d42
5f96a3a
501b648
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,11 +5,12 @@ import { DataGridProcessedProps } from '../../../models/props/DataGridProps'; | |
import { useGridApiMethod } from '../../utils/useGridApiMethod'; | ||
import { GridStateInitializer } from '../../utils/useGridInitializeState'; | ||
|
||
type RootProps = Pick<DataGridProcessedProps, 'disableVirtualization'>; | ||
type RootProps = Pick<DataGridProcessedProps, 'disableVirtualization' | 'autoHeight'>; | ||
|
||
export type GridVirtualizationState = { | ||
enabled: boolean; | ||
enabledForColumns: boolean; | ||
enabledForRows: boolean; | ||
renderContext: GridRenderContext; | ||
}; | ||
|
||
|
@@ -21,9 +22,12 @@ export const EMPTY_RENDER_CONTEXT = { | |
}; | ||
|
||
export const virtualizationStateInitializer: GridStateInitializer<RootProps> = (state, props) => { | ||
const { disableVirtualization, autoHeight } = props; | ||
|
||
const virtualization = { | ||
enabled: !props.disableVirtualization, | ||
enabledForColumns: true, | ||
enabled: !disableVirtualization, | ||
enabledForColumns: !disableVirtualization, | ||
enabledForRows: !disableVirtualization && !autoHeight, | ||
renderContext: EMPTY_RENDER_CONTEXT, | ||
}; | ||
|
||
|
@@ -47,6 +51,8 @@ export function useGridVirtualization( | |
virtualization: { | ||
...state.virtualization, | ||
enabled, | ||
enabledForColumns: enabled, | ||
enabledForRows: enabled && !props.autoHeight, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure how to feel about putting the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we move the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any suggestions where/how @romgrk? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, feel free to ignore if there's no other good place. |
||
}, | ||
})); | ||
}; | ||
|
@@ -75,6 +81,6 @@ export function useGridVirtualization( | |
/* eslint-disable react-hooks/exhaustive-deps */ | ||
React.useEffect(() => { | ||
setVirtualization(!props.disableVirtualization); | ||
}, [props.disableVirtualization]); | ||
}, [props.disableVirtualization, props.autoHeight]); | ||
/* eslint-enable react-hooks/exhaustive-deps */ | ||
} |
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.
There was a bug here.
hasVirtualization
is usinggridVirtualizationColumnEnabledSelector
to get the column virtualization state. This was previously alwaystrue
, even when thedisableVirtualization
prop was passed to DataGrid. The0
here meant that pinned column headers were duplicated were not being removed from the middle section of columns.