Skip to content
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.

Sign up for GitHub

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 horizontal scroll not working on empty grid #3821

Merged
merged 10 commits into from
Feb 14, 2022
Merged

[DataGrid] Fix horizontal scroll not working on empty grid #3821

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
61b7c78
[DataGrid] Move content height style logic to useGridVirtualScroller
cherniavskii Feb 2, 2022
4c1581a
add unit test
cherniavskii Feb 10, 2022
5d14a55
add e2e test
cherniavskii Feb 10, 2022
49ed847
get minHeight from content styles
cherniavskii Feb 10, 2022
018d0d5
get rid of unnecessary state
cherniavskii Feb 10, 2022
39aea4b
code review: Matheus
cherniavskii Feb 14, 2022
6221157
Merge branch 'master' into horizontal-scroll-on-empty-grid
cherniavskii Feb 14, 2022
29789b1
use rootRef instead of apiRef
cherniavskii Feb 14, 2022
1ab5fc7
format
cherniavskii Feb 14, 2022
22fc2d9
Update test/e2e/fixtures/DataGrid/EmptyGrid.tsx
cherniavskii Feb 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion packages/grid/_modules_/grid/hooks/features/virtualization/useGridVirtualScroller.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ export const useGridVirtualScroller = (props: UseGridVirtualScrollerProps) => {
const scrollPosition = React.useRef({ top: 0, left: 0 });
const [containerWidth, setContainerWidth] = React.useState<number | null>(null);
const prevTotalWidth = React.useRef(columnsMeta.totalWidth);
const [shouldExtendContent, setShouldExtendContent] = React.useState(false);

const computeRenderContext = React.useCallback(() => {
if (disableVirtualization) {
Expand Down Expand Up @@ -150,6 +151,26 @@ export const useGridVirtualScroller = (props: UseGridVirtualScrollerProps) => {

useGridApiEventHandler(apiRef, GridEvents.resize, handleResize);

const handleContentSizeChange = React.useCallback(() => {
if (!apiRef.current.windowRef?.current) {
return;
}
setShouldExtendContent(
apiRef.current.windowRef.current.scrollHeight <=
apiRef.current.windowRef.current.clientHeight,
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This callback will be called when the virtualScrollerContentSizeChange event is fired and the event is fired when contentSize changes, which happens inside the hook. We can remove this state update and migrate the minHeight to contentSize, avoiding one rerender.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!
I noticed that the event is emitted in the same hook, but didn't think of removing unnecessary state

}, [apiRef]);

const contentStyle = {
minHeight: shouldExtendContent ? '100%' : 'auto',
};

useGridApiEventHandler(
apiRef,
GridEvents.virtualScrollerContentSizeChange,
handleContentSizeChange,
);

const getRenderableIndexes = ({ firstIndex, lastIndex, buffer, minFirstIndex, maxLastIndex }) => {
return [
clamp(firstIndex - buffer, minFirstIndex, maxLastIndex),
Expand Down Expand Up @@ -371,7 +392,10 @@ export const useGridVirtualScroller = (props: UseGridVirtualScrollerProps) => {
style: { ...style, ...rootStyle },
...other,
}),
getContentProps: ({ style = {} } = {}) => ({ style: { ...style, ...contentSize } }),
getContentProps: ({ style = {} } = {}) => ({
style: { ...contentStyle, ...style, ...contentSize },
}),
getRenderZoneProps: () => ({ ref: renderZoneRef }),
shouldExtendContent,
};
};
24 changes: 2 additions & 22 deletions packages/grid/x-data-grid-pro/src/DataGridProVirtualScroller.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ const DataGridProVirtualScroller = React.forwardRef<
const visibleColumnFields = useGridSelector(apiRef, gridVisibleColumnFieldsSelector);
const leftColumns = React.useRef<HTMLDivElement>(null);
const rightColumns = React.useRef<HTMLDivElement>(null);
const [shouldExtendContent, setShouldExtendContent] = React.useState(false);

const handleRenderZonePositioning = React.useCallback(({ top }) => {
if (leftColumns.current) {
Expand All @@ -144,6 +143,7 @@ const DataGridProVirtualScroller = React.forwardRef<
getContentProps,
getRenderZoneProps,
updateRenderZonePosition,
shouldExtendContent,
} = useGridVirtualScroller({
ref,
renderZoneMinColumnIndex: leftPinnedColumns.length,
Expand All @@ -165,22 +165,6 @@ const DataGridProVirtualScroller = React.forwardRef<
refreshRenderZonePosition();
}, [refreshRenderZonePosition]);

const handleContentSizeChange = React.useCallback(() => {
if (!apiRef.current.windowRef?.current) {
return;
}
setShouldExtendContent(
apiRef.current.windowRef.current.scrollHeight <=
apiRef.current.windowRef.current.clientHeight,
);
}, [apiRef]);

useGridApiEventHandler(
apiRef,
GridEvents.virtualScrollerContentSizeChange,
handleContentSizeChange,
);

const leftRenderContext =
renderContext && leftPinnedColumns.length > 0
? {
Expand All @@ -199,17 +183,13 @@ const DataGridProVirtualScroller = React.forwardRef<
}
: null;

const contentStyle = {
minHeight: shouldExtendContent ? '100%' : 'auto',
};

const pinnedColumnsStyle = {
minHeight: shouldExtendContent ? '100%' : 'auto',
cherniavskii marked this conversation as resolved.
Show resolved Hide resolved
};

return (
<GridVirtualScroller {...getRootProps(other)}>
<GridVirtualScrollerContent {...getContentProps({ style: contentStyle })}>
<GridVirtualScrollerContent {...getContentProps()}>
{leftRenderContext && (
<VirtualScrollerPinnedColumns
ref={leftColumns}
Expand Down