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(#814): update sticky headers when data changes without changing s… #1267

Conversation

MateWW
Copy link
Contributor

@MateWW MateWW commented Jul 10, 2024

Description

Fixes (issue #814)

To mitigate that issue we need to bypass PureComponentWrapper
as before it has been only reacting on sticky header index change.

The easies way to understand that issue is SectionList example.
Let's imagine our stickyHeaderIndices contains 3 sections.
It will contain the following values [0, 5, 10]

When we remove first section it will remove element with index 0
with all associated items till next sticky header.
It will result in the following shift in sticky indexes array:

0 -> x
5 -> 0
10 -> 5

Based on that PureComponentWrapper will not recognise that data changed
as it receives only index of current sticky header which will be constantly equal to 0

Reviewers’ hat-rack 🎩

  • check if sticky header is rendering properly

Screenshots or videos (if needed)

Before After
https://github.com/Shopify/flash-list/assets/16048381/7321e098-e0af-4bda-9a78-7d53196f9ea3 https://github.com/Shopify/flash-list/assets/16048381/67bc12dc-874a-45e4-82fe-65a79f3897fb

Checklist

@MateWW MateWW force-pushed the fix/814/updating-first-sticky-header-when-data-changed branch from f516080 to 7c5f69b Compare July 10, 2024 18:27
@MateWW MateWW force-pushed the fix/814/updating-first-sticky-header-when-data-changed branch from 286481a to 0a984c3 Compare July 13, 2024 13:13
@naqvitalha naqvitalha merged commit 88d6f7e into Shopify:main Sep 12, 2024
11 checks passed
@MateWW MateWW deleted the fix/814/updating-first-sticky-header-when-data-changed branch September 27, 2024 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants