Skip to content

fix: Fix Grid scrollToCell method not working if a scrollTop prop is defined, which is usual for WindowScroller usage #1894

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

Open
driskell wants to merge 1 commit into
base: master
Choose a base branch
from
+33 −4
Open

fix: Fix Grid scrollToCell method not working if a scrollTop prop is defined, which is usual for WindowScroller usage #1894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 20 additions & 0 deletions source/Grid/Grid.jest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,26 @@ describe('Grid', () => {
expect(grid.state.scrollTop).toEqual(1900);
});

it('should preserve scrollTop state after scrollToCell when scrollTop prop is unchanged', () => {
// Render with autoHeight=true and an explicit scrollTop=0 prop, as WindowScroller would apply initially.
const grid = render(
getMarkup({
autoHeight: true,
scrollTop: 0,
}),
);
expect(grid.state.scrollTop).toEqual(0);

// Call scrollToCell imperatively (equivalent to list.scrollToRow()).
// It should result in a scroll movement, ignoring the scrollTop prop.
grid.scrollToCell({rowIndex: 49});

// 100 rows * 20px rowHeight = 2,000px total height
// viewport height = 100px → 5 rows visible
// Minimum scroll to show row 49 at bottom = 49*20 + 20 - 100 = 900
expect(grid.state.scrollTop).toEqual(900);
});

it('should support scrollToPosition() public method', () => {
const grid = render(getMarkup());
expect(grid.state.scrollLeft).toEqual(0);
Expand Down
17 changes: 13 additions & 4 deletions source/Grid/Grid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,8 @@ type InstanceProps = {
prevIsScrolling: boolean,
prevScrollToColumn: number,
prevScrollToRow: number,
prevScrollLeft?: number,
prevScrollTop?: number,

columnSizeAndPositionManager: ScalingCellSizeAndPositionManager,
rowSizeAndPositionManager: ScalingCellSizeAndPositionManager,
Expand Down Expand Up @@ -346,6 +348,8 @@ class Grid extends React.PureComponent<Props, State> {
prevIsScrolling: props.isScrolling === true,
prevScrollToColumn: props.scrollToColumn,
prevScrollToRow: props.scrollToRow,
prevScrollLeft: props.scrollToColumn,
prevScrollTop: props.scrollToRow,

scrollbarSize: 0,
scrollbarSizeMeasured: false,
Expand Down Expand Up @@ -826,6 +830,8 @@ class Grid extends React.PureComponent<Props, State> {
): $Shape<State> {
const newState = {};

let {instanceProps} = prevState;

if (
(nextProps.columnCount === 0 && prevState.scrollLeft !== 0) ||
(nextProps.rowCount === 0 && prevState.scrollTop !== 0)
Expand All @@ -836,9 +842,12 @@ class Grid extends React.PureComponent<Props, State> {
// only use scroll{Left,Top} from props if scrollTo{Column,Row} isn't specified
// scrollTo{Column,Row} should override scroll{Left,Top}
} else if (
(nextProps.scrollLeft !== prevState.scrollLeft &&
(nextProps.scrollLeft !== instanceProps.prevScrollLeft &&
nextProps.scrollLeft !== prevState.scrollLeft &&
nextProps.scrollToColumn < 0) ||
(nextProps.scrollTop !== prevState.scrollTop && nextProps.scrollToRow < 0)
(nextProps.scrollTop !== instanceProps.prevScrollTop &&
nextProps.scrollTop !== prevState.scrollTop &&
nextProps.scrollToRow < 0)
) {
Object.assign(
newState,
Expand All @@ -850,8 +859,6 @@ class Grid extends React.PureComponent<Props, State> {
);
}

let {instanceProps} = prevState;

// Initially we should not clearStyleCache
newState.needToResetStyleCache = false;
if (
Expand Down Expand Up @@ -948,6 +955,8 @@ class Grid extends React.PureComponent<Props, State> {
instanceProps.prevRowHeight = nextProps.rowHeight;
instanceProps.prevScrollToColumn = nextProps.scrollToColumn;
instanceProps.prevScrollToRow = nextProps.scrollToRow;
instanceProps.prevScrollLeft = nextProps.scrollLeft;
instanceProps.prevScrollTop = nextProps.scrollTop;

// getting scrollBarSize (moved from componentWillMount)
instanceProps.scrollbarSize = nextProps.getScrollbarSize();
Expand Down