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.

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

Jump to bottom

[VirtualizedSectionList] - Externalise and handle any sort of data blob #20787

Closed
wants to merge 15 commits into from
Closed

[VirtualizedSectionList] - Externalise and handle any sort of data blob #20787

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
90baadb
Allow VirtualizedSectionList to handle any sort of data blob
magrinj Aug 22, 2018
cc92082
Externalize VirtualizedSectionList
magrinj Aug 22, 2018
66c365a
computeState use this, so keep using UNSAFE_componentWillReceiveProps
magrinj Aug 22, 2018
c00db10
Fix flow error getItem function
magrinj Aug 22, 2018
0762ef6
Fix some errors
magrinj Aug 27, 2018
b1fe9b9
Fix lint error & refreshing flow error
magrinj Aug 27, 2018
880c4dd
Small fix
magrinj Aug 28, 2018
0168f7a
Update VirtualizedSectionList-test.js
hramos Jan 15, 2019
a25a136
Fix conflicts
magrinj Jan 16, 2019
dfe4994
Remove shadow variable
magrinj Jan 16, 2019
5437f8b
Remove unused variable
magrinj Jan 16, 2019
edebcef
Remove getItemParam and replace by getItem and fix flow
magrinj Feb 27, 2019
4ff6035
Run prettier
magrinj Feb 27, 2019
32a5e4c
Merge remote-tracking branch 'upstream/master' into virtualized-secti…
magrinj Apr 18, 2019
1f00f8e
Merge branch 'master' into virtualized-section-list
magrinj Apr 18, 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
Prev Previous commit
Next Next commit
Remove getItemParam and replace by getItem and fix flow
  • Loading branch information
@magrinj
magrinj committed Feb 27, 2019
commit edebceff5f2a71e0655d2d57678b900aed04406a
7 changes: 3 additions & 4 deletions Libraries/Lists/SectionList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ type OptionalProps<SectionT: SectionBase<any>> = {

export type Props<SectionT> = RequiredProps<SectionT> &
OptionalProps<SectionT> &
VirtualizedSectionListProps<SectionT>;
VirtualizedSectionListProps;

const defaultProps = {
...VirtualizedSectionList.defaultProps,
Expand Down Expand Up @@ -335,10 +335,10 @@ class SectionList<SectionT: SectionBase<any>> extends React.PureComponent<
}

_checkProps(props: Props<SectionT>) {
const {getItem, getItemCount, getItemParam} = props;
const {getItem, getItemCount} = props;

if (__DEV__) {
if (getItem || getItemCount || getItemParam) {
if (getItem || getItemCount) {
console.warn('SectionList does not support custom data formats.');
}
}
Expand All @@ -354,7 +354,6 @@ class SectionList<SectionT: SectionBase<any>> extends React.PureComponent<
ref={this._captureRef}
getItemCount={items => items.length}
getItem={(items, index) => items[index]}
getItemParam={(item, param) => item[param]}
/>
);
}
Expand Down
25 changes: 10 additions & 15 deletions Libraries/Lists/VirtualizedSectionList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,11 @@ import type {ViewToken} from 'ViewabilityHelper';
import type {Props as VirtualizedListProps} from 'VirtualizedList';

type Item = any;
type Option = any;

type SectionBase = Object;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a regression. Could you work on putting back the proper Flow Type and running flow on the react-native repo to verify it?

Copy link
Contributor Author

@magrinj magrinj Feb 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

The SectionBase type can't be specified, because it can handle different type of datas, like classic js object or Immutable object, by exemple the data type of VirtualizedList is any.
For SectionList component SectionBase can be specified because we know the data must be javascript object.


type RequiredProps<SectionT: SectionBase> = {
sections: $ReadOnlyArray<SectionT> | $ReadOnly<SectionT>,
/**
* A generic accessor for extracting a section option from any sort of data blob.
*/
getItemParam: (section: any, name: string) => ?Option,
};

type OptionalProps<SectionT: SectionBase> = {
Expand Down Expand Up @@ -130,7 +125,7 @@ class VirtualizedSectionList<SectionT: SectionBase> extends React.PureComponent<
let index = params.itemIndex + 1;
for (let ii = 0; ii < params.sectionIndex; ii++) {
const section = this.props.getItem(this.props.sections, ii);
const sectionData = this.props.getItemParam(section, 'data');
const sectionData = this.props.getItem(section, 'data');
const dataLength = this.props.getItemCount(sectionData);
index += dataLength + 2;
}
Expand Down Expand Up @@ -159,7 +154,7 @@ class VirtualizedSectionList<SectionT: SectionBase> extends React.PureComponent<
const stickyHeaderIndices = [];
const itemCount = props.sections
? props.sections.reduce((v, section) => {
const sectionData = props.getItemParam(section, 'data');
const sectionData = props.getItem(section, 'data');

stickyHeaderIndices.push(v + offset);
return v + props.getItemCount(sectionData) + 2; // Add two for the section header and footer.
Expand Down Expand Up @@ -212,8 +207,8 @@ class VirtualizedSectionList<SectionT: SectionBase> extends React.PureComponent<
const defaultKeyExtractor = this.props.keyExtractor;
for (let ii = 0; ii < this.props.getItemCount(this.props.sections); ii++) {
const section = this.props.getItem(this.props.sections, ii);
const sectionData = this.props.getItemParam(section, 'data');
const key = this.props.getItemParam(section, 'key') || String(ii);
const sectionData = this.props.getItem(section, 'data');
const key = this.props.getItem(section, 'key') || String(ii);
itemIndex -= 1; // The section adds an item for the header
if (itemIndex >= this.props.getItemCount(sectionData) + 1) {
itemIndex -= this.props.getItemCount(sectionData) + 1; // The section adds an item for the footer.
Expand All @@ -235,7 +230,7 @@ class VirtualizedSectionList<SectionT: SectionBase> extends React.PureComponent<
};
} else {
magrinj marked this conversation as resolved.
Show resolved Hide resolved
const keyExtractor =
this.props.getItemParam(section, 'keyExtractor') ||
this.props.getItem(section, 'keyExtractor') ||
defaultKeyExtractor;
return {
section,
Expand All @@ -260,7 +255,7 @@ class VirtualizedSectionList<SectionT: SectionBase> extends React.PureComponent<
return null;
}
const keyExtractor =
this.props.getItemParam(info.section, 'keyExtractor') ||
this.props.getItem(info.section, 'keyExtractor') ||
this.props.keyExtractor;
return {
...viewable,
Expand Down Expand Up @@ -307,7 +302,7 @@ class VirtualizedSectionList<SectionT: SectionBase> extends React.PureComponent<
}
} else {
const renderItem =
this.props.getItemParam(info.section, 'renderItem') ||
this.props.getItem(info.section, 'renderItem') ||
this.props.renderItem;
const SeparatorComponent = this._getSeparatorComponent(index, info);
invariant(renderItem, 'no renderItem!');
Expand Down Expand Up @@ -350,11 +345,11 @@ class VirtualizedSectionList<SectionT: SectionBase> extends React.PureComponent<
return null;
}
const ItemSeparatorComponent =
this.props.getItemParam(info.section, 'ItemSeparatorComponent') ||
this.props.getItem(info.section, 'ItemSeparatorComponent') ||
this.props.ItemSeparatorComponent;
const {SectionSeparatorComponent} = this.props;
const isLastItemInList = index === this.state.childProps.getItemCount() - 1;
const sectionData = this.props.getItemParam(info.section, 'data');
const sectionData = this.props.getItem(info.section, 'data');
const isLastItemInSection =
info.index === this.props.getItemCount(sectionData) - 1;
if (SectionSeparatorComponent && isLastItemInSection) {
Expand Down Expand Up @@ -532,7 +527,7 @@ function getItem(
let itemIdx = index - 1;
for (let ii = 0; ii < props.getItemCount(sections); ii++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Working on this at FB. I think this is supposed to stay sections.length here because sections is always an array. I'm changing this internally, no need to do anything, just wanted to put this comment here for your information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cpojer Thanks ! In fact it can be something else, by exemple if you use an immutable list, to get his length the method is size and not length so it's better to keep it outside to handle immutable data

const section = props.getItem(props.sections, ii);
const sectionData = props.getItemParam(section, 'data');
const sectionData = props.getItem(section, 'data');
if (itemIdx === -1 || itemIdx === props.getItemCount(sectionData)) {
// We intend for there to be overflow by one on both ends of the list.
// This will be for headers and footers. When returning a header or footer
Expand Down
27 changes: 9 additions & 18 deletions Libraries/Lists/__tests__/VirtualizedSectionList-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,8 @@ describe('VirtualizedSectionList', () => {
{title: 's1', data: [{key: 'i1'}, {key: 'i2'}, {key: 'i3'}]},
]}
renderItem={({item}) => <item value={item.key} />}
getItem={(data, index) => data[index]}
getItem={(data, key) => data[key]}
getItemCount={data => data.length}
getItemParam={(data, name) => data[name]}
/>,
);
expect(component).toMatchSnapshot();
Expand All @@ -36,9 +35,8 @@ describe('VirtualizedSectionList', () => {
<VirtualizedSectionList
sections={[]}
renderItem={({item}) => <item value={item.key} />}
getItem={(data, index) => data[index]}
getItem={(data, key) => data[key]}
getItemCount={data => data.length}
getItemParam={(data, name) => data[name]}
/>,
);
expect(component).toMatchSnapshot();
Expand All @@ -49,9 +47,8 @@ describe('VirtualizedSectionList', () => {
<VirtualizedSectionList
sections={undefined}
renderItem={({item}) => <item value={item.key} />}
getItem={(data, index) => data[index]}
getItem={(data, key) => data[key]}
getItemCount={data => 0}
getItemParam={(data, name) => data[name]}
/>,
);
expect(component).toMatchSnapshot();
Expand All @@ -64,9 +61,8 @@ describe('VirtualizedSectionList', () => {
ListEmptyComponent={() => <empty />}
ListFooterComponent={() => <footer />}
ListHeaderComponent={() => <header />}
getItem={(data, index) => data[index]}
getItem={(data, key) => data[key]}
getItemCount={data => data.length}
getItemParam={(data, name) => data[name]}
renderItem={({item}) => <item value={item.key} />}
/>,
);
Expand All @@ -78,9 +74,8 @@ describe('VirtualizedSectionList', () => {
<VirtualizedSectionList
sections={[{title: 's1', data: [{key: 'hello'}]}]}
ListEmptyComponent={() => <empty />}
getItem={(data, index) => data[index]}
getItem={(data, key) => data[key]}
getItemCount={data => data.length}
getItemParam={(data, name) => data[name]}
renderItem={({item}) => <item value={item.key} />}
/>,
);
Expand All @@ -100,9 +95,8 @@ describe('VirtualizedSectionList', () => {
data: new Array(5).fill().map((_, ii) => ({id: String(ii)})),
},
]}
getItem={(data, index) => data[index]}
getItem={(data, key) => data[key]}
getItemCount={data => data.length}
getItemParam={(data, name) => data[name]}
getItemLayout={({index}) => ({length: 50, offset: index * 50})}
inverted={true}
keyExtractor={(item, index) => item.id}
Expand All @@ -126,9 +120,8 @@ describe('VirtualizedSectionList', () => {
infos.push(info);
return <item title={info.item.key} />;
}}
getItem={(data, index) => data[index]}
getItem={(data, key) => data[key]}
getItemCount={data => data.length}
getItemParam={(data, name) => data[name]}
/>,
);
expect(component).toMatchSnapshot();
Expand Down Expand Up @@ -158,14 +151,12 @@ describe('VirtualizedSectionList', () => {
renderItem={innerInfo => {
return <item title={innerInfo.item.key} />;
}}
getItem={(data, index) => data[index]}
getItem={(data, key) => data[key]}
getItemCount={data => data.length}
getItemParam={(data, name) => data[name]}
/>
)}
getItem={(data, index) => data[index]}
getItem={(data, key) => data[key]}
getItemCount={data => data.length}
getItemParam={(data, name) => data[name]}
/>,
);
expect(component).toMatchSnapshot();
Expand Down
5 changes: 0 additions & 5 deletions Libraries/Lists/__tests__/__snapshots__/SectionList-test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ exports[`SectionList rendering empty section headers is fine 1`] = `
disableVirtualization={false}
getItem={[Function]}
getItemCount={[Function]}
getItemParam={[Function]}
horizontal={false}
initialNumToRender={10}
keyExtractor={[Function]}
Expand Down Expand Up @@ -101,7 +100,6 @@ exports[`SectionList renders a footer when there is no data 1`] = `
disableVirtualization={false}
getItem={[Function]}
getItemCount={[Function]}
getItemParam={[Function]}
horizontal={false}
initialNumToRender={10}
keyExtractor={[Function]}
Expand Down Expand Up @@ -168,7 +166,6 @@ exports[`SectionList renders a footer when there is no data and no header 1`] =
disableVirtualization={false}
getItem={[Function]}
getItemCount={[Function]}
getItemParam={[Function]}
horizontal={false}
initialNumToRender={10}
keyExtractor={[Function]}
Expand Down Expand Up @@ -266,7 +263,6 @@ exports[`SectionList renders all the bells and whistles 1`] = `
disableVirtualization={false}
getItem={[Function]}
getItemCount={[Function]}
getItemParam={[Function]}
horizontal={false}
initialNumToRender={Infinity}
keyExtractor={[Function]}
Expand Down Expand Up @@ -502,7 +498,6 @@ exports[`SectionList renders empty list 1`] = `
disableVirtualization={false}
getItem={[Function]}
getItemCount={[Function]}
getItemParam={[Function]}
horizontal={false}
initialNumToRender={10}
keyExtractor={[Function]}
Expand Down
12 changes: 0 additions & 12 deletions Libraries/Lists/__tests__/__snapshots__/VirtualizedSectionList-test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ exports[`VirtualizedSectionList handles nested lists 1`] = `
disableVirtualization={false}
getItem={[Function]}
getItemCount={[Function]}
getItemParam={[Function]}
horizontal={false}
initialNumToRender={10}
keyExtractor={[Function]}
Expand Down Expand Up @@ -81,7 +80,6 @@ exports[`VirtualizedSectionList handles nested lists 1`] = `
disableVirtualization={false}
getItem={[Function]}
getItemCount={[Function]}
getItemParam={[Function]}
horizontal={false}
initialNumToRender={10}
keyExtractor={[Function]}
Expand Down Expand Up @@ -163,7 +161,6 @@ exports[`VirtualizedSectionList handles nested lists 1`] = `
disableVirtualization={false}
getItem={[Function]}
getItemCount={[Function]}
getItemParam={[Function]}
horizontal={true}
initialNumToRender={10}
keyExtractor={[Function]}
Expand Down Expand Up @@ -283,7 +280,6 @@ exports[`VirtualizedSectionList handles separators correctly 1`] = `
disableVirtualization={false}
getItem={[Function]}
getItemCount={[Function]}
getItemParam={[Function]}
horizontal={false}
initialNumToRender={10}
keyExtractor={[Function]}
Expand Down Expand Up @@ -441,7 +437,6 @@ exports[`VirtualizedSectionList handles separators correctly 2`] = `
disableVirtualization={false}
getItem={[Function]}
getItemCount={[Function]}
getItemParam={[Function]}
horizontal={false}
initialNumToRender={10}
keyExtractor={[Function]}
Expand Down Expand Up @@ -599,7 +594,6 @@ exports[`VirtualizedSectionList handles separators correctly 3`] = `
disableVirtualization={false}
getItem={[Function]}
getItemCount={[Function]}
getItemParam={[Function]}
horizontal={false}
initialNumToRender={10}
keyExtractor={[Function]}
Expand Down Expand Up @@ -767,7 +761,6 @@ exports[`VirtualizedSectionList renders all the bells and whistles 1`] = `
getItem={[Function]}
getItemCount={[Function]}
getItemLayout={[Function]}
getItemParam={[Function]}
horizontal={false}
initialNumToRender={10}
invertStickyHeaders={true}
Expand Down Expand Up @@ -1015,7 +1008,6 @@ exports[`VirtualizedSectionList renders empty list 1`] = `
disableVirtualization={false}
getItem={[Function]}
getItemCount={[Function]}
getItemParam={[Function]}
horizontal={false}
initialNumToRender={10}
keyExtractor={[Function]}
Expand Down Expand Up @@ -1047,7 +1039,6 @@ exports[`VirtualizedSectionList renders empty list with empty component 1`] = `
disableVirtualization={false}
getItem={[Function]}
getItemCount={[Function]}
getItemParam={[Function]}
horizontal={false}
initialNumToRender={10}
keyExtractor={[Function]}
Expand Down Expand Up @@ -1100,7 +1091,6 @@ exports[`VirtualizedSectionList renders list with empty component 1`] = `
disableVirtualization={false}
getItem={[Function]}
getItemCount={[Function]}
getItemParam={[Function]}
horizontal={false}
initialNumToRender={10}
keyExtractor={[Function]}
Expand Down Expand Up @@ -1156,7 +1146,6 @@ exports[`VirtualizedSectionList renders null list 1`] = `
disableVirtualization={false}
getItem={[Function]}
getItemCount={[Function]}
getItemParam={[Function]}
horizontal={false}
initialNumToRender={10}
keyExtractor={[Function]}
Expand Down Expand Up @@ -1201,7 +1190,6 @@ exports[`VirtualizedSectionList renders simple list 1`] = `
disableVirtualization={false}
getItem={[Function]}
getItemCount={[Function]}
getItemParam={[Function]}
horizontal={false}
initialNumToRender={10}
keyExtractor={[Function]}
Expand Down