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

add shouldForceUpdate to resetAfterIndex #32

Merged
merged 4 commits into from
Aug 4, 2018
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions src/VariableSizeGrid.js
Original file line number Diff line number Diff line change
Expand Up @@ -396,20 +396,28 @@ const VariableSizeGrid = createGridComponent({
rowMetadataMap: {},
};

instance.resetAfterColumnIndex = (columnIndex: number) => {
this.resetAfterIndices({ columnIndex });
instance.resetAfterColumnIndex = (
columnIndex: number,
shouldForceUpdate?: boolean = true
) => {
this.resetAfterIndices({ columnIndex, shouldForceUpdate });
};

instance.resetAfterRowIndex = (rowIndex: number) => {
this.resetAfterIndices({ rowIndex });
instance.resetAfterRowIndex = (
rowIndex: number,
shouldForceUpdate?: boolean = true
) => {
this.resetAfterIndices({ rowIndex, shouldForceUpdate });
};

instance.resetAfterIndices = ({
columnIndex,
rowIndex,
shouldForceUpdate = true,
}: {
columnIndex?: number,
rowIndex?: number,
shouldForceUpdate: boolean,
}) => {
if (typeof columnIndex === 'number') {
instanceProps.lastMeasuredColumnIndex = Math.min(
Expand All @@ -429,7 +437,10 @@ const VariableSizeGrid = createGridComponent({
// It seems an unnecessary optimization.
// It's unlikely that resetAfterIndex() will be called while a user is scrolling.
instance._itemStyleCache = {};
instance.forceUpdate();

if (shouldForceUpdate) {
instance.forceUpdate();
}
};

return instanceProps;
Expand Down
10 changes: 8 additions & 2 deletions src/VariableSizeList.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,10 @@ const VariableSizeList = createListComponent({
lastMeasuredIndex: -1,
};

instance.resetAfterIndex = (index: number) => {
instance.resetAfterIndex = (
index: number,
shouldForceUpdate?: boolean = true
) => {
instanceProps.lastMeasuredIndex = Math.min(
instanceProps.lastMeasuredIndex,
index - 1
Expand All @@ -262,7 +265,10 @@ const VariableSizeList = createListComponent({
// It seems an unnecessary optimization.
// It's unlikely that resetAfterIndex() will be called while a user is scrolling.
instance._itemStyleCache = {};
instance.forceUpdate();

if (shouldForceUpdate) {
instance.forceUpdate();
}
};

return instanceProps;
Expand Down
57 changes: 57 additions & 0 deletions src/__tests__/VariableSizeGrid.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,63 @@ describe('VariableSizeGrid', () => {
expect(scrollContainer.props.style.width).toEqual(625);
});

it('should delay the recalculation of the estimated total size if shouldForceUpdate is false', () => {
const rendered = ReactTestRenderer.create(
<VariableSizeGrid
{...defaultProps}
estimatedColumnWidth={30}
estimatedRowHeight={30}
overscanCount={1}
columnWidth={index => 50}
rowHeight={index => 25}
/>
);
const scrollContainer = findScrollContainer(rendered);
// The estimated total height should be (100 + 25 * 1 + 30 * 15)px = 575px.
// The estimated total width should be (200 + 50 * 1 + 30 * 5)px = 400px.
expect(scrollContainer.props.style.height).toEqual(575);
expect(scrollContainer.props.style.width).toEqual(400);
// Supplying new item sizes alone should not impact anything.
// Although the grid get re-rendered by passing inline functions,
// but it still use the cached metrics to calculate the estimated size.
rendered.update(
<VariableSizeGrid
{...defaultProps}
estimatedColumnWidth={30}
estimatedRowHeight={30}
overscanCount={1}
columnWidth={index => 40}
rowHeight={index => 20}
/>
);
expect(scrollContainer.props.style.height).toEqual(575);
expect(scrollContainer.props.style.width).toEqual(400);
// Reset calculation cache but don't re-render the grid,
// the estimated total size should stay the same.
rendered.getInstance().resetAfterIndices({
columnIndex: 0,
rowIndex: 0,
shouldForceUpdate: false,
});
expect(scrollContainer.props.style.height).toEqual(575);
expect(scrollContainer.props.style.width).toEqual(400);
// Pass inline function to make the grid re-render.
rendered.update(
<VariableSizeGrid
{...defaultProps}
estimatedColumnWidth={30}
estimatedRowHeight={30}
overscanCount={1}
columnWidth={index => 40}
rowHeight={index => 20}
/>
);
// The estimated total height should be (100 + 20 * 1 + 30 * 14)px = 540px.
// The estimated total width should be (200 + 40 * 1 + 30 * 4)px = 360px.
expect(scrollContainer.props.style.height).toEqual(540);
expect(scrollContainer.props.style.width).toEqual(360);
});
Copy link
Owner

Choose a reason for hiding this comment

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

This is a much better test! Thank you!


it('should re-render items after the specified index with updated styles', () => {
const columnWidth = jest.fn(() => 75);
const rowHeight = jest.fn(() => 35);
Expand Down
41 changes: 41 additions & 0 deletions src/__tests__/VariableSizeList.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,47 @@ describe('VariableSizeList', () => {
expect(scrollContainer.props.style.height).toEqual(750);
});

it('should delay the recalculation of the estimated total size if shouldForceUpdate is false', () => {
const rendered = ReactTestRenderer.create(
<VariableSizeList
{...defaultProps}
estimatedItemSize={30}
overscanCount={1}
itemSize={index => 25}
/>
);
const scrollContainer = findScrollContainer(rendered);
// The estimated total size should be (100 + 25 * 1 + 30 * 15)px = 575px.
expect(scrollContainer.props.style.height).toEqual(575);
// Supplying a new itemSize alone should not impact anything.
// Although the list get re-rendered by passing inline functions,
// but it still use the cached metrics to calculate the estimated size.
rendered.update(
<VariableSizeList
{...defaultProps}
estimatedItemSize={30}
overscanCount={1}
itemSize={index => 20}
/>
);
expect(scrollContainer.props.style.height).toEqual(575);
// Reset calculation cache but don't re-render the list,
// the estimated total size should stay the same.
rendered.getInstance().resetAfterIndex(0, false);
expect(scrollContainer.props.style.height).toEqual(575);
// Pass inline function to make the list re-render.
rendered.update(
<VariableSizeList
{...defaultProps}
estimatedItemSize={30}
overscanCount={1}
itemSize={index => 20}
/>
);
// The estimated total height should be (100 + 20 * 1 + 30 * 14)px = 540px.
expect(scrollContainer.props.style.height).toEqual(540);
});

it('should re-render items after the specified index with updated styles', () => {
const itemSize = jest.fn(() => 75);
const rendered = ReactTestRenderer.create(
Expand Down
74 changes: 50 additions & 24 deletions website/src/routes/api/VariableSizeGrid.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,39 +96,65 @@ const PROPS = [
const METHODS = [
{
description: (
<p>
<code>VariableSizeGrid</code> caches offsets and measurements for each
column index for performance purposes. This method clears that cached
data for all columns after (and including) the specified index. It
should be called whenever a column's width changes. (Note that this is
not a typical occurrance.)
</p>
<Fragment>
<p>
<code>VariableSizeGrid</code> caches offsets and measurements for each
column index for performance purposes. This method clears that cached
data for all columns after (and including) the specified index. It
should be called whenever a column's width changes. (Note that this is
not a typical occurrance.)
</p>
<p>
By default the grid will automatically re-render after the index is
reset. If you would like to delay this re-render until e.g. a state
update has completed in the parent component, specify a value of
<code>false</code> for the second, optional parameter.
</p>
</Fragment>
),
signature: 'resetAfterColumnIndex(index: number): void',
signature:
'resetAfterColumnIndex(index: number, shouldForceUpdate: boolean = true): void',
},
{
description: (
<p>
<code>VariableSizeGrid</code> caches offsets and measurements for each
item for performance purposes. This method clears that cached data for
all items after (and including) the specified indices. It should be
called whenever an items size changes. (Note that this is not a typical
occurrance.)
</p>
<Fragment>
<p>
<code>VariableSizeGrid</code> caches offsets and measurements for each
item for performance purposes. This method clears that cached data for
all items after (and including) the specified indices. It should be
called whenever an items size changes. (Note that this is not a
typical occurrance.)
</p>
<p>
By default the grid will automatically re-render after the index is
reset. If you would like to delay this re-render until e.g. a state
update has completed in the parent component, specify a value of
<code>false</code> for the optional <code>shouldForceUpdate</code> parameter.
</p>
</Fragment>
),
signature:
'resetAfterIndices({ columnIndex: number, rowIndex: number }): void',
'resetAfterIndices({ columnIndex: number, rowIndex: number, shouldForceUpdate: boolean = true }): void',
},
{
description: (
<p>
<code>VariableSizeGrid</code> caches offsets and measurements for each
row index for performance purposes. This method clears that cached data
for all rows after (and including) the specified index. It should be
called whenever a row's height changes. (Note that this is not a typical
occurrance.)
</p>
<Fragment>
<p>
<code>VariableSizeGrid</code> caches offsets and measurements for each
row index for performance purposes. This method clears that cached
data for all rows after (and including) the specified index. It should
be called whenever a row's height changes. (Note that this is not a
typical occurrance.)
</p>
<p>
By default the grid will automatically re-render after the index is
reset. If you would like to delay this re-render until e.g. a state
update has completed in the parent component, specify a value of
<code>false</code> for the second, optional parameter.
</p>
</Fragment>
),
signature: 'resetAfterRowIndex(index: number): void',
signature:
'resetAfterRowIndex(index: number, shouldForceUpdate: boolean = true): void',
},
];
26 changes: 18 additions & 8 deletions website/src/routes/api/VariableSizeList.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,24 @@ const PROPS = [
const METHODS = [
{
description: (
<p>
<code>VariableSizeList</code> caches offsets and measurements for each
index for performance purposes. This method clears that cached data for
all items after (and including) the specified index. It should be called
whenever a item's size changes. (Note that this is not a typical
occurrance.)
</p>
<Fragment>
<p>
<code>VariableSizeList</code> caches offsets and measurements for each
index for performance purposes. This method clears that cached data
for all items after (and including) the specified index. It should be
called whenever a item's size changes. (Note that this is not a
typical occurrance.)
</p>
<p>
By default the list will automatically re-render after the index is
reset. If you would like to delay this re-render until e.g. a state
update has completed in the parent component, specify a value of
<code>false</code>
for the second, optional parameter.
</p>
</Fragment>
),
signature: 'resetAfterIndex(index: number): void',
signature:
'resetAfterIndex(index: number, shouldForceUpdate: boolean = true): void',
},
];