Skip to content

Commit

Permalink
Remove code to support bottom-up layout events in horizontal RTL (#39646
Browse files Browse the repository at this point in the history 
)

Summary:
Pull Request resolved: #39646

We can dramatically simplify this code and remove quirks/hacks, now that we can assume layout events are always fired top down.

Changelog: [Internal]

Reviewed By: yungsters

Differential Revision: D49628669

fbshipit-source-id: 3332770f8cff3c3c423d777a25085a2c58fc9d4c
  • Loading branch information
@NickGerleman @facebook-github-bot
NickGerleman authored and facebook-github-bot committed Oct 10, 2023
1 parent f2d2df4 commit 20b0c41
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 243 deletions.
100 changes: 9 additions & 91 deletions packages/virtualized-lists/Lists/ListMetricsAggregator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import {keyExtractor as defaultKeyExtractor} from './VirtualizeUtils';

import invariant from 'invariant';

type LayoutEventDirection = 'top-down' | 'bottom-up';

export type CellMetrics = {
/**
* Index of the item in the list
Expand Down Expand Up @@ -55,25 +53,12 @@ export type CellMetricProps = {
...
};

type UnresolvedCellMetrics = {
index: number,
layout: Layout,
isMounted: boolean,

// The length of list content at the time of layout is needed to correctly
// resolve flow relative offset in RTL. We are lazily notified of this after
// the layout of the cell, unless the cell relayout does not cause a length
// change. To keep stability, we use content length at time of query, or
// unmount if never queried.
listContentLength?: ?number,
};

/**
* Provides an interface to query information about the metrics of a list and its cells.
*/
export default class ListMetricsAggregator {
_averageCellLength = 0;
_cellMetrics: Map<string, UnresolvedCellMetrics> = new Map();
_cellMetrics: Map<string, CellMetrics> = new Map();
_contentLength: ?number;
_highestMeasuredCellIndex = 0;
_measuredCellsLength = 0;
Expand All @@ -83,10 +68,6 @@ export default class ListMetricsAggregator {
rtl: false,
};

// Fabric and Paper may call onLayout in different orders. We can tell which
// direction layout events happen on the first layout.
_onLayoutDirection: LayoutEventDirection = 'top-down';

/**
* Notify the ListMetricsAggregator that a cell has been laid out.
*
Expand All @@ -103,39 +84,22 @@ export default class ListMetricsAggregator {
orientation: ListOrientation,
layout: Layout,
}): boolean {
if (this._contentLength == null) {
this._onLayoutDirection = 'bottom-up';
}

this._invalidateIfOrientationChanged(orientation);

// If layout is top-down, our most recently cached content length
// corresponds to this cell. Otherwise, we need to resolve when events fire
// up the tree to the new length.
const listContentLength =
this._onLayoutDirection === 'top-down' ? this._contentLength : null;

const next: UnresolvedCellMetrics = {
const next: CellMetrics = {
index: cellIndex,
layout: layout,
length: this._selectLength(layout),
isMounted: true,
listContentLength,
offset: this.flowRelativeOffset(layout),
};
const curr = this._cellMetrics.get(cellKey);

if (
!curr ||
this._selectOffset(next.layout) !== this._selectOffset(curr.layout) ||
this._selectLength(next.layout) !== this._selectLength(curr.layout) ||
(curr.listContentLength != null &&
curr.listContentLength !== this._contentLength)
) {
if (!curr || next.offset !== curr.offset || next.length !== curr.length) {
if (curr) {
const dLength =
this._selectLength(next.layout) - this._selectLength(curr.layout);
const dLength = next.length - curr.length;
this._measuredCellsLength += dLength;
} else {
this._measuredCellsLength += this._selectLength(next.layout);
this._measuredCellsLength += next.length;
this._measuredCellsCount += 1;
}

Expand Down Expand Up @@ -174,21 +138,7 @@ export default class ListMetricsAggregator {
layout: $ReadOnly<{width: number, height: number}>,
}): void {
this._invalidateIfOrientationChanged(orientation);
const newLength = this._selectLength(layout);

// Fill in any just-measured cells which did not have this length available.
// This logic assumes that cell relayout will always change list content
// size, which isn't strictly correct, but issues should be rare and only
// on Paper.
if (this._onLayoutDirection === 'bottom-up') {
for (const cellMetric of this._cellMetrics.values()) {
if (cellMetric.listContentLength == null) {
cellMetric.listContentLength = newLength;
}
}
}

this._contentLength = newLength;
this._contentLength = this._selectLength(layout);
}

/**
Expand Down Expand Up @@ -245,7 +195,7 @@ export default class ListMetricsAggregator {
keyExtractor(getItem(data, index), index),
);
if (frame && frame.index === index) {
return this._resolveCellMetrics(frame);
return frame;
}

if (getItemLayout) {
Expand Down Expand Up @@ -286,26 +236,6 @@ export default class ListMetricsAggregator {
return this._contentLength != null;
}

/**
* Whether the ListMetricsAggregator is notified of cell metrics before
* ScrollView metrics (bottom-up) or ScrollView metrics before cell metrics
* (top-down).
*
* Must be queried after cell layout
*/
getLayoutEventDirection(): LayoutEventDirection {
return this._onLayoutDirection;
}

/**
* Whether the ListMetricsAggregator must be aware of the current length of
* ScrollView content to be able to correctly resolve the (flow-relative)
* metrics of a cell.
*/
needsContentLengthForCellMetrics(): boolean {
return this._orientation.horizontal && this._orientation.rtl;
}

/**
* Finds the flow-relative offset (e.g. starting from the left in LTR, but
* right in RTL) from a layout box.
Expand Down Expand Up @@ -352,7 +282,6 @@ export default class ListMetricsAggregator {

if (orientation.horizontal !== this._orientation.horizontal) {
this._averageCellLength = 0;
this._contentLength = null;
this._highestMeasuredCellIndex = 0;
this._measuredCellsLength = 0;
this._measuredCellsCount = 0;
Expand All @@ -371,15 +300,4 @@ export default class ListMetricsAggregator {
_selectOffset({x, y}: $ReadOnly<{x: number, y: number, ...}>): number {
return this._orientation.horizontal ? x : y;
}

_resolveCellMetrics(metrics: UnresolvedCellMetrics): CellMetrics {
const {index, layout, isMounted, listContentLength} = metrics;

return {
index,
length: this._selectLength(layout),
isMounted,
offset: this.flowRelativeOffset(layout, listContentLength),
};
}
}
35 changes: 3 additions & 32 deletions packages/virtualized-lists/Lists/VirtualizedList.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1302,39 +1302,13 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
orientation: this._orientation(),
});

// In RTL layout we need parent content length to calculate the offset of a
// cell from the start of the list. In Paper, layout events are bottom up,
// so we do not know this yet, and must defer calculation until after
// `onContentSizeChange` is called.
const deferCellMetricCalculation =
this._listMetrics.getLayoutEventDirection() === 'bottom-up' &&
this._listMetrics.needsContentLengthForCellMetrics();

// Note: In Paper RTL logical position may have changed when
// `layoutHasChanged` is false if a cell maintains same X/Y coordinates,
// but contentLength shifts. This will be corrected by
// `onContentSizeChange` triggering a cell update.
if (layoutHasChanged) {
this._scheduleCellsToRenderUpdate({
allowImmediateExecution: !deferCellMetricCalculation,
});
this._scheduleCellsToRenderUpdate();
}

this._triggerRemeasureForChildListsInCell(cellKey);

this._computeBlankness();

if (deferCellMetricCalculation) {
if (!this._pendingViewabilityUpdate) {
this._pendingViewabilityUpdate = true;
setTimeout(() => {
this._updateViewableItems(this.props, this.state.cellsAroundViewport);
this._pendingViewabilityUpdate = false;
}, 0);
}
} else {
this._updateViewableItems(this.props, this.state.cellsAroundViewport);
}
this._updateViewableItems(this.props, this.state.cellsAroundViewport);
};

_onCellFocusCapture(cellKey: string) {
Expand Down Expand Up @@ -1765,9 +1739,7 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
}
}

_scheduleCellsToRenderUpdate(opts?: {allowImmediateExecution?: boolean}) {
const allowImmediateExecution = opts?.allowImmediateExecution ?? true;

_scheduleCellsToRenderUpdate() {
// Only trigger high-priority updates if we've actually rendered cells,
// and with that size estimate, accurately compute how many cells we should render.
// Otherwise, it would just render as many cells as it can (of zero dimension),
Expand All @@ -1776,7 +1748,6 @@ class VirtualizedList extends StateSafePureComponent<Props, State> {
// If this is triggered in an `componentDidUpdate` followed by a hiPri cellToRenderUpdate
// We shouldn't do another hipri cellToRenderUpdate
if (
allowImmediateExecution &&
this._shouldRenderWithPriority() &&
(this._listMetrics.getAverageCellLength() || this.props.getItemLayout) &&
!this._hiPriInProgress
Expand Down

0 comments on commit 20b0c41

Please sign in to comment.