Skip to content

Commit

Permalink
Fix virtualization logic with horizontal RTL lists (facebook#38529)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebook#38529

VirtualizedList internally represents metrics along the scrolling axis using `offset` (x, y), and `length` (width, height). This one dimensional representation allows the list to be generic to being horizontal or vertical.

Right now offset conversion directly takes the `x` or `y` axis value, but  this is not correct when we are using a horizontal FlatList in RTL.

This change converts most VirtualizedList code handling  `offset,length` coordinates consistently flow from start to end, including in horizontal RTL and in inverted FlatList.

This is paired with a fix to Android native code in D47627115. With these together, basic Horizontal FlatList scenarios should work correctly when laid out in RTL.

Changelog:
[General][Fixed] - Fix virtualization logic with horizontal RTL lists

Reviewed By: vincentriemer

Differential Revision: D46586420

fbshipit-source-id: 56a29abc4bdf3b9505b510bcc207fd750c3c08a7
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Jul 27, 2023
1 parent c01c221 commit 3ce1744
Show file tree
Hide file tree
Showing 2 changed files with 326 additions and 136 deletions.
219 changes: 186 additions & 33 deletions packages/virtualized-lists/Lists/ListMetricsAggregator.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* @format
*/

import type {Layout} from 'react-native/Libraries/Types/CoreEventTypes';
import type {Props as VirtualizedListProps} from './VirtualizedListProps';
import {keyExtractor as defaultKeyExtractor} from './VirtualizeUtils';

Expand All @@ -23,7 +24,8 @@ export type CellMetrics = {
*/
length: number,
/**
* Offset to the cell along the scrolling axis
* Distance between this cell and the start of the list along the scrolling
* axis
*/
offset: number,
/**
Expand All @@ -32,6 +34,13 @@ export type CellMetrics = {
isMounted: boolean,
};

// TODO: `inverted` can be incorporated here if it is moved to an order
// based implementation instead of transform.
export type ListOrientation = {
horizontal: boolean,
rtl: boolean,
};

/**
* Subset of VirtualizedList props needed to calculate cell metrics
*/
Expand All @@ -44,52 +53,85 @@ 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;
_frames: {[string]: CellMetrics} = {};
_cellMetrics: {[string]: UnresolvedCellMetrics} = {};
_contentLength: ?number;
_highestMeasuredCellIndex = 0;
_totalCellLength = 0;
_totalCellsMeasured = 0;
_measuredCellsLength = 0;
_measuredCellsCount = 0;
_orientation: ListOrientation = {
horizontal: false,
rtl: false,
};

/**
* Notify the ListMetricsAggregator that a cell has been laid out.
*
* @returns whether the cell layout has changed since last notification
*/
notifyCellLayout(
notifyCellLayout({
cellIndex,
cellKey,
orientation,
layout,
}: {
cellIndex: number,
cellKey: string,
index: number,
length: number,
offset: number,
): boolean {
const next: CellMetrics = {
offset,
length,
index,
orientation: ListOrientation,
layout: Layout,
}): boolean {
this._invalidateIfOrientationChanged(orientation);

const next: UnresolvedCellMetrics = {
index: cellIndex,
layout: layout,
isMounted: true,
};
const curr = this._frames[cellKey];
const curr = this._cellMetrics[cellKey];

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

this._averageCellLength =
this._totalCellLength / this._totalCellsMeasured;
this._frames[cellKey] = next;
this._measuredCellsLength / this._measuredCellsCount;
this._cellMetrics[cellKey] = next;
this._highestMeasuredCellIndex = Math.max(
this._highestMeasuredCellIndex,
index,
cellIndex,
);
return true;
} else {
this._frames[cellKey].isMounted = true;
this._cellMetrics[cellKey].isMounted = true;
return false;
}
}
Expand All @@ -98,12 +140,31 @@ export default class ListMetricsAggregator {
* Notify ListMetricsAggregator that a cell has been unmounted.
*/
notifyCellUnmounted(cellKey: string): void {
const curr = this._frames[cellKey];
const curr = this._cellMetrics[cellKey];
if (curr) {
this._frames[cellKey] = {...curr, isMounted: false};
this._cellMetrics[cellKey] = {
...curr,
isMounted: false,
listContentLength: curr.listContentLength ?? this._contentLength,
};
}
}

/**
* Notify ListMetricsAggregator that the lists content container has been laid out.
*/
notifyListContentLayout({
orientation,
layout,
}: {
orientation: ListOrientation,
layout: $ReadOnly<{width: number, height: number}>,
}): void {
this._invalidateIfOrientationChanged(orientation);
const newLength = this._selectLength(layout);
this._contentLength = newLength;
}

/**
* Return the average length of the cells which have been measured
*/
Expand Down Expand Up @@ -150,17 +211,20 @@ export default class ListMetricsAggregator {
const {data, getItem, getItemCount, getItemLayout} = props;
invariant(
index >= 0 && index < getItemCount(data),
'Tried to get frame for out of range index ' + index,
'Tried to get metrics for out of range cell index ' + index,
);
const keyExtractor = props.keyExtractor ?? defaultKeyExtractor;
const frame = this._frames[keyExtractor(getItem(data, index), index)];
if (!frame || frame.index !== index) {
if (getItemLayout) {
const {length, offset} = getItemLayout(data, index);
return {index, length, offset, isMounted: true};
}
const frame = this._cellMetrics[keyExtractor(getItem(data, index), index)];
if (frame && frame.index === index) {
return this._resolveCellMetrics(frame);
}
return frame;

if (getItemLayout) {
const {length, offset} = getItemLayout(data, index);
return {index, length, offset, isMounted: true};
}

return null;
}

/**
Expand All @@ -176,4 +240,93 @@ export default class ListMetricsAggregator {
return frameMetrics.offset + remainder * frameMetrics.length;
}
}

/**
* Returns the length of all ScrollView content along the scrolling axis.
*/
getContentLength(): number {
return this._contentLength ?? 0;
}

/**
* Whether a content length has been observed
*/
hasContentLength(): boolean {
return this._contentLength != null;
}

/**
* Converts a cartesian offset along the x or y axis to a flow-relative
* offset, (e.g. starting from the left in LTR, but right in RTL).
*/
flowRelativeOffset(layout: Layout, referenceContentLength?: ?number): number {
const {horizontal, rtl} = this._orientation;

if (horizontal && rtl) {
const contentLength = referenceContentLength ?? this._contentLength;
invariant(
contentLength != null,
'ListMetricsAggregator must be notified of list content layout before resolving offsets',
);
return contentLength - this._selectOffset(layout);
} else {
return this._selectOffset(layout);
}
}

/**
* Converts a flow-relative offset to a cartesian offset
*/
cartesianOffset(flowRelativeOffset: number): number {
const {horizontal, rtl} = this._orientation;

if (horizontal && rtl) {
invariant(
this._contentLength != null,
'ListMetricsAggregator must be notified of list content layout before resolving offsets',
);
return this._contentLength - flowRelativeOffset;
} else {
return flowRelativeOffset;
}
}

_invalidateIfOrientationChanged(orientation: ListOrientation): void {
if (orientation.rtl !== this._orientation.rtl) {
this._cellMetrics = {};
}

if (orientation.horizontal !== this._orientation.horizontal) {
this._averageCellLength = 0;
this._contentLength = 0;
this._highestMeasuredCellIndex = 0;
this._measuredCellsLength = 0;
this._measuredCellsCount = 0;
}

this._orientation = orientation;
}

_selectLength({
width,
height,
}: $ReadOnly<{width: number, height: number, ...}>): number {
return this._orientation.horizontal ? width : height;
}

_selectOffset({x, y}: $ReadOnly<{x: number, y: number, ...}>): number {
return this._orientation.horizontal ? x : y;
}

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

return {
index,
length: this._selectLength(layout),
isMounted,
offset: this.flowRelativeOffset(layout, listContentLength),
};
}
}
Loading

0 comments on commit 3ce1744

Please sign in to comment.