Skip to content

rename overscan props for Gird with deprecation #229

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

Merged
bvaughn merged 2 commits into from
May 27, 2019
Merged

rename overscan props for Gird with deprecation #229

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
481f90d
rename overscan props for Gird with deprecation
nihgwu Apr 30, 2019
5d7fb35
address comments
nihgwu May 1, 2019
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
66 changes: 52 additions & 14 deletions src/__tests__/FixedSizeGrid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,15 +211,15 @@ describe('FixedSizeGrid', () => {
});
});

describe('overscanColumnsCount and overscanRowsCount', () => {
describe('overscanColumnCount and overscanRowCount', () => {
it('should require a minimum of 1 overscan to support tabbing', () => {
ReactTestRenderer.create(
<FixedSizeGrid
{...defaultProps}
initialScrollLeft={250}
initialScrollTop={250}
overscanColumnsCount={0}
overscanRowsCount={0}
overscanColumnCount={0}
overscanRowCount={0}
/>
);
expect(onItemsRendered.mock.calls).toMatchSnapshot();
Expand All @@ -231,8 +231,8 @@ describe('FixedSizeGrid', () => {
{...defaultProps}
initialScrollLeft={250}
initialScrollTop={250}
overscanColumnsCount={2}
overscanRowsCount={2}
overscanColumnCount={2}
overscanRowCount={2}
/>
);
rendered.getInstance().scrollTo({ scrollLeft: 1000, scrollTop: 1000 });
Expand All @@ -246,8 +246,8 @@ describe('FixedSizeGrid', () => {
{...defaultProps}
initialScrollLeft={250}
initialScrollTop={250}
overscanColumnsCount={2}
overscanRowsCount={2}
overscanColumnCount={2}
overscanRowCount={2}
/>
);
expect(onItemsRendered.mock.calls).toMatchSnapshot();
Expand All @@ -259,8 +259,8 @@ describe('FixedSizeGrid', () => {
{...defaultProps}
initialScrollLeft={250}
initialScrollTop={250}
overscanColumnsCount={2}
overscanRowsCount={2}
overscanColumnCount={2}
overscanRowCount={2}
/>
);
expect(onItemsRendered.mock.calls).toMatchSnapshot();
Expand Down Expand Up @@ -289,12 +289,17 @@ describe('FixedSizeGrid', () => {
spyOn(console, 'warn');

const renderer = ReactTestRenderer.create(
<FixedSizeGrid {...defaultProps} overscanCount={1} />
<FixedSizeGrid
{...defaultProps}
overscanCount={1}
overscanRowsCount={1}
overscanColumnsCount={1}
/>
);
expect(console.warn).toHaveBeenCalledTimes(1);
expect(console.warn).toHaveBeenLastCalledWith(
'The overscanCount prop has been deprecated. ' +
'Please use the overscanColumnsCount and overscanRowsCount props instead.'
'The overscanCount, overscanColumnsCount and overscanRowsCount props have been deprecated. ' +
Copy link
Owner

@bvaughn bvaughn Apr 30, 2019

Choose a reason for hiding this comment

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

I think this message may be a bit confusing for people who were unaware of the old overscanCount prop. Seems like it would be better to split these into two warnings?

Copy link
Contributor Author

@nihgwu nihgwu May 1, 2019

Choose a reason for hiding this comment

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

OK, will do

'Please use the overscanColumnCount and overscanRowCount props instead.'
);

renderer.update(<FixedSizeGrid {...defaultProps} overscanCount={1} />);
Expand All @@ -318,7 +323,7 @@ describe('FixedSizeGrid', () => {
expect(onItemsRendered.mock.calls).toMatchSnapshot();
});

it('should use overscanRowsCount if both it and overscanCount are provided', () => {
it('should use overscanRowCount if both it and overscanCount are provided', () => {
spyOn(console, 'warn');

ReactTestRenderer.create(
Expand All @@ -327,7 +332,25 @@ describe('FixedSizeGrid', () => {
initialScrollLeft={100}
initialScrollTop={100}
overscanCount={2}
overscanRowsCount={3}
overscanRowCount={3}
/>
);
expect(onItemsRendered.mock.calls).toMatchSnapshot();
});

it('should use overscanColumnCount and overscanRowCount if both them and deprecated props are provided', () => {
spyOn(console, 'warn');

ReactTestRenderer.create(
<FixedSizeGrid
{...defaultProps}
initialScrollLeft={100}
initialScrollTop={100}
overscanCount={1}
overscanColumnsCount={2}
overscanColumnCount={3}
overscanRowsCount={2}
overscanRowCount={3}
/>
);
expect(onItemsRendered.mock.calls).toMatchSnapshot();
Expand All @@ -346,6 +369,21 @@ describe('FixedSizeGrid', () => {
);
expect(onItemsRendered.mock.calls).toMatchSnapshot();
});

it('should support deprecated overscanColumnsCount and overscanRowsCount', () => {
spyOn(console, 'warn');

ReactTestRenderer.create(
<FixedSizeGrid
{...defaultProps}
initialScrollLeft={100}
initialScrollTop={100}
overscanColumnsCount={2}
overscanRowsCount={2}
/>
);
expect(onItemsRendered.mock.calls).toMatchSnapshot();
});
});
});

Expand Down
20 changes: 10 additions & 10 deletions src/__tests__/VariableSizeGrid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ describe('VariableSizeGrid', () => {
columnWidth={columnWidth}
estimatedColumnWidth={200}
estimatedRowHeight={100}
overscanColumnsCount={0}
overscanRowsCount={0}
overscanColumnCount={0}
overscanRowCount={0}
rowCount={50}
rowHeight={rowHeight}
/>
Expand All @@ -131,8 +131,8 @@ describe('VariableSizeGrid', () => {
columnWidth={columnWidth}
estimatedColumnWidth={200}
estimatedRowHeight={100}
overscanColumnsCount={0}
overscanRowsCount={0}
overscanColumnCount={0}
overscanRowCount={0}
rowCount={50}
rowHeight={rowHeight}
/>
Expand Down Expand Up @@ -439,8 +439,8 @@ describe('VariableSizeGrid', () => {
{...defaultProps}
estimatedColumnWidth={30}
estimatedRowHeight={30}
overscanColumnsCount={1}
overscanRowsCount={1}
overscanColumnCount={1}
overscanRowCount={1}
columnWidth={index => 50}
rowHeight={index => 25}
/>
Expand All @@ -458,8 +458,8 @@ describe('VariableSizeGrid', () => {
{...defaultProps}
estimatedColumnWidth={30}
estimatedRowHeight={30}
overscanColumnsCount={1}
overscanRowsCount={1}
overscanColumnCount={1}
overscanRowCount={1}
columnWidth={index => 40}
rowHeight={index => 20}
/>
Expand All @@ -481,8 +481,8 @@ describe('VariableSizeGrid', () => {
{...defaultProps}
estimatedColumnWidth={30}
estimatedRowHeight={30}
overscanColumnsCount={1}
overscanRowsCount={1}
overscanColumnCount={1}
overscanRowCount={1}
columnWidth={index => 40}
rowHeight={index => 20}
/>
Expand Down
52 changes: 43 additions & 9 deletions src/__tests__/__snapshots__/FixedSizeGrid.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ Array [
]
`;

exports[`FixedSizeGrid overscanColumnsCount and overscanRowsCount overscanCount should support deprecated overscanCount 1`] = `
exports[`FixedSizeGrid overscanColumnCount and overscanRowCount overscanCount should support deprecated overscanColumnsCount and overscanRowsCount 1`] = `
Array [
Array [
Object {
Expand All @@ -113,7 +113,41 @@ Array [
]
`;

exports[`FixedSizeGrid overscanColumnsCount and overscanRowsCount overscanCount should use overscanColumnsCount if both it and overscanCount are provided 1`] = `
exports[`FixedSizeGrid overscanColumnCount and overscanRowCount overscanCount should support deprecated overscanCount 1`] = `
Array [
Array [
Object {
"overscanColumnStartIndex": 0,
"overscanColumnStopIndex": 5,
"overscanRowStartIndex": 2,
"overscanRowStopIndex": 10,
"visibleColumnStartIndex": 1,
"visibleColumnStopIndex": 3,
"visibleRowStartIndex": 4,
"visibleRowStopIndex": 8,
},
],
]
`;

exports[`FixedSizeGrid overscanColumnCount and overscanRowCount overscanCount should use overscanColumnCount and overscanRowCount if both them and deprecated props are provided 1`] = `
Array [
Array [
Object {
"overscanColumnStartIndex": 0,
"overscanColumnStopIndex": 6,
"overscanRowStartIndex": 1,
"overscanRowStopIndex": 11,
"visibleColumnStartIndex": 1,
"visibleColumnStopIndex": 3,
"visibleRowStartIndex": 4,
"visibleRowStopIndex": 8,
},
],
]
`;

exports[`FixedSizeGrid overscanColumnCount and overscanRowCount overscanCount should use overscanColumnsCount if both it and overscanCount are provided 1`] = `
Array [
Array [
Object {
Expand All @@ -130,7 +164,7 @@ Array [
]
`;

exports[`FixedSizeGrid overscanColumnsCount and overscanRowsCount overscanCount should use overscanRowsCount if both it and overscanCount are provided 1`] = `
exports[`FixedSizeGrid overscanColumnCount and overscanRowCount overscanCount should use overscanRowCount if both it and overscanCount are provided 1`] = `
Array [
Array [
Object {
Expand All @@ -147,7 +181,7 @@ Array [
]
`;

exports[`FixedSizeGrid overscanColumnsCount and overscanRowsCount should accommodate a custom overscan 1`] = `
exports[`FixedSizeGrid overscanColumnCount and overscanRowCount should accommodate a custom overscan 1`] = `
Array [
Array [
Object {
Expand All @@ -164,7 +198,7 @@ Array [
]
`;

exports[`FixedSizeGrid overscanColumnsCount and overscanRowsCount should not scan past the beginning of the grid 1`] = `
exports[`FixedSizeGrid overscanColumnCount and overscanRowCount should not scan past the beginning of the grid 1`] = `
Array [
Array [
Object {
Expand All @@ -181,7 +215,7 @@ Array [
]
`;

exports[`FixedSizeGrid overscanColumnsCount and overscanRowsCount should not scan past the end of the grid 1`] = `
exports[`FixedSizeGrid overscanColumnCount and overscanRowCount should not scan past the end of the grid 1`] = `
Array [
Array [
Object {
Expand All @@ -198,7 +232,7 @@ Array [
]
`;

exports[`FixedSizeGrid overscanColumnsCount and overscanRowsCount should overscan in both directions when not scrolling 1`] = `
exports[`FixedSizeGrid overscanColumnCount and overscanRowCount should overscan in both directions when not scrolling 1`] = `
Array [
Array [
Object {
Expand All @@ -215,7 +249,7 @@ Array [
]
`;

exports[`FixedSizeGrid overscanColumnsCount and overscanRowsCount should overscan in the direction being scrolled 1`] = `
exports[`FixedSizeGrid overscanColumnCount and overscanRowCount should overscan in the direction being scrolled 1`] = `
Array [
Array [
Object {
Expand Down Expand Up @@ -256,7 +290,7 @@ Array [
]
`;

exports[`FixedSizeGrid overscanColumnsCount and overscanRowsCount should require a minimum of 1 overscan to support tabbing 1`] = `
exports[`FixedSizeGrid overscanColumnCount and overscanRowCount should require a minimum of 1 overscan to support tabbing 1`] = `
Array [
Array [
Object {
Expand Down
22 changes: 15 additions & 7 deletions src/createGridComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,11 @@ export type Props<T> = {|
outerRef?: any,
outerElementType?: React$ElementType,
outerTagName?: string, // deprecated
overscanColumnsCount?: number,
overscanColumnCount?: number,
overscanColumnsCount?: number, // deprecated
overscanCount?: number, // deprecated
overscanRowsCount?: number,
overscanRowCount?: number,
overscanRowsCount?: number, // deprecated
rowCount: number,
rowHeight: itemSize,
style?: Object,
Expand Down Expand Up @@ -586,14 +588,15 @@ export default function createGridComponent({
_getHorizontalRangeToRender(): [number, number, number, number] {
const {
columnCount,
overscanColumnCount,
overscanColumnsCount,
overscanCount,
rowCount,
} = this.props;
const { horizontalScrollDirection, isScrolling, scrollLeft } = this.state;

const overscanCountResolved: number =
overscanColumnsCount || overscanCount || 1;
overscanColumnCount || overscanColumnsCount || overscanCount || 1;

if (columnCount === 0 || rowCount === 0) {
return [0, 0, 0, 0];
Expand Down Expand Up @@ -634,13 +637,14 @@ export default function createGridComponent({
const {
columnCount,
overscanCount,
overscanRowCount,
overscanRowsCount,
rowCount,
} = this.props;
const { isScrolling, verticalScrollDirection, scrollTop } = this.state;

const overscanCountResolved: number =
overscanRowsCount || overscanCount || 1;
overscanRowCount || overscanRowsCount || overscanCount || 1;

if (columnCount === 0 || rowCount === 0) {
return [0, 0, 0, 0];
Expand Down Expand Up @@ -774,12 +778,16 @@ const validateSharedProps = (
{ instance }: State
): void => {
if (process.env.NODE_ENV !== 'production') {
if (typeof overscanCount === 'number') {
if (
typeof overscanCount === 'number' ||
typeof overscanColumnsCount === 'number' ||
typeof overscanRowsCount === 'number'
) {
if (devWarningsOverscanCount && !devWarningsOverscanCount.has(instance)) {
devWarningsOverscanCount.add(instance);
console.warn(
'The overscanCount prop has been deprecated. ' +
'Please use the overscanColumnsCount and overscanRowsCount props instead.'
'The overscanCount, overscanColumnsCount and overscanRowsCount props have been deprecated. ' +
'Please use the overscanColumnCount and overscanRowCount props instead.'
);
}
}
Expand Down
Loading