From f702cbecba6248394f9bb660aa834c1b27f3bedd Mon Sep 17 00:00:00 2001 From: Caleb Meredith Date: Thu, 25 May 2017 10:18:48 -0700 Subject: [PATCH] Render section footer in sections with no data Summary: Fixes https://github.com/facebook/react-native/issues/13784 The section footer was only rendered with the last item of the section. However, that meant in sections where no items were rendered, no section footer would be rendered. This patch makes sure that when there are no items the section footer is rendered with the section header in addition to adding tests asserting the existance of section footers in empty lists. One potential point of contention is whether or not a section separator (as defined by the `SectionSeparatorComponent` prop to ``) should be rendered in an empty list. I did not include a section separator for empty lists, but let me know if you think one should be included. See the test plan below for an image of an empty section rendered without a section separator. I was also running into a lint error, `no-alert`, in `SectionListExample.js` around line 135 that blocked me from publishing. This error looks to be triggered when the `alert()` global function is called, so to fix the error I added an import for the `Alert` module and called the `alert()` function on that module. To help debug the `scrollToLocation()` behavior that was modified as a part of this PR I added three buttons (can be seen in the test plan image) which scroll to arbitrary points in the list. Reviewed By: sahrens Differential Revision: D5084095 fbshipit-source-id: 4c98bebc1c3f1ceaa5a634fa144685d83d1072df --- Libraries/Lists/VirtualizedSectionList.js | 57 ++++-- Libraries/Lists/__tests__/SectionList-test.js | 22 +++ .../__snapshots__/SectionList-test.js.snap | 168 ++++++++++++++++-- RNTester/js/SectionListExample.js | 29 ++- 4 files changed, 244 insertions(+), 32 deletions(-) diff --git a/Libraries/Lists/VirtualizedSectionList.js b/Libraries/Lists/VirtualizedSectionList.js index 42b7e15b6..474f80862 100644 --- a/Libraries/Lists/VirtualizedSectionList.js +++ b/Libraries/Lists/VirtualizedSectionList.js @@ -145,7 +145,7 @@ class VirtualizedSectionList }) { let index = params.itemIndex + 1; for (let ii = 0; ii < params.sectionIndex; ii++) { - index += this.props.sections[ii].data.length + 1; + index += this.props.sections[ii].data.length + 2; } const toIndexParams = { ...params, @@ -169,6 +169,7 @@ class VirtualizedSectionList section: SectionT, key: string, // Key of the section or combined key for section + item index: ?number, // Relative index within the section + header?: ?boolean, // True if this is the section header leadingItem?: ?Item, leadingSection?: ?SectionT, trailingItem?: ?Item, @@ -179,11 +180,25 @@ class VirtualizedSectionList for (let ii = 0; ii < this.props.sections.length; ii++) { const section = this.props.sections[ii]; const key = section.key || String(ii); - itemIndex -= 1; // The section itself is an item - if (itemIndex >= section.data.length) { - itemIndex -= section.data.length; + itemIndex -= 1; // The section adds an item for the header + if (itemIndex >= section.data.length + 1) { + itemIndex -= section.data.length + 1; // The section adds an item for the footer. } else if (itemIndex === -1) { - return {section, key, index: null, trailingSection: this.props.sections[ii + 1]}; + return { + section, + key: key + ':header', + index: null, + header: true, + trailingSection: this.props.sections[ii + 1], + }; + } else if (itemIndex === section.data.length) { + return { + section, + key: key + ':footer', + index: null, + header: false, + trailingSection: this.props.sections[ii + 1], + }; } else { const keyExtractor = section.keyExtractor || defaultKeyExtractor; return { @@ -232,8 +247,14 @@ class VirtualizedSectionList } const infoIndex = info.index; if (infoIndex == null) { - const {renderSectionHeader} = this.props; - return renderSectionHeader ? renderSectionHeader({section: info.section}) : null; + const {section} = info; + if (info.header === true) { + const {renderSectionHeader} = this.props; + return renderSectionHeader ? renderSectionHeader({section}) : null; + } else { + const {renderSectionFooter} = this.props; + return renderSectionFooter ? renderSectionFooter({section}) : null; + } } else { const renderItem = info.section.renderItem || this.props.renderItem; const SeparatorComponent = this._getSeparatorComponent(index, info); @@ -254,10 +275,6 @@ class VirtualizedSectionList prevCellKey={(this._subExtractor(index - 1) || {}).key} ref={(ref) => {this._cellRefs[info.key] = ref;}} renderItem={renderItem} - renderSectionFooter={infoIndex === info.section.data.length - 1 - ? this.props.renderSectionFooter - : undefined - } section={info.section} trailingItem={info.trailingItem} trailingSection={info.trailingSection} @@ -296,7 +313,7 @@ class VirtualizedSectionList const itemCount = props.sections.reduce( (v, section) => { stickyHeaderIndices.push(v + offset); - return v + section.data.length + 1; + return v + section.data.length + 2; // Add two for the section header and footer. }, 0 ); @@ -345,7 +362,6 @@ class ItemWithSeparator extends React.Component { onUpdateSeparator: (cellKey: string, newProps: Object) => void, prevCellKey?: ?string, renderItem: Function, - renderSectionFooter: ?Function, section: Object, leadingItem: ?Item, leadingSection: ?Object, @@ -406,9 +422,8 @@ class ItemWithSeparator extends React.Component { const leadingSeparator = LeadingSeparatorComponent && ; const separator = SeparatorComponent && ; - const footer = this.props.renderSectionFooter && this.props.renderSectionFooter({section}); - return (leadingSeparator || separator || footer) - ? {leadingSeparator}{element}{separator}{footer} + return (leadingSeparator || separator) + ? {leadingSeparator}{element}{separator} : element; } } @@ -419,12 +434,16 @@ function getItem(sections: ?$ReadOnlyArray, index: number): ?Item { } let itemIdx = index - 1; for (let ii = 0; ii < sections.length; ii++) { - if (itemIdx === -1) { - return sections[ii]; // The section itself is the item + if (itemIdx === -1 || itemIdx === sections[ii].data.length) { + // 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 + // item the section itself is the item. + return sections[ii]; } else if (itemIdx < sections[ii].data.length) { + // If we are in the bounds of the list's data then return the item. return sections[ii].data[itemIdx]; } else { - itemIdx -= (sections[ii].data.length + 1); + itemIdx -= (sections[ii].data.length + 2); // Add two for the header and footer } } return null; diff --git a/Libraries/Lists/__tests__/SectionList-test.js b/Libraries/Lists/__tests__/SectionList-test.js index 2250af966..4e529238d 100644 --- a/Libraries/Lists/__tests__/SectionList-test.js +++ b/Libraries/Lists/__tests__/SectionList-test.js @@ -39,6 +39,7 @@ describe('SectionList', () => { it('renders all the bells and whistles', () => { const component = ReactTestRenderer.create( } ListEmptyComponent={(props) => } ListFooterComponent={(props) =>