diff --git a/Libraries/Lists/VirtualizedList.js b/Libraries/Lists/VirtualizedList.js index 25e02db40d31ef..46309df0477c13 100644 --- a/Libraries/Lists/VirtualizedList.js +++ b/Libraries/Lists/VirtualizedList.js @@ -755,7 +755,7 @@ class VirtualizedList extends React.PureComponent { static _createRenderMask( props: Props, cellsAroundViewport: {first: number, last: number}, - lastFocusedItem: ?number, + additionalRegions?: ?$ReadOnlyArray<{first: number, last: number}>, ): CellRenderMask { const itemCount = props.getItemCount(props.data); @@ -768,17 +768,12 @@ class VirtualizedList extends React.PureComponent { const renderMask = new CellRenderMask(itemCount); - // Keep the items around the last focused rendered, to allow for keyboard - // navigation - if (lastFocusedItem) { - const first = Math.max(0, lastFocusedItem - 1); - const last = Math.min(itemCount - 1, lastFocusedItem + 1); - renderMask.addCells({first, last}); - } - if (itemCount > 0) { - if (cellsAroundViewport.last >= cellsAroundViewport.first) { - renderMask.addCells(cellsAroundViewport); + const allRegions = [cellsAroundViewport, ...(additionalRegions ?? [])]; + for (const region of allRegions) { + if (region.last >= region.first) { + renderMask.addCells(region); + } } // The initially rendered cells are retained as part of the @@ -956,23 +951,25 @@ class VirtualizedList extends React.PureComponent { this._fillRateHelper.deactivateAndFlush(); } - static getDerivedStateFromProps(newProps: Props, prevState: State): State { + UNSAFE_componentWillReceiveProps(newProps: Props) { // first and last could be stale (e.g. if a new, shorter items props is passed in), so we make // sure we're rendering a reasonable range here. const itemCount = newProps.getItemCount(newProps.data); - if (itemCount === prevState.renderMask.numCells()) { - return prevState; - } - - const constrainedCells = VirtualizedList._constrainToItemCount( - prevState.cellsAroundViewport, - newProps, - ); + if (itemCount !== this.state.renderMask.numCells()) { + const constrainedCells = VirtualizedList._constrainToItemCount( + this.state.cellsAroundViewport, + newProps, + ); - return { - cellsAroundViewport: constrainedCells, - renderMask: VirtualizedList._createRenderMask(newProps, constrainedCells), - }; + this.setState({ + cellsAroundViewport: constrainedCells, + renderMask: VirtualizedList._createRenderMask( + newProps, + constrainedCells, + this._getNonViewportRenderRegions(), + ), + }); + } } _pushCells( @@ -1016,7 +1013,7 @@ class VirtualizedList extends React.PureComponent { prevCellKey={prevCellKey} onUpdateSeparators={this._onUpdateSeparators} onLayout={e => this._onCellLayout(e, key, ii)} - onFocusCapture={e => this._onCellFocusCapture(ii)} + onFocusCapture={e => this._onCellFocusCapture(key)} onUnmount={this._onCellUnmount} parentProps={this.props} ref={ref => { @@ -1364,7 +1361,7 @@ class VirtualizedList extends React.PureComponent { _averageCellLength = 0; // Maps a cell key to the set of keys for all outermost child lists within that cell _cellKeysToChildListKeys: Map> = new Map(); - _cellRefs = {}; + _cellRefs: {[string]: ?CellRenderer} = {}; _fillRateHelper: FillRateHelper; _frames = {}; _footerLength = 0; @@ -1376,7 +1373,7 @@ class VirtualizedList extends React.PureComponent { _hiPriInProgress: boolean = false; // flag to prevent infinite hiPri cell limit update _highestMeasuredFrameIndex = 0; _indicesToKeys: Map = new Map(); - _lastFocusedItem: ?number = null; + _lastFocusedCellKey: ?string = null; _nestedChildLists: Map< string, { @@ -1485,12 +1482,12 @@ class VirtualizedList extends React.PureComponent { this._updateViewableItems(this.props.data); } - _onCellFocusCapture(itemIndex: number) { - this._lastFocusedItem = itemIndex; + _onCellFocusCapture(cellKey: string) { + this._lastFocusedCellKey = cellKey; const renderMask = VirtualizedList._createRenderMask( this.props, this.state.cellsAroundViewport, - this._lastFocusedItem, + this._getNonViewportRenderRegions(), ); if (!renderMask.equals(this.state.renderMask)) { @@ -1926,7 +1923,7 @@ class VirtualizedList extends React.PureComponent { const renderMask = VirtualizedList._createRenderMask( props, cellsAroundViewport, - this._lastFocusedItem, + this._getNonViewportRenderRegions(), ); if ( @@ -1998,6 +1995,57 @@ class VirtualizedList extends React.PureComponent { return frame; }; + _getNonViewportRenderRegions = (): $ReadOnlyArray<{ + first: number, + last: number, + }> => { + // Keep a viewport's worth of content around the last focused cell to allow + // random navigation around it without any blanking. E.g. tabbing from one + // focused item out of viewport to another. + if ( + !(this._lastFocusedCellKey && this._cellRefs[this._lastFocusedCellKey]) + ) { + return []; + } + + const lastFocusedCellRenderer = this._cellRefs[this._lastFocusedCellKey]; + const focusedCellIndex = lastFocusedCellRenderer.props.index; + const itemCount = this.props.getItemCount(this.props.data); + + // The cell may have been unmounted and have a stale index + if ( + focusedCellIndex >= itemCount || + this._indicesToKeys.get(focusedCellIndex) !== this._lastFocusedCellKey + ) { + return []; + } + + let first = focusedCellIndex; + let heightOfCellsBeforeFocused = 0; + for ( + let i = first - 1; + i >= 0 && heightOfCellsBeforeFocused < this._scrollMetrics.visibleLength; + i-- + ) { + first--; + heightOfCellsBeforeFocused += this._getFrameMetricsApprox(i).length; + } + + let last = focusedCellIndex; + let heightOfCellsAfterFocused = 0; + for ( + let i = last + 1; + i < itemCount && + heightOfCellsAfterFocused < this._scrollMetrics.visibleLength; + i++ + ) { + last++; + heightOfCellsAfterFocused += this._getFrameMetricsApprox(i).length; + } + + return [{first, last}]; + }; + _updateViewableItems(data: any) { const {getItemCount} = this.props; diff --git a/Libraries/Lists/__tests__/VirtualizedList-test.js b/Libraries/Lists/__tests__/VirtualizedList-test.js index 6ec0a178c3b0ba..9b4176a58c682a 100644 --- a/Libraries/Lists/__tests__/VirtualizedList-test.js +++ b/Libraries/Lists/__tests__/VirtualizedList-test.js @@ -1445,7 +1445,7 @@ it('renders windowSize derived region at bottom', () => { expect(component).toMatchSnapshot(); }); -it('keeps last focused item rendered', () => { +it('keeps viewport below last focused rendered', () => { const items = generateItems(20); const ITEM_HEIGHT = 10; @@ -1479,24 +1479,203 @@ it('keeps last focused item rendered', () => { performAllBatches(); }); - // Cells 1-4 should remain rendered after scrolling to the bottom of the list + // Cells 1-8 should remain rendered after scrolling to the bottom of the list expect(component).toMatchSnapshot(); +}); + +it('virtualizes away last focused item if focus changes to a new cell', () => { + const items = generateItems(20); + const ITEM_HEIGHT = 10; + + let component; + ReactTestRenderer.act(() => { + component = ReactTestRenderer.create( + , + ); + }); + + ReactTestRenderer.act(() => { + simulateLayout(component, { + viewport: {width: 10, height: 50}, + content: {width: 10, height: 200}, + }); + + performAllBatches(); + }); + + ReactTestRenderer.act(() => { + component.getInstance()._onCellFocusCapture(3); + }); + + ReactTestRenderer.act(() => { + simulateScroll(component, {x: 0, y: 150}); + performAllBatches(); + }); ReactTestRenderer.act(() => { component.getInstance()._onCellFocusCapture(17); }); - // Cells 2-4 should no longer be rendered after focus is moved to the end of + // Cells 1-8 should no longer be rendered after focus is moved to the end of // the list expect(component).toMatchSnapshot(); +}); + +it('keeps viewport above last focused rendered', () => { + const items = generateItems(20); + const ITEM_HEIGHT = 10; + + let component; + ReactTestRenderer.act(() => { + component = ReactTestRenderer.create( + , + ); + }); + + ReactTestRenderer.act(() => { + simulateLayout(component, { + viewport: {width: 10, height: 50}, + content: {width: 10, height: 200}, + }); + + performAllBatches(); + }); + + ReactTestRenderer.act(() => { + component.getInstance()._onCellFocusCapture(3); + }); + + ReactTestRenderer.act(() => { + simulateScroll(component, {x: 0, y: 150}); + performAllBatches(); + }); + + ReactTestRenderer.act(() => { + component.getInstance()._onCellFocusCapture(17); + }); + + ReactTestRenderer.act(() => { + simulateScroll(component, {x: 0, y: 0}); + performAllBatches(); + }); + + // Cells 12-19 should remain rendered after scrolling to the top of the list + expect(component).toMatchSnapshot(); +}); + +it('virtualizes away last focused index if item removed', () => { + const items = generateItems(20); + const ITEM_HEIGHT = 10; + + let component; + ReactTestRenderer.act(() => { + component = ReactTestRenderer.create( + , + ); + }); + + ReactTestRenderer.act(() => { + simulateLayout(component, { + viewport: {width: 10, height: 50}, + content: {width: 10, height: 200}, + }); + + performAllBatches(); + }); + + ReactTestRenderer.act(() => { + component.getInstance()._onCellFocusCapture(3); + }); + + ReactTestRenderer.act(() => { + simulateScroll(component, {x: 0, y: 150}); + performAllBatches(); + }); + + const itemsWithoutFocused = [...items.slice(0, 3), ...items.slice(4)]; + ReactTestRenderer.act(() => { + component.update( + , + ); + }); + + // Cells 1-8 should no longer be rendered + expect(component).toMatchSnapshot(); +}); + +it('renders area around last focused after items shift', () => { + const items = generateItems(20); + const ITEM_HEIGHT = 10; + + let component; + ReactTestRenderer.act(() => { + component = ReactTestRenderer.create( + , + ); + }); + + ReactTestRenderer.act(() => { + simulateLayout(component, { + viewport: {width: 10, height: 50}, + content: {width: 10, height: 200}, + }); + + performAllBatches(); + }); + + ReactTestRenderer.act(() => { + simulateScroll(component, {x: 0, y: 150}); + performAllBatches(); + }); + + ReactTestRenderer.act(() => { + component.getInstance()._onCellFocusCapture(17); + }); ReactTestRenderer.act(() => { simulateScroll(component, {x: 0, y: 0}); performAllBatches(); }); - // Cells 16-18 should remain rendered after scrolling back to the top of the - // list + const truncatedItems = items.slice(5); + ReactTestRenderer.act(() => { + component.update( + , + ); + performAllBatches(); + }); + + // Cells 12-19 should remain rendered after scrolling to the bottom of the list expect(component).toMatchSnapshot(); }); diff --git a/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap b/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap index 6f4721ff239a16..28aa4e2236d9c0 100644 --- a/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap +++ b/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap @@ -2781,7 +2781,7 @@ exports[`expands render area by maxToRenderPerBatch on tick 1`] = ` `; -exports[`keeps last focused item rendered 1`] = ` +exports[`keeps viewport above last focused rendered 1`] = ` + onFocusCapture={[Function]} + style={null} + > + + + + + + + + `; -exports[`keeps last focused item rendered 2`] = ` +exports[`keeps viewport below last focused rendered 1`] = ` + + + + + + + + + + + + + + + + + + + + + + + + @@ -3112,7 +3193,7 @@ exports[`keeps last focused item rendered 2`] = ` `; -exports[`keeps last focused item rendered 3`] = ` +exports[`renders a zero-height tail spacer on initial render if getItemLayout not defined 1`] = ` + + + + + + + + + + + + + +`; + +exports[`renders area around last focused after items shift 1`] = ` + + + + + + + + + + + + + + onFocusCapture={[Function]} + style={null} + > + + `; -exports[`renders a zero-height tail spacer on initial render if getItemLayout not defined 1`] = ` +exports[`renders full tail spacer if all cells measured 1`] = ` + + + + + + @@ -3366,7 +3574,7 @@ exports[`renders a zero-height tail spacer on initial render if getItemLayout no `; -exports[`renders full tail spacer if all cells measured 1`] = ` +exports[`renders initialNumToRender cells when virtualization disabled 1`] = ` - - - - - - - - - - - - - - - - - - - -`; - -exports[`renders initialNumToRender cells when virtualization disabled 1`] = ` - `; + +exports[`virtualizes away last focused index if item removed 1`] = ` + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +`; + +exports[`virtualizes away last focused item if focus changes to a new cell 1`] = ` + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +`;