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

fix: unexpected jumping of list while using sticky headers #47345

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MateWW
Copy link
Contributor

@MateWW MateWW commented Oct 31, 2024

Summary:

Part 1

Currently while using sticky headers inside SectionList or FlatList we can be impacted by some wired jumping behaviour.
This behaviour comes form 1 very small missed aspect when implementing sticky elements inside ScrollView.

The issue is related to the fact that ScrollView is wrapping elements defined inside stickyHeaderIndices with it's own component called StickyHeaderComponent that allows to manipulate and keep the header in the right coordinates.
The pain point is that this component is component has position: 'relative' style defined.
That causes the child component to return coordinates relative to that StickyHeaderComponent instead returning coordinates relative to ScrollView root like every non sticky children.

Playground
https://snack.expo.dev/AWCvfHHhsHEUeoUXOPDJh

Part 2

VirtualisedList which is a base for both FlatList and SectionList is actively using onLayout coordinates to render spacers/placeholders for non visible components.
To acquire that coordinates it creates another wrapper around each item inside CellRenderer which supply onLayout property.
The simplified structure looks like this:
image

Summary

Now when onLayout is triggered inside CellRenderer which is rendered inside StickyHeaderComponent the coordinates are relative to StickyHeaderComponent and always returns offset as 0.

Now when spacers/placeholders size calculation is happening it is doing the following math:

lastItemCoordinates.offset + lastItemCoordinates.length - firstItemCoordinates.offset 

// What for instance when firstItem is sticky header results in
2500 + 50 - 0 = 2550

Changelog:

[GENERAL] [FIXED] - FlatList and SectionList odd jumping while using sticky headers

Test Plan:

Platform Before After
iOS
ios-scrolling-issue.mov
ios-fixed-scrolling-issue.mov
Android
android-scrolling-issue.mov
android-fixed-scrolling-issue.mp4

Reproduction code:

Click me to expand code!
/**
 * Copyright (c) Meta Platforms, Inc. and affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 *
 * @format
 * @flow strict-local
 */

'use strict';
import SectionListBaseExample from './SectionListBaseExample';
import * as React from 'react';

function repeat(array, count) {
  let result = [];
  for (let i = 0; i < count; i++) {
    result = result.concat(array);
  }
  return result;
}

const DATA = repeat(
  [
    {
      title: 'Main dishes',
      data: [
        'Pizza',
        'Burger',
        'Risotto',
        'Pizza',
        'Burger',
        'Risotto',
        'Pizza',
        'Burger',
        'Risotto',
        'Pizza',
        'Burger',
        'Risotto',
        'Pizza',
        'Burger',
        'Risotto',
        'Pizza',
        'Burger',
        'Risotto',
      ],
    },
    {
      title: 'Sides',
      data: ['French Fries', 'Onion Rings', 'Fried Shrimps'],
    },
    {
      title: 'Empty section',
      data: [],
    },
    {
      title: 'Drinks',
      data: ['Water', 'Coke', 'Beer'],
    },
    {
      title: 'Desserts',
      data: ['Cheesecake', 'Ice Cream'],
    },
  ],
  100,
).map((item, index) => ({
  ...item,
  data: index > 0 ? repeat(item.data, Math.random() * 10 + 1) : item.data,
}));

const RenderHeader = () => {
  return <></>;
};

export function SectionList_ScrollingIssue(): React.Node {
  const [output, setOutput] = React.useState(
    'stickySectionHeadersEnabled true',
  );
  const [exampleProps, setExampleProps] = React.useState({
    // debug: true,
    key: 'exampleKey',
    stickySectionHeadersEnabled: true,
    sections: DATA,
    refreshControl: undefined,
    ListHeaderComponent: <RenderHeader />,
    ListFooterComponent: undefined,
    testID: 'testId',
    keyExtractor: (item, index) => `tx_${item}_${index}`,
    scrollEnabled: true,
    onEndReached: () => {},
    onEndReachedThreshold: 0.3,
    overScrollMode: 'always',
    keyboardShouldPersistTaps: 'handled',
    nestedScrollEnabled: true,
    showVerticalScrollIndicator: false,
    contentInsetAdjustmentBehavior: 'automatic',
    initialNumToRender: 20,
    maxToRenderPerBatch: 20,
    keyboardDismissMode: 'on-drag',
  });

  const onTest = () => {
    setExampleProps({
      stickySectionHeadersEnabled: !exampleProps.stickySectionHeadersEnabled,
    });
    setOutput(
      `stickySectionHeadersEnabled ${(!exampleProps.stickySectionHeadersEnabled).toString()}`,
    );
  };

  return (
    <SectionListBaseExample exampleProps={exampleProps} testOutput={output} />
  );
}

export default {
  title: 'SectionList scrolling issue',
  name: 'SectionList-scrollingissue',
  description: 'Toggle sticky headers on/off',
  render: function (): React.MixedElement {
    return <SectionList_ScrollingIssue />;
  },
};

Impacted versions:

Version Affected architecture
0.76 - 0.68 new and old
<0.68 wasn't able to build

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Oct 31, 2024
@MateWW MateWW force-pushed the fix/sticky-header-calculation branch 8 times, most recently from 3c876ee to 4d4593d Compare November 6, 2024 20:55
Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this and for the detailed description in the Summary.
The code looks good to me, but I'm not the best person for review this.
I'll import the change to get ore eyes on it.

@MateWW MateWW force-pushed the fix/sticky-header-calculation branch 3 times, most recently from 3bb480c to f87266a Compare February 24, 2025 20:00
@MateWW MateWW force-pushed the fix/sticky-header-calculation branch from f87266a to 6e68ac6 Compare February 24, 2025 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants