From 4303e0ff2e3a3bbfd87ba9bd6fc7cf436dd593d7 Mon Sep 17 00:00:00 2001 From: MARKFalconbridge Date: Fri, 14 Jun 2019 09:25:18 +0100 Subject: [PATCH 1/2] Prevent toggling between min and max scroll offsets at end of grid --- src/FixedSizeGrid.js | 4 ++ src/VariableSizeGrid.js | 2 + src/__tests__/FixedSizeGrid.js | 75 +++++++++++++++++++++++++++++++ src/__tests__/VariableSizeGrid.js | 70 +++++++++++++++++++++++++++++ 4 files changed, 151 insertions(+) diff --git a/src/FixedSizeGrid.js b/src/FixedSizeGrid.js index 452536a9..ae65992e 100644 --- a/src/FixedSizeGrid.js +++ b/src/FixedSizeGrid.js @@ -65,6 +65,8 @@ const FixedSizeGrid = createGridComponent({ default: if (scrollLeft >= minOffset && scrollLeft <= maxOffset) { return scrollLeft; + } else if (minOffset > maxOffset) { + return minOffset; } else if (scrollLeft - minOffset < maxOffset - scrollLeft) { return minOffset; } else { @@ -115,6 +117,8 @@ const FixedSizeGrid = createGridComponent({ default: if (scrollTop >= minOffset && scrollTop <= maxOffset) { return scrollTop; + } else if (minOffset > maxOffset) { + return minOffset; } else if (scrollTop - minOffset < maxOffset - scrollTop) { return minOffset; } else { diff --git a/src/VariableSizeGrid.js b/src/VariableSizeGrid.js index a1273e19..bd82adb1 100644 --- a/src/VariableSizeGrid.js +++ b/src/VariableSizeGrid.js @@ -274,6 +274,8 @@ const getOffsetForIndexAndAlignment = ( default: if (scrollOffset >= minOffset && scrollOffset <= maxOffset) { return scrollOffset; + } else if (minOffset > maxOffset) { + return minOffset; } else if (scrollOffset - minOffset < maxOffset - scrollOffset) { return minOffset; } else { diff --git a/src/__tests__/FixedSizeGrid.js b/src/__tests__/FixedSizeGrid.js index a214bd0f..05c3498e 100644 --- a/src/__tests__/FixedSizeGrid.js +++ b/src/__tests__/FixedSizeGrid.js @@ -569,6 +569,81 @@ describe('FixedSizeGrid', () => { expect(onItemsRendered.mock.calls).toMatchSnapshot(); }); + it('should scroll to the correct item for align = "auto" at the bottom of the grid', () => { + getScrollbarSize.mockImplementation(() => 20); + + const rendered = ReactTestRenderer.create( + + ); + onItemsRendered.mockClear(); + + // Scroll down to the last row in the list. + rendered + .getInstance() + .scrollToItem({ columnIndex: 5, rowIndex: 19, align: 'auto' }); + + expect(onItemsRendered).toHaveBeenCalledTimes(1); + expect(onItemsRendered).toHaveBeenLastCalledWith( + expect.objectContaining({ + visibleRowStartIndex: 17, + visibleRowStopIndex: 19, + }) + ); + // Repeat the previous scrollToItem call. + rendered + .getInstance() + .scrollToItem({ columnIndex: 5, rowIndex: 19, align: 'auto' }); + + // Shouldn't have been called again + expect(onItemsRendered).toHaveBeenCalledTimes(1); + expect(onItemsRendered).toHaveBeenLastCalledWith( + expect.objectContaining({ + visibleRowStartIndex: 17, + visibleRowStopIndex: 19, + }) + ); + }); + + it('should scroll to the correct item for align = "auto" at the end of the grid', () => { + getScrollbarSize.mockImplementation(() => 20); + + const rendered = ReactTestRenderer.create( + + ); + onItemsRendered.mockClear(); + + // Scroll across to the last row in the list. + rendered + .getInstance() + .scrollToItem({ columnIndex: 19, rowIndex: 19, align: 'auto' }); + + expect(onItemsRendered).toHaveBeenCalledTimes(1); + expect(onItemsRendered).toHaveBeenLastCalledWith( + expect.objectContaining({ + visibleColumnStartIndex: 18, + visibleColumnStopIndex: 19, + }) + ); + // Repeat the previous scrollToItem call. + rendered + .getInstance() + .scrollToItem({ columnIndex: 19, rowIndex: 19, align: 'auto' }); + + // Shouldn't have been called again + expect(onItemsRendered).toHaveBeenCalledTimes(1); + expect(onItemsRendered).toHaveBeenLastCalledWith( + expect.objectContaining({ + visibleColumnStartIndex: 18, + visibleColumnStopIndex: 19, + }) + ); + }); + it('should scroll to the correct item for align = "start"', () => { const rendered = ReactTestRenderer.create( diff --git a/src/__tests__/VariableSizeGrid.js b/src/__tests__/VariableSizeGrid.js index 871321cf..2c4d66fd 100644 --- a/src/__tests__/VariableSizeGrid.js +++ b/src/__tests__/VariableSizeGrid.js @@ -189,6 +189,76 @@ describe('VariableSizeGrid', () => { expect(onItemsRendered.mock.calls).toMatchSnapshot(); }); + it('should scroll to the correct item for align = "auto" at the bottom of the grid', () => { + getScrollbarSize.mockImplementation(() => 20); + + const rendered = ReactTestRenderer.create( + + ); + onItemsRendered.mockClear(); + + // Scroll down to the last row in the list. + rendered + .getInstance() + .scrollToItem({ columnIndex: 5, rowIndex: 19, align: 'auto' }); + + expect(onItemsRendered).toHaveBeenCalledTimes(1); + expect(onItemsRendered).toHaveBeenLastCalledWith( + expect.objectContaining({ + visibleRowStartIndex: 18, + visibleRowStopIndex: 19, + }) + ); + // Repeat the previous scrollToItem call. + rendered + .getInstance() + .scrollToItem({ columnIndex: 5, rowIndex: 19, align: 'auto' }); + + // Shouldn't have been called again + expect(onItemsRendered).toHaveBeenCalledTimes(1); + expect(onItemsRendered).toHaveBeenLastCalledWith( + expect.objectContaining({ + visibleRowStartIndex: 18, + visibleRowStopIndex: 19, + }) + ); + }); + + it('should scroll to the correct item for align = "auto" at the end of the grid', () => { + getScrollbarSize.mockImplementation(() => 20); + + const rendered = ReactTestRenderer.create( + + ); + onItemsRendered.mockClear(); + + // Scroll scross to the last row in the list. + rendered + .getInstance() + .scrollToItem({ columnIndex: 9, rowIndex: 10, align: 'auto' }); + + expect(onItemsRendered).toHaveBeenCalledTimes(1); + expect(onItemsRendered).toHaveBeenLastCalledWith( + expect.objectContaining({ + visibleColumnStartIndex: 8, + visibleColumnStopIndex: 9, + }) + ); + // Repeat the previous scrollToItem call. + rendered + .getInstance() + .scrollToItem({ columnIndex: 9, rowIndex: 10, align: 'auto' }); + + // Shouldn't have been called again + expect(onItemsRendered).toHaveBeenCalledTimes(1); + expect(onItemsRendered).toHaveBeenLastCalledWith( + expect.objectContaining({ + visibleColumnStartIndex: 8, + visibleColumnStopIndex: 9, + }) + ); + }); + it('should scroll to the correct item for align = "start"', () => { const rendered = ReactTestRenderer.create( From 4cd1d72256f3039206336639f5cd0b2e46ff20e7 Mon Sep 17 00:00:00 2001 From: MARKFalconbridge Date: Mon, 17 Jun 2019 08:37:47 +0100 Subject: [PATCH 2/2] Add comment --- src/FixedSizeGrid.js | 2 ++ src/VariableSizeGrid.js | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/FixedSizeGrid.js b/src/FixedSizeGrid.js index ae65992e..69f9bd0e 100644 --- a/src/FixedSizeGrid.js +++ b/src/FixedSizeGrid.js @@ -117,6 +117,8 @@ const FixedSizeGrid = createGridComponent({ default: if (scrollTop >= minOffset && scrollTop <= maxOffset) { return scrollTop; + // Because we only take into account the scrollbar size when calculating minOffset + // this value can be larger than maxOffset when at the end of the list } else if (minOffset > maxOffset) { return minOffset; } else if (scrollTop - minOffset < maxOffset - scrollTop) { diff --git a/src/VariableSizeGrid.js b/src/VariableSizeGrid.js index bd82adb1..11f5eb01 100644 --- a/src/VariableSizeGrid.js +++ b/src/VariableSizeGrid.js @@ -274,6 +274,8 @@ const getOffsetForIndexAndAlignment = ( default: if (scrollOffset >= minOffset && scrollOffset <= maxOffset) { return scrollOffset; + // Because we only take into account the scrollbar size when calculating minOffset + // this value can be larger than maxOffset when at the end of the list } else if (minOffset > maxOffset) { return minOffset; } else if (scrollOffset - minOffset < maxOffset - scrollOffset) {