From 3be01bd324a831a71de402ea43120c9f04704b95 Mon Sep 17 00:00:00 2001 From: Eric Rozell Date: Tue, 22 Oct 2024 09:05:37 -0700 Subject: [PATCH] Allow reverse-order lists Summary: We have a special case for treating horizontal RTL lists differently. This change adds similar functionality for capturing the correct virtualized list cell metrics when a list items children are rendered in reverse order, e.g., with `flexDirection: 'column-reverse'`. ## Changelog [General][Fixed] Fixed FlatList to support reverse ordered items Differential Revision: D64575365 --- .../Lists/ListMetricsAggregator.js | 18 +- .../Lists/VirtualizedList.js | 29 +- .../__tests__/ListMetricsAggregator-test.js | 315 ++++++++++++++++-- 3 files changed, 325 insertions(+), 37 deletions(-) diff --git a/packages/virtualized-lists/Lists/ListMetricsAggregator.js b/packages/virtualized-lists/Lists/ListMetricsAggregator.js index 9516269aee024e..79991fe7efd030 100644 --- a/packages/virtualized-lists/Lists/ListMetricsAggregator.js +++ b/packages/virtualized-lists/Lists/ListMetricsAggregator.js @@ -38,6 +38,7 @@ export type CellMetrics = { // based implementation instead of transform. export type ListOrientation = { horizontal: boolean, + reversed: boolean, rtl: boolean, }; @@ -66,6 +67,7 @@ export default class ListMetricsAggregator { _orientation: ListOrientation = { horizontal: false, rtl: false, + reversed: false, }; /** @@ -265,9 +267,9 @@ export default class ListMetricsAggregator { * right in RTL) from a layout box. */ flowRelativeOffset(layout: Layout, referenceContentLength?: ?number): number { - const {horizontal, rtl} = this._orientation; + const {horizontal, reversed, rtl} = this._orientation; - if (horizontal && rtl) { + if ((horizontal && rtl) || reversed) { const contentLength = referenceContentLength ?? this._contentLength; invariant( contentLength != null, @@ -286,9 +288,9 @@ export default class ListMetricsAggregator { * Converts a flow-relative offset to a cartesian offset */ cartesianOffset(flowRelativeOffset: number): number { - const {horizontal, rtl} = this._orientation; + const {horizontal, reversed, rtl} = this._orientation; - if (horizontal && rtl) { + if ((horizontal && rtl) || reversed) { invariant( this._contentLength != null, 'ListMetricsAggregator must be notified of list content layout before resolving offsets', @@ -311,6 +313,14 @@ export default class ListMetricsAggregator { this._measuredCellsCount = 0; } + if (orientation.reversed !== this._orientation.reversed) { + this._cellMetrics.clear(); + this._averageCellLength = 0; + this._highestMeasuredCellIndex = 0; + this._measuredCellsLength = 0; + this._measuredCellsCount = 0; + } + this._orientation = orientation; } diff --git a/packages/virtualized-lists/Lists/VirtualizedList.js b/packages/virtualized-lists/Lists/VirtualizedList.js index 4acdd08684c451..3114e8578c99c3 100644 --- a/packages/virtualized-lists/Lists/VirtualizedList.js +++ b/packages/virtualized-lists/Lists/VirtualizedList.js @@ -252,10 +252,14 @@ class VirtualizedList extends StateSafePureComponent { return; } - const {horizontal, rtl} = this._orientation(); - if (horizontal && rtl && !this._listMetrics.hasContentLength()) { + const {horizontal, reversed, rtl} = this._orientation(); + if ( + ((horizontal && rtl) || reversed) && + !this._listMetrics.hasContentLength() + ) { + const mode = horizontal && rtl ? 'RTL' : 'reversed lists'; console.warn( - 'scrollToOffset may not be called in RTL before content is laid out', + `scrollToOffset may not be called in ${mode} before content is laid out`, ); return; } @@ -267,8 +271,8 @@ class VirtualizedList extends StateSafePureComponent { } _scrollToParamsFromOffset(offset: number): {x?: number, y?: number} { - const {horizontal, rtl} = this._orientation(); - if (horizontal && rtl) { + const {horizontal, reversed, rtl} = this._orientation(); + if ((horizontal && rtl) || reversed) { // Add the visible length of the scrollview so that the offset is right-aligned const cartOffset = this._listMetrics.cartesianOffset( offset + this._scrollMetrics.visibleLength, @@ -1489,8 +1493,17 @@ class VirtualizedList extends StateSafePureComponent { } _orientation(): ListOrientation { + const horizontal = horizontalOrDefault(this.props.horizontal); + const contentFlexDirection = StyleSheet.flatten( + this.props.contentContainerStyle, + )?.flexDirection; + const reversed = + (horizontal && contentFlexDirection === 'row-reverse') || + contentFlexDirection === 'column-reverse'; + return { - horizontal: horizontalOrDefault(this.props.horizontal), + horizontal, + reversed, rtl: I18nManager.isRTL, }; } @@ -1731,8 +1744,8 @@ class VirtualizedList extends StateSafePureComponent { _offsetFromScrollEvent(e: ScrollEvent): number { const {contentOffset, contentSize, layoutMeasurement} = e.nativeEvent; - const {horizontal, rtl} = this._orientation(); - if (horizontal && rtl) { + const {horizontal, reversed, rtl} = this._orientation(); + if ((horizontal && rtl) || reversed) { return ( this._selectLength(contentSize) - (this._selectOffset(contentOffset) + diff --git a/packages/virtualized-lists/Lists/__tests__/ListMetricsAggregator-test.js b/packages/virtualized-lists/Lists/__tests__/ListMetricsAggregator-test.js index 7b5e416d41960e..74d5db2e861bb1 100644 --- a/packages/virtualized-lists/Lists/__tests__/ListMetricsAggregator-test.js +++ b/packages/virtualized-lists/Lists/__tests__/ListMetricsAggregator-test.js @@ -16,7 +16,7 @@ import nullthrows from 'nullthrows'; describe('ListMetricsAggregator', () => { it('keeps a running average length of measured cells', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: false, rtl: false}; + const orientation = {horizontal: false, reversed: false, rtl: false}; expect(listMetrics.getAverageCellLength()).toEqual(0); @@ -49,7 +49,7 @@ describe('ListMetricsAggregator', () => { it('adjusts the average cell length when layout changes', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: false, rtl: false}; + const orientation = {horizontal: false, reversed: false, rtl: false}; expect(listMetrics.getAverageCellLength()).toEqual(0); @@ -95,7 +95,7 @@ describe('ListMetricsAggregator', () => { it('keeps track of the highest measured cell index', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: false, rtl: false}; + const orientation = {horizontal: false, reversed: false, rtl: false}; expect(listMetrics.getHighestMeasuredCellIndex()).toEqual(0); @@ -133,7 +133,7 @@ describe('ListMetricsAggregator', () => { listMetrics.notifyCellLayout({ cellIndex: 0, cellKey: '0', - orientation: {horizontal: false, rtl: false}, + orientation: {horizontal: false, reversed: false, rtl: false}, layout: { height: 10, width: 5, @@ -146,7 +146,7 @@ describe('ListMetricsAggregator', () => { listMetrics.notifyCellLayout({ cellIndex: 1, cellKey: '1', - orientation: {horizontal: true, rtl: false}, + orientation: {horizontal: true, reversed: false, rtl: false}, layout: { height: 20, width: 5, @@ -159,7 +159,7 @@ describe('ListMetricsAggregator', () => { it('resolves metrics of already measured cell', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: false, rtl: false}; + const orientation = {horizontal: false, reversed: false, rtl: false}; const props: CellMetricProps = { data: [1, 2, 3, 4, 5], getItemCount: () => nullthrows(props.data).length, @@ -206,7 +206,7 @@ describe('ListMetricsAggregator', () => { it('estimates metrics of unmeasured cell', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: false, rtl: false}; + const orientation = {horizontal: false, reversed: false, rtl: false}; const props: CellMetricProps = { data: [1, 2, 3, 4, 5], getItemCount: () => nullthrows(props.data).length, @@ -248,7 +248,7 @@ describe('ListMetricsAggregator', () => { it('uses getItemLayout for metrics of unmeasured cell', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: false, rtl: false}; + const orientation = {horizontal: false, reversed: false, rtl: false}; const props: CellMetricProps = { data: [1, 2, 3, 4, 5], getItemCount: () => nullthrows(props.data).length, @@ -294,7 +294,7 @@ describe('ListMetricsAggregator', () => { it('resolves horizontal metrics of already measured cell', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: true, rtl: false}; + const orientation = {horizontal: true, reversed: false, rtl: false}; const props: CellMetricProps = { data: [1, 2, 3, 4, 5], getItemCount: () => nullthrows(props.data).length, @@ -341,7 +341,7 @@ describe('ListMetricsAggregator', () => { it('estimates horizontal metrics of unmeasured cell', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: true, rtl: false}; + const orientation = {horizontal: true, reversed: false, rtl: false}; const props: CellMetricProps = { data: [1, 2, 3, 4, 5], getItemCount: () => nullthrows(props.data).length, @@ -383,7 +383,7 @@ describe('ListMetricsAggregator', () => { it('uses getItemLayout for horizontal metrics of unmeasured cell', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: true, rtl: false}; + const orientation = {horizontal: true, reversed: false, rtl: false}; const props: CellMetricProps = { data: [1, 2, 3, 4, 5], getItemCount: () => nullthrows(props.data).length, @@ -429,7 +429,7 @@ describe('ListMetricsAggregator', () => { it('resolves RTL metrics of already measured cell', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: true, rtl: true}; + const orientation = {horizontal: true, reversed: false, rtl: true}; const props: CellMetricProps = { data: [1, 2, 3, 4, 5], getItemCount: () => nullthrows(props.data).length, @@ -479,9 +479,61 @@ describe('ListMetricsAggregator', () => { }); }); + it('resolves reversed metrics of already measured cell', () => { + const listMetrics = new ListMetricsAggregator(); + const orientation = {horizontal: false, reversed: true, rtl: false}; + const props: CellMetricProps = { + data: [1, 2, 3, 4, 5], + getItemCount: () => nullthrows(props.data).length, + getItem: (i: number) => nullthrows(props.data)[i], + }; + + listMetrics.notifyListContentLayout({ + layout: {width: 5, height: 100}, + orientation, + }); + + listMetrics.notifyCellLayout({ + cellIndex: 0, + cellKey: '0', + orientation, + layout: { + height: 10, + width: 5, + x: 0, + y: 90, + }, + }); + + listMetrics.notifyCellLayout({ + cellIndex: 1, + cellKey: '1', + orientation, + layout: { + height: 20, + width: 5, + x: 0, + y: 70, + }, + }); + + expect(listMetrics.getCellMetrics(1, props)).toEqual({ + index: 1, + length: 20, + offset: 10, + isMounted: true, + }); + expect(listMetrics.getCellMetricsApprox(1, props)).toEqual({ + index: 1, + length: 20, + offset: 10, + isMounted: true, + }); + }); + it('estimates RTL metrics of unmeasured cell', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: true, rtl: true}; + const orientation = {horizontal: true, reversed: false, rtl: true}; const props: CellMetricProps = { data: [1, 2, 3, 4, 5], getItemCount: () => nullthrows(props.data).length, @@ -525,9 +577,55 @@ describe('ListMetricsAggregator', () => { }); }); + it('estimates reversed metrics of unmeasured cell', () => { + const listMetrics = new ListMetricsAggregator(); + const orientation = {horizontal: false, reversed: true, rtl: false}; + const props: CellMetricProps = { + data: [1, 2, 3, 4, 5], + getItemCount: () => nullthrows(props.data).length, + getItem: (i: number) => nullthrows(props.data)[i], + }; + + listMetrics.notifyListContentLayout({ + layout: {width: 5, height: 100}, + orientation, + }); + + listMetrics.notifyCellLayout({ + cellIndex: 0, + cellKey: '0', + orientation, + layout: { + height: 10, + width: 5, + x: 0, + y: 70, + }, + }); + + listMetrics.notifyCellLayout({ + cellIndex: 1, + cellKey: '1', + orientation, + layout: { + height: 20, + width: 5, + x: 0, + y: 50, + }, + }); + + expect(listMetrics.getCellMetricsApprox(2, props)).toEqual({ + index: 2, + length: 15, + offset: 50, + isMounted: false, + }); + }); + it('uses getItemLayout for RTL metrics of unmeasured cell', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: true, rtl: true}; + const orientation = {horizontal: true, reversed: false, rtl: true}; const props: CellMetricProps = { data: [1, 2, 3, 4, 5], getItemCount: () => nullthrows(props.data).length, @@ -576,9 +674,60 @@ describe('ListMetricsAggregator', () => { }); }); + it('uses getItemLayout for reversed metrics of unmeasured cell', () => { + const listMetrics = new ListMetricsAggregator(); + const orientation = {horizontal: false, reversed: true, rtl: false}; + const props: CellMetricProps = { + data: [1, 2, 3, 4, 5], + getItemCount: () => nullthrows(props.data).length, + getItem: (i: number) => nullthrows(props.data)[i], + getItemLayout: () => ({index: 2, length: 40, offset: 30}), + }; + + listMetrics.notifyListContentLayout({ + layout: {width: 5, height: 100}, + orientation, + }); + + listMetrics.notifyCellLayout({ + cellIndex: 0, + cellKey: '0', + orientation, + layout: { + height: 10, + width: 5, + x: 0, + y: 90, + }, + }); + + listMetrics.notifyCellLayout({ + cellIndex: 1, + cellKey: '1', + orientation, + layout: { + height: 20, + width: 5, + x: 0, + y: 70, + }, + }); + + expect(listMetrics.getCellMetrics(2, props)).toMatchObject({ + index: 2, + length: 40, + offset: 30, + }); + expect(listMetrics.getCellMetricsApprox(2, props)).toMatchObject({ + index: 2, + length: 40, + offset: 30, + }); + }); + it('resolves vertical rtl metrics of already measured cell', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: false, rtl: true}; + const orientation = {horizontal: false, reversed: false, rtl: true}; const props: CellMetricProps = { data: [1, 2, 3, 4, 5], getItemCount: () => nullthrows(props.data).length, @@ -625,7 +774,7 @@ describe('ListMetricsAggregator', () => { it('estimates vertical RTL metrics of unmeasured cell', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: false, rtl: true}; + const orientation = {horizontal: false, reversed: false, rtl: true}; const props: CellMetricProps = { data: [1, 2, 3, 4, 5], getItemCount: () => nullthrows(props.data).length, @@ -667,7 +816,7 @@ describe('ListMetricsAggregator', () => { it('uses getItemLayout for vertical RTL metrics of unmeasured cell', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: false, rtl: true}; + const orientation = {horizontal: false, reversed: false, rtl: true}; const props: CellMetricProps = { data: [1, 2, 3, 4, 5], getItemCount: () => nullthrows(props.data).length, @@ -713,7 +862,7 @@ describe('ListMetricsAggregator', () => { it('resolves metrics of unmounted cell after list shift', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: true, rtl: true}; + const orientation = {horizontal: true, reversed: false, rtl: true}; const props: CellMetricProps = { data: [1, 2, 3, 4, 5], getItemCount: () => nullthrows(props.data).length, @@ -802,9 +951,100 @@ describe('ListMetricsAggregator', () => { }); }); + it('resolves metrics of unmounted cell after list shift when reversed', () => { + const listMetrics = new ListMetricsAggregator(); + const orientation = {horizontal: false, reversed: true, rtl: false}; + const props: CellMetricProps = { + data: [1, 2, 3, 4, 5], + getItemCount: () => nullthrows(props.data).length, + getItem: (i: number) => nullthrows(props.data)[i], + }; + + listMetrics.notifyListContentLayout({ + layout: {width: 5, height: 100}, + orientation, + }); + + listMetrics.notifyCellLayout({ + cellIndex: 0, + cellKey: '0', + orientation, + layout: { + height: 10, + width: 5, + x: 0, + y: 90, + }, + }); + + listMetrics.notifyCellLayout({ + cellIndex: 1, + cellKey: '1', + orientation, + layout: { + height: 20, + width: 5, + x: 0, + y: 70, + }, + }); + + expect(listMetrics.getCellMetrics(1, props)).toEqual({ + index: 1, + length: 20, + offset: 10, + isMounted: true, + }); + + listMetrics.notifyListContentLayout({ + layout: {width: 5, height: 120}, + orientation, + }); + + listMetrics.notifyCellLayout({ + cellIndex: 2, + cellKey: '2', + orientation, + layout: { + height: 20, + width: 5, + x: 0, + y: 50, + }, + }); + + expect(listMetrics.getCellMetrics(1, props)).toEqual({ + index: 1, + length: 20, + offset: 10, + isMounted: true, + }); + + listMetrics.notifyListContentLayout({ + layout: {width: 5, height: 100}, + orientation, + }); + + expect(listMetrics.getCellMetrics(1, props)).toEqual({ + index: 1, + length: 20, + offset: 10, + isMounted: true, + }); + + listMetrics.notifyCellUnmounted('1'); + + expect(listMetrics.getCellMetrics(1, props)).toEqual({ + index: 1, + length: 20, + offset: 10, + isMounted: false, + }); + }); + it('resolves integral offset of already measured cell', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: false, rtl: false}; + const orientation = {horizontal: false, reversed: false, rtl: false}; const props: CellMetricProps = { data: [1, 2, 3, 4, 5], getItemCount: () => nullthrows(props.data).length, @@ -840,7 +1080,7 @@ describe('ListMetricsAggregator', () => { it('estimates integral offset of unmeasured cell', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: false, rtl: false}; + const orientation = {horizontal: false, reversed: false, rtl: false}; const props: CellMetricProps = { data: [1, 2, 3, 4, 5], getItemCount: () => nullthrows(props.data).length, @@ -876,7 +1116,7 @@ describe('ListMetricsAggregator', () => { it('resolves fractional offset of already measured cell', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: false, rtl: false}; + const orientation = {horizontal: false, reversed: false, rtl: false}; const props: CellMetricProps = { data: [1, 2, 3, 4, 5], getItemCount: () => nullthrows(props.data).length, @@ -912,7 +1152,7 @@ describe('ListMetricsAggregator', () => { it('estimates fractional offset of unmeasured cell', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: false, rtl: false}; + const orientation = {horizontal: false, reversed: false, rtl: false}; const props: CellMetricProps = { data: [1, 2, 3, 4, 5], getItemCount: () => nullthrows(props.data).length, @@ -948,7 +1188,7 @@ describe('ListMetricsAggregator', () => { it('remembers most recent content length', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: false, rtl: false}; + const orientation = {horizontal: false, reversed: false, rtl: false}; expect(listMetrics.hasContentLength()).toBe(false); expect(listMetrics.getContentLength()).toBe(0); @@ -968,7 +1208,7 @@ describe('ListMetricsAggregator', () => { it('remembers most recent horizontal content length', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: true, rtl: false}; + const orientation = {horizontal: true, reversed: false, rtl: false}; expect(listMetrics.hasContentLength()).toBe(false); expect(listMetrics.getContentLength()).toBe(0); @@ -988,7 +1228,32 @@ describe('ListMetricsAggregator', () => { it('requires contentLength to resolve RTL metrics', () => { const listMetrics = new ListMetricsAggregator(); - const orientation = {horizontal: true, rtl: true}; + const orientation = {horizontal: true, reversed: false, rtl: true}; + + const props: CellMetricProps = { + data: [1, 2, 3, 4, 5], + getItemCount: () => nullthrows(props.data).length, + getItem: (i: number) => nullthrows(props.data)[i], + }; + + expect(() => + listMetrics.notifyCellLayout({ + cellIndex: 0, + cellKey: '0', + orientation, + layout: { + height: 10, + width: 5, + x: 0, + y: 0, + }, + }), + ).toThrow(); + }); + + it('requires contentLength to resolve reversed metrics', () => { + const listMetrics = new ListMetricsAggregator(); + const orientation = {horizontal: false, reversed: true, rtl: false}; const props: CellMetricProps = { data: [1, 2, 3, 4, 5],