From baead237bfdfe39835e951f012b6691ab6a8b1d2 Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Thu, 7 Sep 2023 02:16:30 +0100 Subject: [PATCH 1/6] migrate BaseInvertedFlatList.js to function component --- .../InvertedFlatList/BaseInvertedFlatList.js | 174 +++++++++--------- 1 file changed, 88 insertions(+), 86 deletions(-) diff --git a/src/components/InvertedFlatList/BaseInvertedFlatList.js b/src/components/InvertedFlatList/BaseInvertedFlatList.js index d3fcda0ea5fd..5d684ef2c7f6 100644 --- a/src/components/InvertedFlatList/BaseInvertedFlatList.js +++ b/src/components/InvertedFlatList/BaseInvertedFlatList.js @@ -1,8 +1,7 @@ -/* eslint-disable react/jsx-props-no-multi-spaces */ +import React, {forwardRef, useCallback, useRef} from 'react'; +import {View, FlatList as NativeFlatlist} from 'react-native'; import _ from 'underscore'; -import React, {forwardRef, Component} from 'react'; import PropTypes from 'prop-types'; -import {View, FlatList as NativeFlatlist} from 'react-native'; import * as CollectionUtils from '../../libs/CollectionUtils'; import FlatList from '../FlatList'; @@ -29,19 +28,14 @@ const defaultProps = { shouldMeasureItems: false, }; -class BaseInvertedFlatList extends Component { - constructor(props) { - super(props); +function BaseInvertedFlatList(props) { + const {initialRowHeight, shouldMeasureItems, innerRef, renderItem} = props; - this.renderItem = this.renderItem.bind(this); - this.getItemLayout = this.getItemLayout.bind(this); - - // Stores each item's computed height after it renders - // once and is then referenced for the life of this component. - // This is essential to getting FlatList inverted to work on web - // and also enables more predictable scrolling on native platforms. - this.sizeMap = {}; - } + // Stores each item's computed height after it renders + // once and is then referenced for the life of this component. + // This is essential to getting FlatList inverted to work on web + // and also enables more predictable scrolling on native platforms. + const sizeMap = useRef({}).current; /** * Return default or previously cached height for @@ -52,33 +46,36 @@ class BaseInvertedFlatList extends Component { * * @return {Object} */ - getItemLayout(data, index) { - const size = this.sizeMap[index]; + const getItemLayout = useCallback( + (data, index) => { + const size = sizeMap[index]; + + if (size) { + return { + length: size.length, + offset: size.offset, + index, + }; + } + + // If we don't have a size yet means we haven't measured this + // item yet. However, we can still calculate the offset by looking + // at the last size we have recorded (if any) + const lastMeasuredItem = CollectionUtils.lastItem(sizeMap); - if (size) { return { - length: size.length, - offset: size.offset, + // We haven't measured this so we must return the minimum row height + length: initialRowHeight, + + // Offset will either be based on the lastMeasuredItem or the index + + // initialRowHeight since we can only assume that all previous items + // have not yet been measured + offset: _.isUndefined(lastMeasuredItem) ? initialRowHeight * index : lastMeasuredItem.offset + initialRowHeight, index, }; - } - - // If we don't have a size yet means we haven't measured this - // item yet. However, we can still calculate the offset by looking - // at the last size we have recorded (if any) - const lastMeasuredItem = CollectionUtils.lastItem(this.sizeMap); - - return { - // We haven't measured this so we must return the minimum row height - length: this.props.initialRowHeight, - - // Offset will either be based on the lastMeasuredItem or the index + - // initialRowHeight since we can only assume that all previous items - // have not yet been measured - offset: _.isUndefined(lastMeasuredItem) ? this.props.initialRowHeight * index : lastMeasuredItem.offset + this.props.initialRowHeight, - index, - }; - } + }, + [initialRowHeight, sizeMap], + ); /** * Measure item and cache the returned length (a.k.a. height) @@ -86,26 +83,29 @@ class BaseInvertedFlatList extends Component { * @param {React.NativeSyntheticEvent} nativeEvent * @param {Number} index */ - measureItemLayout(nativeEvent, index) { - const computedHeight = nativeEvent.layout.height; - - // We've already measured this item so we don't need to - // measure it again. - if (this.sizeMap[index]) { - return; - } - - const previousItem = this.sizeMap[index - 1] || {}; - - // If there is no previousItem this can mean we haven't yet measured - // the previous item or that we are at index 0 and there is no previousItem - const previousLength = previousItem.length || 0; - const previousOffset = previousItem.offset || 0; - this.sizeMap[index] = { - length: computedHeight, - offset: previousLength + previousOffset, - }; - } + const measureItemLayout = useCallback( + (nativeEvent, index) => { + const computedHeight = nativeEvent.layout.height; + + // We've already measured this item so we don't need to + // measure it again. + if (sizeMap[index]) { + return; + } + + const previousItem = sizeMap[index - 1] || {}; + + // If there is no previousItem this can mean we haven't yet measured + // the previous item or that we are at index 0 and there is no previousItem + const previousLength = previousItem.length || 0; + const previousOffset = previousItem.offset || 0; + sizeMap[index] = { + length: computedHeight, + offset: previousLength + previousOffset, + }; + }, + [sizeMap], + ); /** * Render item method wraps the prop renderItem to render in a @@ -118,38 +118,40 @@ class BaseInvertedFlatList extends Component { * * @return {React.Component} */ - renderItem({item, index}) { - if (this.props.shouldMeasureItems) { - return this.measureItemLayout(nativeEvent, index)}>{this.props.renderItem({item, index})}; - } - - return this.props.renderItem({item, index}); - } - - render() { - return ( - - ); - } + const renderItemFromProp = useCallback( + ({item, index}) => { + if (shouldMeasureItems) { + return measureItemLayout(nativeEvent, index)}>{renderItem({item, index})}; + } + + return renderItem({item, index}); + }, + [shouldMeasureItems, measureItemLayout, renderItem], + ); + + return ( + + ); } BaseInvertedFlatList.propTypes = propTypes; BaseInvertedFlatList.defaultProps = defaultProps; +BaseInvertedFlatList.displayName = 'BaseInvertedFlatList'; export default forwardRef((props, ref) => ( Date: Thu, 7 Sep 2023 09:25:34 +0100 Subject: [PATCH 2/6] update use of callback and dependency --- .../InvertedFlatList/BaseInvertedFlatList.js | 94 +++++++++---------- 1 file changed, 44 insertions(+), 50 deletions(-) diff --git a/src/components/InvertedFlatList/BaseInvertedFlatList.js b/src/components/InvertedFlatList/BaseInvertedFlatList.js index 5d684ef2c7f6..e65dc512ce57 100644 --- a/src/components/InvertedFlatList/BaseInvertedFlatList.js +++ b/src/components/InvertedFlatList/BaseInvertedFlatList.js @@ -35,7 +35,7 @@ function BaseInvertedFlatList(props) { // once and is then referenced for the life of this component. // This is essential to getting FlatList inverted to work on web // and also enables more predictable scrolling on native platforms. - const sizeMap = useRef({}).current; + const sizeMap = useRef({}); /** * Return default or previously cached height for @@ -46,36 +46,33 @@ function BaseInvertedFlatList(props) { * * @return {Object} */ - const getItemLayout = useCallback( - (data, index) => { - const size = sizeMap[index]; - - if (size) { - return { - length: size.length, - offset: size.offset, - index, - }; - } - - // If we don't have a size yet means we haven't measured this - // item yet. However, we can still calculate the offset by looking - // at the last size we have recorded (if any) - const lastMeasuredItem = CollectionUtils.lastItem(sizeMap); + const getItemLayout = (data, index) => { + const size = sizeMap.current[index]; + if (size) { return { - // We haven't measured this so we must return the minimum row height - length: initialRowHeight, - - // Offset will either be based on the lastMeasuredItem or the index + - // initialRowHeight since we can only assume that all previous items - // have not yet been measured - offset: _.isUndefined(lastMeasuredItem) ? initialRowHeight * index : lastMeasuredItem.offset + initialRowHeight, + length: size.length, + offset: size.offset, index, }; - }, - [initialRowHeight, sizeMap], - ); + } + + // If we don't have a size yet means we haven't measured this + // item yet. However, we can still calculate the offset by looking + // at the last size we have recorded (if any) + const lastMeasuredItem = CollectionUtils.lastItem(sizeMap.current); + + return { + // We haven't measured this so we must return the minimum row height + length: initialRowHeight, + + // Offset will either be based on the lastMeasuredItem or the index + + // initialRowHeight since we can only assume that all previous items + // have not yet been measured + offset: _.isUndefined(lastMeasuredItem) ? initialRowHeight * index : lastMeasuredItem.offset + initialRowHeight, + index, + }; + }; /** * Measure item and cache the returned length (a.k.a. height) @@ -83,29 +80,26 @@ function BaseInvertedFlatList(props) { * @param {React.NativeSyntheticEvent} nativeEvent * @param {Number} index */ - const measureItemLayout = useCallback( - (nativeEvent, index) => { - const computedHeight = nativeEvent.layout.height; - - // We've already measured this item so we don't need to - // measure it again. - if (sizeMap[index]) { - return; - } - - const previousItem = sizeMap[index - 1] || {}; - - // If there is no previousItem this can mean we haven't yet measured - // the previous item or that we are at index 0 and there is no previousItem - const previousLength = previousItem.length || 0; - const previousOffset = previousItem.offset || 0; - sizeMap[index] = { - length: computedHeight, - offset: previousLength + previousOffset, - }; - }, - [sizeMap], - ); + const measureItemLayout = useCallback((nativeEvent, index) => { + const computedHeight = nativeEvent.layout.height; + + // We've already measured this item so we don't need to + // measure it again. + if (sizeMap.current[index]) { + return; + } + + const previousItem = sizeMap.current[index - 1] || {}; + + // If there is no previousItem this can mean we haven't yet measured + // the previous item or that we are at index 0 and there is no previousItem + const previousLength = previousItem.length || 0; + const previousOffset = previousItem.offset || 0; + sizeMap.current[index] = { + length: computedHeight, + offset: previousLength + previousOffset, + }; + }, []); /** * Render item method wraps the prop renderItem to render in a From 32f682cf75671c8429ac1b2e92ba581a5cde3cef Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Mon, 18 Sep 2023 10:13:52 +0100 Subject: [PATCH 3/6] Update src/components/InvertedFlatList/BaseInvertedFlatList.js Co-authored-by: Carlos Alvarez --- src/components/InvertedFlatList/BaseInvertedFlatList.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/InvertedFlatList/BaseInvertedFlatList.js b/src/components/InvertedFlatList/BaseInvertedFlatList.js index e65dc512ce57..4cdbd7518220 100644 --- a/src/components/InvertedFlatList/BaseInvertedFlatList.js +++ b/src/components/InvertedFlatList/BaseInvertedFlatList.js @@ -129,6 +129,7 @@ function BaseInvertedFlatList(props) { {...props} ref={innerRef} renderItem={renderItemFromProp} + // Native platforms do not need to measure items and work fine without this. // Web requires that items be measured or else crazy things happen when scrolling. getItemLayout={shouldMeasureItems ? getItemLayout : undefined} From 53874aa202995f1cea618a59b13b12fcdda8b8b4 Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Mon, 18 Sep 2023 10:14:04 +0100 Subject: [PATCH 4/6] Update src/components/InvertedFlatList/BaseInvertedFlatList.js Co-authored-by: Carlos Alvarez --- src/components/InvertedFlatList/BaseInvertedFlatList.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/InvertedFlatList/BaseInvertedFlatList.js b/src/components/InvertedFlatList/BaseInvertedFlatList.js index 4cdbd7518220..3be92d06292b 100644 --- a/src/components/InvertedFlatList/BaseInvertedFlatList.js +++ b/src/components/InvertedFlatList/BaseInvertedFlatList.js @@ -133,6 +133,7 @@ function BaseInvertedFlatList(props) { // Native platforms do not need to measure items and work fine without this. // Web requires that items be measured or else crazy things happen when scrolling. getItemLayout={shouldMeasureItems ? getItemLayout : undefined} + // We keep this property very low so that chat switching remains fast maxToRenderPerBatch={1} windowSize={15} From 89f05befaaba4bdedbd0f325e58bb691bfff4864 Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Mon, 18 Sep 2023 15:26:54 +0100 Subject: [PATCH 5/6] fix lint and remove todo comments --- src/components/InvertedFlatList/BaseInvertedFlatList.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/components/InvertedFlatList/BaseInvertedFlatList.js b/src/components/InvertedFlatList/BaseInvertedFlatList.js index 3be92d06292b..2ad8102f4beb 100644 --- a/src/components/InvertedFlatList/BaseInvertedFlatList.js +++ b/src/components/InvertedFlatList/BaseInvertedFlatList.js @@ -129,18 +129,14 @@ function BaseInvertedFlatList(props) { {...props} ref={innerRef} renderItem={renderItemFromProp} - // Native platforms do not need to measure items and work fine without this. // Web requires that items be measured or else crazy things happen when scrolling. + getItemLayout={shouldMeasureItems ? getItemLayout : undefined} - // We keep this property very low so that chat switching remains fast + maxToRenderPerBatch={1} windowSize={15} - - // Commenting the line below as it breaks the unread indicator test - // we will look at fixing/reusing this after RN v0.72 - // maintainVisibleContentPosition={{minIndexForVisible: 0, autoscrollToTopThreshold: 0}} /> ); } From 8774ee636de1215319de4342015ae713a9273dac Mon Sep 17 00:00:00 2001 From: Etotaziba Olei Date: Mon, 18 Sep 2023 15:29:26 +0100 Subject: [PATCH 6/6] fix prettier whitespace --- src/components/InvertedFlatList/BaseInvertedFlatList.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/components/InvertedFlatList/BaseInvertedFlatList.js b/src/components/InvertedFlatList/BaseInvertedFlatList.js index 2ad8102f4beb..5a2e0c3c8114 100644 --- a/src/components/InvertedFlatList/BaseInvertedFlatList.js +++ b/src/components/InvertedFlatList/BaseInvertedFlatList.js @@ -131,10 +131,8 @@ function BaseInvertedFlatList(props) { renderItem={renderItemFromProp} // Native platforms do not need to measure items and work fine without this. // Web requires that items be measured or else crazy things happen when scrolling. - getItemLayout={shouldMeasureItems ? getItemLayout : undefined} // We keep this property very low so that chat switching remains fast - maxToRenderPerBatch={1} windowSize={15} />