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.

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

fix(react-grid): scrollbar jumps when scrolling a virtual table with different row heights from bottom to top (T1171681) #3657

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -537,4 +537,88 @@ describe('VirtualTableLayout', () => {
});
});
});

describe('row heights', () => {
const getBoundingClientRect = () => ({
height: 70,
});

const newProps = {
...defaultProps,
tableComponent: ({ forwardedRef, ...props }) => {
(forwardedRef as any).current = { getBoundingClientRect };
return <table {...props} />;
},
rowComponent: ({ forwardedRef }) => {
(forwardedRef as any)({ getBoundingClientRect });
return null;
},
};

it('should specify correct row height at startup', () => {
expect.hasAssertions();

const rows = [
{ key: 1 },
{ key: 2, height: 10 },
];

getCollapsedGrids
.mockImplementationOnce((args) => {
const { getRowHeight } = args;
expect(getRowHeight(rows[0]))
.toEqual(newProps.estimatedRowHeight);
expect(getRowHeight(rows[1]))
.toEqual(10);

return jest.requireActual('@devexpress/dx-grid-core').getCollapsedGrids(args);
});

mount((
<VirtualTableLayout
{...newProps}
bodyRows={rows}
/>
));
});

it('should store row height when rendered', () => {
const rows = [
{ key: 1 },
{ key: 2, height: 10 },
];

mount((
<VirtualTableLayout
{...newProps}
bodyRows={rows}
/>
));

const { getRowHeight } = getCollapsedGrids.mock.calls[0][0];
expect(getRowHeight(rows[0]))
.toEqual(70);
expect(getRowHeight(rows[1]))
.toEqual(70);
});

it('should clear row height when rows updated', () => {
const rows = [
{ key: 11 },
{ key: 12 },
];

const tree = mount((
<VirtualTableLayout
{...newProps}
bodyRows={rows.slice(0, 2)}
/>
));
tree.setProps({ bodyRows: [rows[0]] });

const { getRowHeight } = getCollapsedGrids.mock.calls[0][0];
expect(getRowHeight(rows[1]))
.toEqual(newProps.estimatedRowHeight);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export class VirtualTableLayout extends React.PureComponent<PropsType, VirtualTa
super(props);

this.state = {
rowHeights: new Map<any, number>(),
viewportTop: 0,
skipItems: [0, 0],
containerHeight: 600,
Expand Down Expand Up @@ -63,13 +64,19 @@ export class VirtualTableLayout extends React.PureComponent<PropsType, VirtualTa
return;
}
if (ref === null) {
this.rowRefs.delete(row.key);
this.rowRefs.delete(row);
} else {
this.rowRefs.set(row.key, ref);
this.rowRefs.set(row, ref);
}
}

componentDidMount() {
this.storeRowHeights();
}

componentDidUpdate(prevProps, prevState) {
setTimeout(this.storeRowHeights.bind(this));

const { bodyRows, columns } = this.props;
// NOTE: the boundaries depend not only on scroll position and container dimensions
// but on body rows too. This boundaries update is especially important when
Expand Down Expand Up @@ -103,12 +110,49 @@ export class VirtualTableLayout extends React.PureComponent<PropsType, VirtualTa
}
}

static getDerivedStateFromProps(nextProps, prevState) {
const { rowHeights: prevRowHeight } = prevState;
const rowHeights = [...nextProps.headerRows, ...nextProps.bodyRows, ...nextProps.footerRows]
.reduce(
(acc, row) => {
const rowHeight = prevRowHeight.get(row.key);
if (rowHeight !== undefined) {
acc.set(row.key, rowHeight);
}
return acc;
},
new Map(),
);
return { rowHeights };
}

getRowHeight = (row) => {
const { rowHeights } = this.state;
const { estimatedRowHeight } = this.props;
if (row) {
const realHeight = this.rowRefs.get(row.key)?.getBoundingClientRect().height;
return row.height || realHeight || this.props.estimatedRowHeight;
const storedHeight = rowHeights.get(row.key);
if (storedHeight !== undefined) return storedHeight;
if (row.height) return row.height;
}
return estimatedRowHeight;
}

storeRowHeights() {
const rowsWithChangedHeights = Array.from(this.rowRefs.entries())
.map(([row, ref]) => [row, ref])
.filter(([, node]) => !!node)
.map(([row, node]) => [row, node.getBoundingClientRect().height])
.filter(([row, height]) => row.type !== TABLE_STUB_TYPE && height !== this.getRowHeight(row));

if (rowsWithChangedHeights.length) {
const { rowHeights } = this.state;
rowsWithChangedHeights
.forEach(([row, height]) => rowHeights.set(row.key, height));

this.setState({
rowHeights,
});
}
return this.props.estimatedRowHeight;
}

onScroll = (e) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export interface VirtualTableLayoutProps extends TableLayoutProps {
}
/** @internal */
export type VirtualTableLayoutState = {
rowHeights: Map<any, number>,
viewportTop: number,
skipItems: [number, number],
containerHeight: number,
Expand Down