Skip to content

Commit

Permalink
fix(react-grid): scrollbar jumps when scrolling a virtual table with …
Browse files Browse the repository at this point in the history
…different row heights from bottom to top (T1171681) (#3657)

* try restore rows height storage functionality and test ad hoc

* restore rowRefs structure

* tweak test mocks

* address Yulia's remarks

* .

* fix linter
  • Loading branch information
VasilyStrelyaev authored Jun 29, 2023
1 parent 5655906 commit cd0da0c
Show file tree
Hide file tree
Showing 3 changed files with 134 additions and 5 deletions.
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

0 comments on commit cd0da0c

Please sign in to comment.