Add support for sticky header and fix static header issues with numColumns #129

naqvitalha · 2022-02-25T07:47:22Z

Fixes header implementation and adds Sticky headers.

Changes

Header now goes on top of autolayout and footer as well. New offsets added to to these are taken into account on the native side for blank and visible window detections.
Sticky header implementation and fixes to correct them for use along with headers.
Code refactor to make things more manageable
New pure component wrapper that allows for preventing re-renders in an easier way
Preventing some props to be passed to ScrollView which can impact behaviour of the list

Tophatting

I've tested the changes in Android/iOS. You can add header or pass sticky indices to existing samples.

resolves #95
resolves #44
resolves #96

naqvitalha · 2022-02-28T15:46:23Z

src/errors/ExceptionList.ts

    type: "InvariantViolation",
  },
+  stickyWhileHorizontalNotSupported: {


FlatList does the same, these features don't make much sense when in horizontal mode

naqvitalha · 2022-02-28T15:47:26Z

src/WrapperComponent.tsx

+  [other: string]: unknown;
+}
+
+export class PureComponentWrapper extends React.PureComponent<PureComponentWrapperProps> {


Any props can be passed to this wrapper and it rerenders if any prop changes. This view can also show or hide the view on request.

naqvitalha · 2022-02-28T15:48:18Z

ios/Sources/AutoLayoutView.swift

                    renderAheadOffset: renderAheadOffset,
                    windowSize: windowSize,
                    isHorizontal: horizontal
                )
            else { return }
-
+            let isNextCellVisible = isWithinBounds(


This is only to make blank events more accurate. Existing compute is not impacted

can we add this to the tests, too?

I see that tests are not setup for iOS clearGaps method. It's not very straightforward, I'll track it in another issue. I've added the tests for Android. Good that you pointed this somehow I forgot.

Tracking here: #133

yeah, let's not increase the scope of this PR, thanks!

naqvitalha · 2022-02-28T15:51:33Z

.eslintrc.js

@@ -16,6 +16,7 @@ module.exports = {
    "@shopify/jsx-no-hardcoded-content": "off",
    "@shopify/jest/no-vague-titles": "off",
    "@shopify/jsx-no-complex-expressions": "off",
+    "@shopify/react-prefer-private-members": "off",


We can have certain public APIs that are not used internally. This rule didn't make sense for a library like this.

android/src/main/kotlin/com/shopify/reactnative/recycler_flat_list/AutoLayoutShadow.kt

naqvitalha · 2022-02-28T16:00:53Z

android/src/main/kotlin/com/shopify/reactnative/recycler_flat_list/AutoLayoutShadow.kt

-        blankOffsetAtStart = lastMinBound - actualScrollOffset
-        blankOffsetAtEnd = actualScrollOffset + windowSize - renderOffset - lastMaxBound
+    fun computeBlankFromGivenOffset(actualScrollOffset: Int, distanceFromWindowStart: Int, distanceFromWindowEnd: Int): Int {
+        val actualScrollOffset = actualScrollOffset - offsetFromStart;


By subtracting minY of autolayout from scroll offset we can fix most of the compute

naqvitalha · 2022-02-28T16:01:45Z

android/src/main/kotlin/com/shopify/reactnative/recycler_flat_list/AutoLayoutView.kt

+            val startOffset = if (alShadow.horizontal) left else top
+            val endOffset = if (alShadow.horizontal) right else bottom
+
+            val distanceFromWindowStart = kotlin.math.max(startOffset - scrollOffset, 0)


Visible portions of header and footer will be removed from the blank area visible on the screen.

ios/Sources/AutoLayoutView.swift

naqvitalha · 2022-02-28T16:03:36Z

ios/Sources/AutoLayoutView.swift

    /// 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]) {
        var maxBound: CGFloat = 0
        var minBound: CGFloat = CGFloat(Int.max)
-
+        var maxBoundNextCell: CGFloat = 0
+        let correctedScrollOffset = scrollOffset - (horizontal ? self.frame.minX : self.frame.minY)


Accounting for offset from top in compute. I've tested the accuracy in iOS as well.

naqvitalha · 2022-02-28T16:16:37Z

src/RecyclerFlatList.tsx

+   * You can also include any padding that might have been added to the start of the list. RecyclerFlatList needs to this to determine
+   * where the first item in the list starts from.
+   */
+  estimatedHeaderSize?: number;


This is only required if someone wants to use initialScrollIndex and expect a lot of accuracy. It's rare but enabling that use case.

naqvitalha · 2022-02-28T16:19:45Z

src/RecyclerFlatList.tsx

  render() {
    if (this.state.dataProvider.getSize() === 0) {
-      return this.props.ListEmptyComponent || null;


naqvitalha · 2022-02-28T16:20:09Z

src/RecyclerFlatList.tsx

-        };
-      }
+    const {
+      drawDistance,


preventing props being passed to scrollview unless we want some of them to

naqvitalha · 2022-02-28T16:20:40Z

src/RecyclerFlatList.tsx

-        {children}
-      </AutoLayoutView>
+      <>
+        <PureComponentWrapper


header with no unnecessary renders using the new wrapper

naqvitalha · 2022-02-28T16:20:48Z

src/RecyclerFlatList.tsx

+        >
+          {children}
+        </AutoLayoutView>
+        <PureComponentWrapper


footer with no unnecessary renders using the new wrapper

naqvitalha · 2022-02-28T16:22:14Z

src/RecyclerFlatList.tsx

    );
  };

  private itemContainer = (props, parentProps, children) => {
    return (
-      <ItemContainer {...props} index={parentProps.index}>
+      <ItemContainer


styling fixes to support flex:1 in grid mode without breaking other layouts. Consistent with Flatlist now

naqvitalha · 2022-02-28T16:27:56Z

src/RecyclerFlatList.tsx

  public scrollToEnd(params?: { animated?: boolean | null | undefined }) {
    this.rlvRef?.scrollToEnd(Boolean(params?.animated));
  }

-  // eslint-disable-next-line @shopify/react-prefer-private-members
+  // TODO: Improve accuracy with headers


Will do this later #131

As of now scrollToIndex doesn't account for header. I'll improve this as a separate PR.

android/src/main/kotlin/com/shopify/reactnative/recycler_flat_list/AutoLayoutShadow.kt

ios/Sources/AutoLayoutView.swift

fortmarek · 2022-03-01T13:56:00Z

ios/Sources/AutoLayoutView.swift

                    renderAheadOffset: renderAheadOffset,
                    windowSize: windowSize,
                    isHorizontal: horizontal
                )
            else { return }
-
+            let isNextCellVisible = isWithinBounds(


can we add this to the tests, too?

src/RecyclerFlatList.tsx

fortmarek · 2022-03-01T13:58:11Z

src/RecyclerFlatList.tsx

@@ -34,6 +39,13 @@ export interface RecyclerFlatListProps<T> extends FlatListProps<T> {
   */
  estimatedListSize?: { height: number; width: number };

+  /**
+   * Provide estimated size of the header that is going to be rendered. This can help make initialScrollIndex prop more accurate.


In this case, how exactly will the inaccuracy manifest?

If will fall short by this exact value. Let's say you want to start at index 5 and each item is 100. So we want to start at 400px but if there's a header we need to start at header size + 400px.
This only impacts initial render. We can make it better however, right now I've just provided a way to handle it. In future we can try getting rid of this.

I agree - could you create an issue for this if there already isn't one? I'm ok with the limitation for now.

fortmarek · 2022-03-01T13:59:58Z

src/RecyclerFlatList.tsx

@@ -117,6 +132,9 @@ class RecyclerFlatList<T> extends React.PureComponent<
        this.listFixedDimensionSize = props.estimatedListSize.width;
      }
    }
+    this.distanceFromWindow = props.estimatedHeaderSize;


I'd rather keep the RecyclerFlatList named estimatedHeaderSize and do the translation to how recycler-list-view calls it as late as possible. To me, it's then more obvious why we rename it. But subjective for sure)

src/RecyclerFlatList.tsx

src/WrapperComponent.tsx

Co-authored-by: Marek Fořt <marek.fort@shopify.com>

naqvitalha · 2022-03-01T21:04:09Z

@fortmarek I've addressed the comments. Regarding header size I've renamed it. I want to keep it as a workaround but I do want to explore alternative solutions. It's just not a big priority right now.

fortmarek

This is great, thanks Talha!

fortmarek · 2022-03-02T15:00:13Z

src/RecyclerFlatList.tsx

 import GridLayoutProviderWithProps from "./GridLayoutProviderWithProps";
 import CustomError from "./errors/CustomError";
 import ExceptionList from "./errors/ExceptionList";

+interface StickyProps extends StickyContainerProps {
+  children: any;


could this be JSX.Element?

naqvitalha · 2022-03-02T15:10:57Z

Thanks for the review @fortmarek. I've created another issue to track estimatedFirstItemOffset improvement. #134. Merging this.

naqvitalha added 12 commits February 24, 2022 14:31

Header and Footer now render on top and bottom of AutoLayout

b75e70d

fixed header flash issue during measure

258697b

Fixed flex handling in grid case

5acd842

fixed sticky bug

646f0a8

merge with main

5146c29

Improved blank area accuracy and makde adjustments for header and footer

ef034b5

added ios support for header corrections

e0e0486

sticky implementation complete

4f80aaf

added exceptions for horizontal mode

2a9e68a

Method name change

e1eba6c

lint fixes

3ffaa14

Prevent sticky rerenders that were not needed

a6f1483

naqvitalha changed the title ~~[Draft] Header upgrade~~ Header upgrade Feb 28, 2022

naqvitalha commented Feb 28, 2022

View reviewed changes

comment

a408c1e

naqvitalha commented Feb 28, 2022

View reviewed changes

android/src/main/kotlin/com/shopify/reactnative/recycler_flat_list/AutoLayoutShadow.kt Show resolved Hide resolved

naqvitalha commented Feb 28, 2022

View reviewed changes

ios/Sources/AutoLayoutView.swift Show resolved Hide resolved

naqvitalha commented Feb 28, 2022

View reviewed changes

naqvitalha requested review from fortmarek and davebcn87 February 28, 2022 17:18

lint fix

02a1fba

naqvitalha changed the title ~~Header upgrade~~ Add support for sticky header and fix static header issues with numColumns Feb 28, 2022

fortmarek reviewed Mar 1, 2022

View reviewed changes

naqvitalha and others added 5 commits March 1, 2022 06:28

Update src/WrapperComponent.tsx

ff36bde

Co-authored-by: Marek Fořt <marek.fort@shopify.com>

Update src/RecyclerFlatList.tsx

63e10ed

Co-authored-by: Marek Fořt <marek.fort@shopify.com>

review fixes

c68f3bd

removed reference to self

762b5ab

added tests and fixed some review comments

5531425

naqvitalha mentioned this pull request Mar 1, 2022

Style support added to list #132

Merged

fortmarek approved these changes Mar 2, 2022

View reviewed changes

naqvitalha merged commit 3788556 into main Mar 2, 2022

naqvitalha deleted the headerUpgrade branch March 2, 2022 15:11

shopify-shipit bot temporarily deployed to production March 2, 2022 17:53 Inactive

shopify-shipit bot temporarily deployed to production March 7, 2022 17:03 Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for sticky header and fix static header issues with numColumns #129

Add support for sticky header and fix static header issues with numColumns #129

naqvitalha commented Feb 25, 2022 •

edited

naqvitalha Feb 28, 2022

naqvitalha Feb 28, 2022

naqvitalha Feb 28, 2022

fortmarek Mar 1, 2022

naqvitalha Mar 1, 2022

naqvitalha Mar 1, 2022

fortmarek Mar 2, 2022

naqvitalha Feb 28, 2022

naqvitalha Feb 28, 2022

naqvitalha Feb 28, 2022

naqvitalha Feb 28, 2022

naqvitalha Feb 28, 2022

naqvitalha Feb 28, 2022

naqvitalha Feb 28, 2022

naqvitalha Feb 28, 2022

naqvitalha Feb 28, 2022

naqvitalha Feb 28, 2022

naqvitalha Feb 28, 2022

naqvitalha Feb 28, 2022

fortmarek Mar 1, 2022

fortmarek Mar 1, 2022

naqvitalha Mar 1, 2022 •

edited

fortmarek Mar 2, 2022

naqvitalha Mar 2, 2022

fortmarek Mar 1, 2022

naqvitalha commented Mar 1, 2022

fortmarek left a comment

fortmarek Mar 2, 2022

naqvitalha commented Mar 2, 2022

Add support for sticky header and fix static header issues with numColumns #129

Add support for sticky header and fix static header issues with numColumns #129

Conversation

naqvitalha commented Feb 25, 2022 • edited

Changes

Tophatting

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naqvitalha Mar 1, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naqvitalha commented Mar 1, 2022

fortmarek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naqvitalha commented Mar 2, 2022

naqvitalha commented Feb 25, 2022 •

edited

naqvitalha Mar 1, 2022 •

edited