Skip to content

Commit 03f180f

Browse files
committed
Add more sophisticated logic for "last focused" render region
1 parent f0de65b commit 03f180f

File tree

3 files changed

+592
-154
lines changed

3 files changed

+592
-154
lines changed

Libraries/Lists/VirtualizedList.js

Lines changed: 64 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -757,7 +757,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
757757
static _createRenderMask(
758758
props: Props,
759759
cellsAroundViewport: {first: number, last: number},
760-
lastFocusedItem: ?number,
760+
additionalRegions?: ?$ReadOnlyArray<{first: number, last: number}>,
761761
): CellRenderMask {
762762
const itemCount = props.getItemCount(props.data);
763763

@@ -770,17 +770,12 @@ class VirtualizedList extends React.PureComponent<Props, State> {
770770

771771
const renderMask = new CellRenderMask(itemCount);
772772

773-
// Keep the items around the last focused rendered, to allow for keyboard
774-
// navigation
775-
if (lastFocusedItem) {
776-
const first = Math.max(0, lastFocusedItem - 1);
777-
const last = Math.min(itemCount - 1, lastFocusedItem + 1);
778-
renderMask.addCells({first, last});
779-
}
780-
781773
if (itemCount > 0) {
782-
if (cellsAroundViewport.last >= cellsAroundViewport.first) {
783-
renderMask.addCells(cellsAroundViewport);
774+
const allRegions = [cellsAroundViewport, ...(additionalRegions ?? [])];
775+
for (const region of allRegions) {
776+
if (region.last >= region.first) {
777+
renderMask.addCells(region);
778+
}
784779
}
785780

786781
// The initially rendered cells are retained as part of the
@@ -1018,7 +1013,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
10181013
prevCellKey={prevCellKey}
10191014
onUpdateSeparators={this._onUpdateSeparators}
10201015
onLayout={e => this._onCellLayout(e, key, ii)}
1021-
onFocusCapture={e => this._onCellFocusCapture(ii)}
1016+
onFocusCapture={e => this._onCellFocusCapture(key)}
10221017
onUnmount={this._onCellUnmount}
10231018
parentProps={this.props}
10241019
ref={ref => {
@@ -1366,7 +1361,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
13661361
_averageCellLength = 0;
13671362
// Maps a cell key to the set of keys for all outermost child lists within that cell
13681363
_cellKeysToChildListKeys: Map<string, Set<string>> = new Map();
1369-
_cellRefs = {};
1364+
_cellRefs: {[string]: ?CellRenderer} = {};
13701365
_fillRateHelper: FillRateHelper;
13711366
_frames = {};
13721367
_footerLength = 0;
@@ -1378,7 +1373,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
13781373
_hiPriInProgress: boolean = false; // flag to prevent infinite hiPri cell limit update
13791374
_highestMeasuredFrameIndex = 0;
13801375
_indicesToKeys: Map<number, string> = new Map();
1381-
_lastFocusedItem: ?number = null;
1376+
_lastFocusedCellKey: ?string = null;
13821377
_nestedChildLists: Map<
13831378
string,
13841379
{
@@ -1487,12 +1482,12 @@ class VirtualizedList extends React.PureComponent<Props, State> {
14871482
this._updateViewableItems(this.props.data);
14881483
}
14891484

1490-
_onCellFocusCapture(itemIndex: number) {
1491-
this._lastFocusedItem = itemIndex;
1485+
_onCellFocusCapture(cellKey: string) {
1486+
this._lastFocusedCellKey = cellKey;
14921487
const renderMask = VirtualizedList._createRenderMask(
14931488
this.props,
14941489
this.state.cellsAroundViewport,
1495-
this._lastFocusedItem,
1490+
this._getNonViewportRenderRegions(),
14961491
);
14971492

14981493
if (!renderMask.equals(this.state.renderMask)) {
@@ -1928,7 +1923,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
19281923
const renderMask = VirtualizedList._createRenderMask(
19291924
props,
19301925
cellsAroundViewport,
1931-
this._lastFocusedItem,
1926+
this._getNonViewportRenderRegions(),
19321927
);
19331928

19341929
if (
@@ -2000,6 +1995,57 @@ class VirtualizedList extends React.PureComponent<Props, State> {
20001995
return frame;
20011996
};
20021997

1998+
_getNonViewportRenderRegions = (): $ReadOnlyArray<{
1999+
first: number,
2000+
last: number,
2001+
}> => {
2002+
// Keep a viewport's worth of content around the last focused cell to allow
2003+
// random navigation around it without any blanking. E.g. tabbing from one
2004+
// focused item out of viewport to another.
2005+
if (
2006+
!(this._lastFocusedCellKey && this._cellRefs[this._lastFocusedCellKey])
2007+
) {
2008+
return [];
2009+
}
2010+
2011+
const lastFocusedCellRenderer = this._cellRefs[this._lastFocusedCellKey];
2012+
const focusedCellIndex = lastFocusedCellRenderer.props.index;
2013+
const itemCount = this.props.getItemCount(this.props.data);
2014+
2015+
// The cell may have been unmounted and have a stale index
2016+
if (
2017+
focusedCellIndex >= itemCount ||
2018+
this._indicesToKeys.get(focusedCellIndex) !== this._lastFocusedCellKey
2019+
) {
2020+
return [];
2021+
}
2022+
2023+
let first = focusedCellIndex;
2024+
let heightOfCellsBeforeFocused = 0;
2025+
for (
2026+
let i = first - 1;
2027+
i >= 0 && heightOfCellsBeforeFocused < this._scrollMetrics.visibleLength;
2028+
i--
2029+
) {
2030+
first--;
2031+
heightOfCellsBeforeFocused += this._getFrameMetricsApprox(i).length;
2032+
}
2033+
2034+
let last = focusedCellIndex;
2035+
let heightOfCellsAfterFocused = 0;
2036+
for (
2037+
let i = last + 1;
2038+
i < itemCount &&
2039+
heightOfCellsAfterFocused < this._scrollMetrics.visibleLength;
2040+
i++
2041+
) {
2042+
last++;
2043+
heightOfCellsAfterFocused += this._getFrameMetricsApprox(i).length;
2044+
}
2045+
2046+
return [{first, last}];
2047+
};
2048+
20032049
_updateViewableItems(data: any) {
20042050
const {getItemCount} = this.props;
20052051

Libraries/Lists/__tests__/VirtualizedList-test.js

Lines changed: 128 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1445,7 +1445,7 @@ it('renders windowSize derived region at bottom', () => {
14451445
expect(component).toMatchSnapshot();
14461446
});
14471447

1448-
it('keeps last focused item rendered', () => {
1448+
it('keeps viewport below last focused rendered', () => {
14491449
const items = generateItems(20);
14501450
const ITEM_HEIGHT = 10;
14511451

@@ -1479,24 +1479,147 @@ it('keeps last focused item rendered', () => {
14791479
performAllBatches();
14801480
});
14811481

1482-
// Cells 1-4 should remain rendered after scrolling to the bottom of the list
1482+
// Cells 1-8 should remain rendered after scrolling to the bottom of the list
14831483
expect(component).toMatchSnapshot();
1484+
});
1485+
1486+
it('virtualizes away last focused item if focus changes to a new cell', () => {
1487+
const items = generateItems(20);
1488+
const ITEM_HEIGHT = 10;
1489+
1490+
let component;
1491+
ReactTestRenderer.act(() => {
1492+
component = ReactTestRenderer.create(
1493+
<VirtualizedList
1494+
initialNumToRender={1}
1495+
windowSize={1}
1496+
{...baseItemProps(items)}
1497+
{...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
1498+
/>,
1499+
);
1500+
});
1501+
1502+
ReactTestRenderer.act(() => {
1503+
simulateLayout(component, {
1504+
viewport: {width: 10, height: 50},
1505+
content: {width: 10, height: 200},
1506+
});
1507+
1508+
performAllBatches();
1509+
});
1510+
1511+
ReactTestRenderer.act(() => {
1512+
component.getInstance()._onCellFocusCapture(3);
1513+
});
1514+
1515+
ReactTestRenderer.act(() => {
1516+
simulateScroll(component, {x: 0, y: 150});
1517+
performAllBatches();
1518+
});
14841519

14851520
ReactTestRenderer.act(() => {
14861521
component.getInstance()._onCellFocusCapture(17);
14871522
});
14881523

1489-
// Cells 2-4 should no longer be rendered after focus is moved to the end of
1524+
// Cells 1-8 should no longer be rendered after focus is moved to the end of
14901525
// the list
14911526
expect(component).toMatchSnapshot();
1527+
});
1528+
1529+
it('keeps viewport above last focused rendered', () => {
1530+
const items = generateItems(20);
1531+
const ITEM_HEIGHT = 10;
1532+
1533+
let component;
1534+
ReactTestRenderer.act(() => {
1535+
component = ReactTestRenderer.create(
1536+
<VirtualizedList
1537+
initialNumToRender={1}
1538+
windowSize={1}
1539+
{...baseItemProps(items)}
1540+
{...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
1541+
/>,
1542+
);
1543+
});
1544+
1545+
ReactTestRenderer.act(() => {
1546+
simulateLayout(component, {
1547+
viewport: {width: 10, height: 50},
1548+
content: {width: 10, height: 200},
1549+
});
1550+
1551+
performAllBatches();
1552+
});
1553+
1554+
ReactTestRenderer.act(() => {
1555+
component.getInstance()._onCellFocusCapture(3);
1556+
});
1557+
1558+
ReactTestRenderer.act(() => {
1559+
simulateScroll(component, {x: 0, y: 150});
1560+
performAllBatches();
1561+
});
1562+
1563+
ReactTestRenderer.act(() => {
1564+
component.getInstance()._onCellFocusCapture(17);
1565+
});
14921566

14931567
ReactTestRenderer.act(() => {
14941568
simulateScroll(component, {x: 0, y: 0});
14951569
performAllBatches();
14961570
});
14971571

1498-
// Cells 16-18 should remain rendered after scrolling back to the top of the
1499-
// list
1572+
// Cells 12-19 should remain rendered after scrolling to the top of the list
1573+
expect(component).toMatchSnapshot();
1574+
});
1575+
1576+
it('virtualizes away last focused index if item removed', () => {
1577+
const items = generateItems(20);
1578+
const ITEM_HEIGHT = 10;
1579+
1580+
let component;
1581+
ReactTestRenderer.act(() => {
1582+
component = ReactTestRenderer.create(
1583+
<VirtualizedList
1584+
initialNumToRender={1}
1585+
windowSize={1}
1586+
{...baseItemProps(items)}
1587+
{...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
1588+
/>,
1589+
);
1590+
});
1591+
1592+
ReactTestRenderer.act(() => {
1593+
simulateLayout(component, {
1594+
viewport: {width: 10, height: 50},
1595+
content: {width: 10, height: 200},
1596+
});
1597+
1598+
performAllBatches();
1599+
});
1600+
1601+
ReactTestRenderer.act(() => {
1602+
component.getInstance()._onCellFocusCapture(3);
1603+
});
1604+
1605+
ReactTestRenderer.act(() => {
1606+
simulateScroll(component, {x: 0, y: 150});
1607+
performAllBatches();
1608+
});
1609+
1610+
const itemsWithoutFocused = [...items.slice(0, 3), ...items.slice(4)];
1611+
ReactTestRenderer.act(() => {
1612+
component.update(
1613+
<VirtualizedList
1614+
initialNumToRender={1}
1615+
windowSize={1}
1616+
{...baseItemProps(itemsWithoutFocused)}
1617+
{...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
1618+
/>,
1619+
);
1620+
});
1621+
1622+
// Cells 1-8 should no longer be rendered
15001623
expect(component).toMatchSnapshot();
15011624
});
15021625

0 commit comments

Comments
 (0)