Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add experimentalMaintainTopContentPosition support to FlashList #772

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased]

- Fix definition conflicts with previous value
- https://github.com/Shopify/flash-list/pull/795
- Add support for `experimentalMaintainTopContentPosition`
- https://github.com/Shopify/flash-list/issues/547

## [1.4.2] - 2023-03-20

Expand Down
10 changes: 5 additions & 5 deletions fixture/ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -366,9 +366,9 @@ PODS:
- React-Core
- SDWebImage (~> 5.11.1)
- SDWebImageWebPCoder (~> 0.8.4)
- RNFlashList (1.4.1):
- RNFlashList (1.4.2):
- React-Core
- RNFlashList/Tests (1.4.1):
- RNFlashList/Tests (1.4.2):
- React-Core
- RNGestureHandler (2.5.0):
- React-Core
Expand Down Expand Up @@ -597,11 +597,11 @@ SPEC CHECKSUMS:
Flipper-RSocket: d9d9ade67cbecf6ac10730304bf5607266dd2541
FlipperKit: cbdee19bdd4e7f05472a66ce290f1b729ba3cb86
fmt: ff9d55029c625d3757ed641535fd4a75fedc7ce9
glog: 85ecdd10ee8d8ec362ef519a6a45ff9aa27b2e85
glog: 476ee3e89abb49e07f822b48323c51c57124b572
libevent: 4049cae6c81cdb3654a443be001fb9bdceff7913
libwebp: 98a37e597e40bfdb4c911fc98f2c53d0b12d05fc
OpenSSL-Universal: ebc357f1e6bc71fa463ccb2fe676756aff50e88c
RCT-Folly: 803a9cfd78114b2ec0f140cfa6fa2a6bafb2d685
RCT-Folly: 4d8508a426467c48885f1151029bc15fa5d7b3b8
RCTRequired: 0f06b6068f530932d10e1a01a5352fad4eaacb74
RCTTypeSafety: b0ee81f10ef1b7d977605a2b266823dabd565e65
React: 3becd12bd51ea8a43bdde7e09d0f40fba7820e03
Expand Down Expand Up @@ -630,7 +630,7 @@ SPEC CHECKSUMS:
ReactCommon: 149e2c0acab9bac61378da0db5b2880a1b5ff59b
ReactNativePerformanceListsProfiler: b9f7cfe8d08631fbce8e4729d388a5a3f7f562c2
RNFastImage: 1f2cab428712a4baaf78d6169eaec7f622556dd7
RNFlashList: 8ec7f7454721145fe84566bb9e88bcf58981c9fe
RNFlashList: 7fbca4fc075484a9426f1610d648dbea2de94eb0
RNGestureHandler: bad495418bcbd3ab47017a38d93d290ebd406f50
RNReanimated: 3d1432ce7b6b7fc31f375dcabe5b4585e0634a43
RNScreens: 40a2cb40a02a609938137a1e0acfbf8fc9eebf19
Expand Down
98 changes: 97 additions & 1 deletion ios/Sources/AutoLayoutView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,12 @@ import UIKit
self.disableAutoLayout = disableAutoLayout
}

@objc func setExperimentalMaintainTopContentPosition(_ experimentalMaintainTopContentPosition: Bool) {
self.maintainTopContentPosition = experimentalMaintainTopContentPosition
}

private var horizontal = false
private var maintainTopContentPosition = false
private var scrollOffset: CGFloat = 0
private var windowSize: CGFloat = 0
private var renderAheadOffset: CGFloat = 0
Expand All @@ -46,9 +51,19 @@ import UIKit
/// Tracks where first pixel is drawn in the visible window
private var lastMinBound: CGFloat = 0

/// State that informs us whether this is the first render
private var isInitialRender: Bool = true

/// Id of the anchor element when using `maintainTopContentPosition`
private var anchorStableId: String = ""

/// Offset of the anchor when using `maintainTopContentPosition`
private var anchorOffset: CGFloat = 0

override func layoutSubviews() {
fixLayout()
super.layoutSubviews()
self.isInitialRender = false

guard enableInstrumentation, let scrollView = getScrollView() else { return }

Expand Down Expand Up @@ -81,6 +96,23 @@ import UIKit
return sequence(first: self, next: { $0.superview }).first(where: { $0 is UIScrollView }) as? UIScrollView
}

func getScrollViewOffset(for scrollView: UIScrollView?) -> CGFloat {
/// When using `maintainTopContentPosition` we can't use the offset provided by React
/// Native. Because its async, it is sometimes sent in too late for the position maintainence
/// calculation causing list jumps or sometimes wrong scroll positions altogether. Since this is still
/// experimental, the old scrollOffset is here to not regress previous functionality if the feature
/// doesn't work at scale.
///
/// The goal is that we can remove this in the future and get the offset from only one place 🤞
if let scrollView, maintainTopContentPosition {
return horizontal ?
scrollView.contentOffset.x :
scrollView.contentOffset.y
}

return scrollOffset
}

/// Sorts views by index and then invokes clearGaps which does the correction.
/// Performance: Sort is needed. Given relatively low number of views in RecyclerListView render tree this should be a non issue.
private func fixLayout() {
Expand All @@ -104,16 +136,58 @@ import UIKit
fixFooter()
}

/// Finds the item with the first stable id and adjusts the scroll view offset based on how much
/// it moved when a new item is added.
private func adjustTopContentPosition(
cellContainers: [CellContainer],
scrollView: UIScrollView?
) {
guard let scrollView = scrollView, !self.isInitialRender else { return }

for cellContainer in cellContainers {
let minValue = horizontal ?
cellContainer.frame.minX :
cellContainer.frame.minY

if cellContainer.stableId == anchorStableId {
if minValue != anchorOffset {
let diff = minValue - anchorOffset

let currentOffset = horizontal
? scrollView.contentOffset.x
: scrollView.contentOffset.y

let scrollValue = diff + currentOffset

scrollView.contentOffset = CGPoint(
x: horizontal ? scrollValue : 0,
y: horizontal ? 0 : scrollValue
)

// You only need to adjust the scroll view once. Break the
// loop after this
return
}
}
}
}

/// Checks for overlaps or gaps between adjacent items and then applies a correction.
/// Performance: RecyclerListView renders very small number of views and this is not going to trigger multiple layouts on the iOS side.
private func clearGaps(for cellContainers: [CellContainer]) {
let scrollView = getScrollView()
var maxBound: CGFloat = 0
var minBound: CGFloat = CGFloat(Int.max)
var maxBoundNextCell: CGFloat = 0
let correctedScrollOffset = scrollOffset - (horizontal ? frame.minX : frame.minY)
let correctedScrollOffset = getScrollViewOffset(for: scrollView)

lastMaxBoundOverall = 0
var nextAnchorStableId = ""
var nextAnchorOffset: CGFloat = 0

cellContainers.indices.dropLast().forEach { index in
let cellContainer = cellContainers[index]

let cellTop = cellContainer.frame.minY
let cellBottom = cellContainer.frame.maxY
let cellLeft = cellContainer.frame.minX
Expand Down Expand Up @@ -185,11 +259,33 @@ import UIKit
maxBoundNextCell = max(maxBound, nextCell.frame.maxY)
}
}

let isAnchorFound =
nextAnchorStableId == "" ||
nextCell.stableId == anchorStableId

if maintainTopContentPosition && isAnchorFound {
nextAnchorOffset = horizontal ?
nextCell.frame.minX :
nextCell.frame.minY

nextAnchorStableId = nextCell.stableId
}

updateLastMaxBoundOverall(currentCell: cellContainer, nextCell: nextCell)
}

if maintainTopContentPosition {
adjustTopContentPosition(
cellContainers: cellContainers,
scrollView: scrollView
)
}

lastMaxBound = maxBoundNextCell
lastMinBound = minBound
anchorStableId = nextAnchorStableId
anchorOffset = nextAnchorOffset
}

private func updateLastMaxBoundOverall(currentCell: CellContainer, nextCell: CellContainer) {
Expand Down
1 change: 1 addition & 0 deletions ios/Sources/AutoLayoutViewManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ @interface RCT_EXTERN_MODULE(AutoLayoutViewManager, RCTViewManager)
RCT_EXPORT_VIEW_PROPERTY(enableInstrumentation, BOOL)
RCT_EXPORT_VIEW_PROPERTY(disableAutoLayout, BOOL)
RCT_EXPORT_VIEW_PROPERTY(onBlankAreaEvent, RCTDirectEventBlock)
RCT_EXPORT_VIEW_PROPERTY(experimentalMaintainTopContentPosition, BOOL)

@end
5 changes: 5 additions & 0 deletions ios/Sources/CellContainer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,13 @@ import Foundation

@objc class CellContainer: UIView {
var index: Int = -1
var stableId: String = ""

@objc func setIndex(_ index: Int) {
self.index = index
}

@objc func setStableId(_ stableId: String) {
self.stableId = stableId
}
}
1 change: 1 addition & 0 deletions ios/Sources/CellContainerManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@
@interface RCT_EXTERN_MODULE(CellContainerManager, RCTViewManager)

RCT_EXPORT_VIEW_PROPERTY(index, NSInteger)
RCT_EXPORT_VIEW_PROPERTY(stableId, NSString)

@end
14 changes: 11 additions & 3 deletions src/FlashList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,6 @@ class FlashList<T> extends React.PureComponent<
newState.numColumns,
nextProps
);
// RLV retries to reposition the first visible item on layout provider change.
// It's not required in our case so we're disabling it
newState.layoutProvider.shouldRefreshWithAnchoring = false;
}
if (nextProps.data !== prevState.data) {
newState.data = nextProps.data;
Expand All @@ -188,6 +185,10 @@ class FlashList<T> extends React.PureComponent<
newState.extraData = { value: nextProps.extraData };
}
newState.renderItem = nextProps.renderItem;

// RLV retries to reposition the first visible item on layout provider change.
// It's not required in our case so we're disabling it
newState.layoutProvider.shouldRefreshWithAnchoring = false;
return newState;
}

Expand Down Expand Up @@ -465,6 +466,9 @@ class FlashList<T> extends React.PureComponent<
onBlankAreaEvent={this.props.onBlankArea}
onLayout={this.updateDistanceFromWindow}
disableAutoLayout={this.props.disableAutoLayout}
experimentalMaintainTopContentPosition={
this.props.experimentalMaintainTopContentPosition
}
>
{children}
</AutoLayoutView>
Expand Down Expand Up @@ -500,6 +504,10 @@ class FlashList<T> extends React.PureComponent<
...getCellContainerPlatformStyles(this.props.inverted!!, parentProps),
}}
index={parentProps.index}
stableId={
/* Empty string is used so the list can still render without an extractor */
this.props.keyExtractor?.(parentProps.data, parentProps.index) ?? ""
}
>
<PureComponentWrapper
extendedState={parentProps.extendedState}
Expand Down
8 changes: 8 additions & 0 deletions src/FlashListProps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,4 +332,12 @@ export interface FlashListProps<TItem> extends ScrollViewProps {
* `false` again.
*/
disableAutoLayout?: boolean;

/**
* If enabled, FlashList will try and maintain the position of the list when items are added from the top.
* This prop requires you define a `keyExtractor` function. The `keyExtractor` is used to compute the list
* top anchor. Without it, the list will fail to render. If in debug mode, you may see flashes if new data
* comes in quickly. If this happens, please confirm you see this in release mode before reporting an issue.
*/
experimentalMaintainTopContentPosition?: boolean;
}
4 changes: 4 additions & 0 deletions src/native/auto-layout/AutoLayoutView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export interface AutoLayoutViewProps {
onBlankAreaEvent?: BlankAreaEventHandler;
onLayout?: (event: LayoutChangeEvent) => void;
disableAutoLayout?: boolean;
experimentalMaintainTopContentPosition?: boolean;
}

class AutoLayoutView extends React.Component<AutoLayoutViewProps> {
Expand Down Expand Up @@ -63,6 +64,9 @@ class AutoLayoutView extends React.Component<AutoLayoutViewProps> {
listeners.length !== 0 || Boolean(this.props.onBlankAreaEvent)
}
disableAutoLayout={this.props.disableAutoLayout}
experimentalMaintainTopContentPosition={Boolean(
this.props.experimentalMaintainTopContentPosition
)}
>
{this.props.children}
</AutoLayoutViewNativeComponent>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,5 @@ export interface AutoLayoutViewNativeComponentProps {
onBlankAreaEvent: OnBlankAreaEventHandler;
enableInstrumentation: boolean;
disableAutoLayout?: boolean;
experimentalMaintainTopContentPosition?: boolean;
}