Skip to content

Commit 8543ab6

Browse files
NickGerlemanfacebook-github-bot
authored andcommitted
Remove code to support bottom-up layout events in horizontal RTL
Summary: We can dramatically simplify this code and remove quirks/hacks, now that we can assume layout events are always fired top down. Changelog: [Internal] Differential Revision: D49628669 fbshipit-source-id: c9b1a233b7564f45e873f18b22036d9303f9176d
1 parent be59611 commit 8543ab6

File tree

3 files changed

+41
-243
lines changed

3 files changed

+41
-243
lines changed

packages/virtualized-lists/Lists/ListMetricsAggregator.js

Lines changed: 9 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@ import {keyExtractor as defaultKeyExtractor} from './VirtualizeUtils';
1414

1515
import invariant from 'invariant';
1616

17-
type LayoutEventDirection = 'top-down' | 'bottom-up';
18-
1917
export type CellMetrics = {
2018
/**
2119
* Index of the item in the list
@@ -55,25 +53,12 @@ export type CellMetricProps = {
5553
...
5654
};
5755

58-
type UnresolvedCellMetrics = {
59-
index: number,
60-
layout: Layout,
61-
isMounted: boolean,
62-
63-
// The length of list content at the time of layout is needed to correctly
64-
// resolve flow relative offset in RTL. We are lazily notified of this after
65-
// the layout of the cell, unless the cell relayout does not cause a length
66-
// change. To keep stability, we use content length at time of query, or
67-
// unmount if never queried.
68-
listContentLength?: ?number,
69-
};
70-
7156
/**
7257
* Provides an interface to query information about the metrics of a list and its cells.
7358
*/
7459
export default class ListMetricsAggregator {
7560
_averageCellLength = 0;
76-
_cellMetrics: Map<string, UnresolvedCellMetrics> = new Map();
61+
_cellMetrics: Map<string, CellMetrics> = new Map();
7762
_contentLength: ?number;
7863
_highestMeasuredCellIndex = 0;
7964
_measuredCellsLength = 0;
@@ -83,10 +68,6 @@ export default class ListMetricsAggregator {
8368
rtl: false,
8469
};
8570

86-
// Fabric and Paper may call onLayout in different orders. We can tell which
87-
// direction layout events happen on the first layout.
88-
_onLayoutDirection: LayoutEventDirection = 'top-down';
89-
9071
/**
9172
* Notify the ListMetricsAggregator that a cell has been laid out.
9273
*
@@ -103,39 +84,22 @@ export default class ListMetricsAggregator {
10384
orientation: ListOrientation,
10485
layout: Layout,
10586
}): boolean {
106-
if (this._contentLength == null) {
107-
this._onLayoutDirection = 'bottom-up';
108-
}
109-
11087
this._invalidateIfOrientationChanged(orientation);
11188

112-
// If layout is top-down, our most recently cached content length
113-
// corresponds to this cell. Otherwise, we need to resolve when events fire
114-
// up the tree to the new length.
115-
const listContentLength =
116-
this._onLayoutDirection === 'top-down' ? this._contentLength : null;
117-
118-
const next: UnresolvedCellMetrics = {
89+
const next: CellMetrics = {
11990
index: cellIndex,
120-
layout: layout,
91+
length: this._selectLength(layout),
12192
isMounted: true,
122-
listContentLength,
93+
offset: this.flowRelativeOffset(layout),
12394
};
12495
const curr = this._cellMetrics.get(cellKey);
12596

126-
if (
127-
!curr ||
128-
this._selectOffset(next.layout) !== this._selectOffset(curr.layout) ||
129-
this._selectLength(next.layout) !== this._selectLength(curr.layout) ||
130-
(curr.listContentLength != null &&
131-
curr.listContentLength !== this._contentLength)
132-
) {
97+
if (!curr || next.offset !== curr.offset || next.length !== curr.length) {
13398
if (curr) {
134-
const dLength =
135-
this._selectLength(next.layout) - this._selectLength(curr.layout);
99+
const dLength = next.length - curr.length;
136100
this._measuredCellsLength += dLength;
137101
} else {
138-
this._measuredCellsLength += this._selectLength(next.layout);
102+
this._measuredCellsLength += next.length;
139103
this._measuredCellsCount += 1;
140104
}
141105

@@ -174,21 +138,7 @@ export default class ListMetricsAggregator {
174138
layout: $ReadOnly<{width: number, height: number}>,
175139
}): void {
176140
this._invalidateIfOrientationChanged(orientation);
177-
const newLength = this._selectLength(layout);
178-
179-
// Fill in any just-measured cells which did not have this length available.
180-
// This logic assumes that cell relayout will always change list content
181-
// size, which isn't strictly correct, but issues should be rare and only
182-
// on Paper.
183-
if (this._onLayoutDirection === 'bottom-up') {
184-
for (const cellMetric of this._cellMetrics.values()) {
185-
if (cellMetric.listContentLength == null) {
186-
cellMetric.listContentLength = newLength;
187-
}
188-
}
189-
}
190-
191-
this._contentLength = newLength;
141+
this._contentLength = this._selectLength(layout);
192142
}
193143

194144
/**
@@ -245,7 +195,7 @@ export default class ListMetricsAggregator {
245195
keyExtractor(getItem(data, index), index),
246196
);
247197
if (frame && frame.index === index) {
248-
return this._resolveCellMetrics(frame);
198+
return frame;
249199
}
250200

251201
if (getItemLayout) {
@@ -286,26 +236,6 @@ export default class ListMetricsAggregator {
286236
return this._contentLength != null;
287237
}
288238

289-
/**
290-
* Whether the ListMetricsAggregator is notified of cell metrics before
291-
* ScrollView metrics (bottom-up) or ScrollView metrics before cell metrics
292-
* (top-down).
293-
*
294-
* Must be queried after cell layout
295-
*/
296-
getLayoutEventDirection(): LayoutEventDirection {
297-
return this._onLayoutDirection;
298-
}
299-
300-
/**
301-
* Whether the ListMetricsAggregator must be aware of the current length of
302-
* ScrollView content to be able to correctly resolve the (flow-relative)
303-
* metrics of a cell.
304-
*/
305-
needsContentLengthForCellMetrics(): boolean {
306-
return this._orientation.horizontal && this._orientation.rtl;
307-
}
308-
309239
/**
310240
* Finds the flow-relative offset (e.g. starting from the left in LTR, but
311241
* right in RTL) from a layout box.
@@ -352,7 +282,6 @@ export default class ListMetricsAggregator {
352282

353283
if (orientation.horizontal !== this._orientation.horizontal) {
354284
this._averageCellLength = 0;
355-
this._contentLength = null;
356285
this._highestMeasuredCellIndex = 0;
357286
this._measuredCellsLength = 0;
358287
this._measuredCellsCount = 0;
@@ -371,15 +300,4 @@ export default class ListMetricsAggregator {
371300
_selectOffset({x, y}: $ReadOnly<{x: number, y: number, ...}>): number {
372301
return this._orientation.horizontal ? x : y;
373302
}
374-
375-
_resolveCellMetrics(metrics: UnresolvedCellMetrics): CellMetrics {
376-
const {index, layout, isMounted, listContentLength} = metrics;
377-
378-
return {
379-
index,
380-
length: this._selectLength(layout),
381-
isMounted,
382-
offset: this.flowRelativeOffset(layout, listContentLength),
383-
};
384-
}
385303
}

packages/virtualized-lists/Lists/VirtualizedList.js

Lines changed: 3 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1302,39 +1302,13 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
13021302
orientation: this._orientation(),
13031303
});
13041304

1305-
// In RTL layout we need parent content length to calculate the offset of a
1306-
// cell from the start of the list. In Paper, layout events are bottom up,
1307-
// so we do not know this yet, and must defer calculation until after
1308-
// `onContentSizeChange` is called.
1309-
const deferCellMetricCalculation =
1310-
this._listMetrics.getLayoutEventDirection() === 'bottom-up' &&
1311-
this._listMetrics.needsContentLengthForCellMetrics();
1312-
1313-
// Note: In Paper RTL logical position may have changed when
1314-
// `layoutHasChanged` is false if a cell maintains same X/Y coordinates,
1315-
// but contentLength shifts. This will be corrected by
1316-
// `onContentSizeChange` triggering a cell update.
13171305
if (layoutHasChanged) {
1318-
this._scheduleCellsToRenderUpdate({
1319-
allowImmediateExecution: !deferCellMetricCalculation,
1320-
});
1306+
this._scheduleCellsToRenderUpdate();
13211307
}
13221308

13231309
this._triggerRemeasureForChildListsInCell(cellKey);
1324-
13251310
this._computeBlankness();
1326-
1327-
if (deferCellMetricCalculation) {
1328-
if (!this._pendingViewabilityUpdate) {
1329-
this._pendingViewabilityUpdate = true;
1330-
setTimeout(() => {
1331-
this._updateViewableItems(this.props, this.state.cellsAroundViewport);
1332-
this._pendingViewabilityUpdate = false;
1333-
}, 0);
1334-
}
1335-
} else {
1336-
this._updateViewableItems(this.props, this.state.cellsAroundViewport);
1337-
}
1311+
this._updateViewableItems(this.props, this.state.cellsAroundViewport);
13381312
};
13391313

13401314
_onCellFocusCapture(cellKey: string) {
@@ -1765,9 +1739,7 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
17651739
}
17661740
}
17671741

1768-
_scheduleCellsToRenderUpdate(opts?: {allowImmediateExecution?: boolean}) {
1769-
const allowImmediateExecution = opts?.allowImmediateExecution ?? true;
1770-
1742+
_scheduleCellsToRenderUpdate() {
17711743
// Only trigger high-priority updates if we've actually rendered cells,
17721744
// and with that size estimate, accurately compute how many cells we should render.
17731745
// Otherwise, it would just render as many cells as it can (of zero dimension),
@@ -1776,7 +1748,6 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
17761748
// If this is triggered in an `componentDidUpdate` followed by a hiPri cellToRenderUpdate
17771749
// We shouldn't do another hipri cellToRenderUpdate
17781750
if (
1779-
allowImmediateExecution &&
17801751
this._shouldRenderWithPriority() &&
17811752
(this._listMetrics.getAverageCellLength() || this.props.getItemLayout) &&
17821753
!this._hiPriInProgress

0 commit comments

Comments
 (0)