Fix regression of VirtualizedList jumpy header #22025

danilobuerger · 2018-10-30T18:16:52Z

Fixes #20956, #21361, #21198, #21468
Keeps the intended outcome of #18105

Test Plan:

See Video and Demo in #20956

Release Notes:

[GENERAL] [BUGFIX] [VirtualizedList] - Fix regression for jumpy header

analysis-bot

Code analysis results:

eslint found some issues.

analysis-bot · 2018-10-30T18:18:24Z

Libraries/Lists/VirtualizedList.js

@@ -805,7 +805,9 @@ class VirtualizedList extends React.PureComponent<Props, State> {
            if (stickyIndicesFromProps.has(ii + stickyOffset)) {
              const initBlock = this._getFrameMetricsApprox(lastInitialIndex);
              const stickyBlock = this._getFrameMetricsApprox(ii);
-              const leadSpace = stickyBlock.offset - initBlock.offset;
+              const leadSpace =
+                stickyBlock.offset - initBlock.offset -


prettier/prettier: Insert ⏎···············

llostris · 2018-12-07T18:50:37Z

Considering there's a PR ready to fix this issue, is there anything we can do to help out in making it approved and merged?
It's affecting our app as well.

danilobuerger · 2018-12-08T10:26:35Z

I still have a PR thats been open now for 8 months without a review, so I am not getting my hopes up for this one either. Its really disappointing how contributions are handled in this repository.

wontonst · 2018-12-10T13:33:13Z

I'm hitting this issue on prod and this PR resolves the issue, can we get eyes on this PR please?

cjroth · 2018-12-31T04:40:25Z

This fixes the issue for me as well, thank you! Would be great to see this merged.

hamidhadi · 2019-01-02T02:24:49Z

I tested these changes but the scroll is junkier than before 😐

danilobuerger · 2019-01-02T19:39:19Z

@hamidhadi as someone else pointed out in your issue, maybe you just didn't implement getItemLayout correctly. It would be nice to continue that in your issue and not here.

hamidhadi · 2019-01-02T20:25:40Z

@danilobuerger I just shared my experience with your changes.
It seems my getItemLayout is working properly. Check it out

danibonilha · 2019-01-04T23:16:35Z

@danibonilha I just shared my experience with your changes.
It seems my getItemLayout is working properly. Check it out

Hey @hamidhadi, it seems you tagged the wrong person.

mjniday

Tested the changes locally, and this works great!

modeinnovations · 2019-01-17T06:49:52Z

This PR has definitely improved the jumpy behaviour of my sectionList but unfortunately the issue persists.
On a 24 section list with 260 total items the headers hold together until about 2/3 of the way down, at which point the headers jump about 4 pixels (which is an improvement from jumping 100-200 pixels before).
Are there any changes I should be looking at to improve this? My best guess is it is something to do with the list not being able to render as fast as a user can scroll.
My getItemLayout prop is setup correctly and I'm using react-native-section-list-get-item-layout (Crazy long name).
Any thoughts would be appreciated. Not a huge issue but staring at it all day it starts to bug me.

Freddy03h · 2019-01-17T10:05:53Z

@modeinnovations @hamidhadi It seem there is an issue using getItemLayout (in the libs or in your code, I don't know) because I have no more issue on a SectionList with 51 sections and 473 items without getItemLayout.

Personally I already use this fix on production since a month !

Fixes: zulip#3286 and zulip#3176 Related but not ready to merge: facebook/react-native#22025

Fixes: zulip#3286 and zulip#3176 Related fix upstream but not ready to merge: facebook/react-native#22025 A planed minor redesign (discussed with Rishi) to make the Unread tabs less flashy and 'circus-colors-like' requires us to remove the stickiness from the headers. Even though though the design is still WIP, it makes sense to use this change as a fix to an issue that was already reported multiple times. This makes the UnreadCards SectionList have normal, non-sticky headers. We have to set `stickySectionHeadersEnabled` explicitly to `false` as the defaults differ for iOS (true) and Android (false)

cpojer

Hey everyone,

sorry we missed this for a long time. We promise to get better at reviewing and landing PRs. I'm gonna go ahead and ship this.

facebook-github-bot

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

react-native-bot · 2019-02-04T16:25:05Z

@danilobuerger merged commit e4fd9ba into facebook:master.

Summary: Fixes facebook#20956, facebook#21361, facebook#21198, facebook#21468 Keeps the intended outcome of facebook#18105 Pull Request resolved: facebook#22025 Differential Revision: D13941915 Pulled By: cpojer fbshipit-source-id: 59a0a834ea2d0dd4678e80a82ddaf95cecf87d38

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 30, 2018

danilobuerger mentioned this pull request Oct 30, 2018

[Regression] VirtualizedList jumpy header #20956

Closed

3 tasks

analysis-bot suggested changes Oct 30, 2018

View reviewed changes

Fix regression of VirtualizedList jumpy header

c0043ae

danilobuerger force-pushed the patch-1 branch from e801d7b to c0043ae Compare October 30, 2018 18:33

react-native-bot added 🔶Lists Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. labels Oct 30, 2018

hramos removed 🔶 Components labels Nov 1, 2018

This was referenced Nov 29, 2018

feature, home screen: Add ability to mark messages in bulk, mute topics etc zulip/zulip-mobile#3175

Open

Headings jitter on scroll through unreads or users list zulip/zulip-mobile#3176

Closed

rye mentioned this pull request Dec 26, 2018

List items are misbehaving StoDevX/AAO-React-Native#3348

Closed

danilobuerger mentioned this pull request Jan 2, 2019

Jumpy sticky section headers in SectionList #22775

Closed

3 tasks

itsmepetrov approved these changes Jan 6, 2019

View reviewed changes

wontonst approved these changes Jan 7, 2019

View reviewed changes

cjroth added a commit to cjroth/react-native that referenced this pull request Jan 10, 2019

fix virtualized list (facebook#22025)

fb9fab8

sfhallahan approved these changes Jan 10, 2019

View reviewed changes

mjniday approved these changes Jan 10, 2019

View reviewed changes

llostris approved these changes Jan 10, 2019

View reviewed changes

borisyankov added a commit to borisyankov/zulip-mobile that referenced this pull request Jan 22, 2019

No sticky headers in Unread

cfd516c

Fixes: zulip#3286 and zulip#3176 Related but not ready to merge: facebook/react-native#22025

borisyankov mentioned this pull request Jan 22, 2019

No sticky headers in Unread tab zulip/zulip-mobile#3305

Closed

hramos removed 🔶Components labels Jan 24, 2019

0x0E approved these changes Feb 2, 2019

View reviewed changes

cpojer approved these changes Feb 4, 2019

View reviewed changes

facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Feb 4, 2019

facebook-github-bot approved these changes Feb 4, 2019

View reviewed changes

facebook-github-bot closed this in e4fd9ba Feb 4, 2019

facebook locked as resolved and limited conversation to collaborators Feb 4, 2019

react-native-bot added the Merged This PR has been merged. label Feb 4, 2019

hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix regression of VirtualizedList jumpy header #22025

Fix regression of VirtualizedList jumpy header #22025

danilobuerger commented Oct 30, 2018

analysis-bot left a comment

analysis-bot Oct 30, 2018

llostris commented Dec 7, 2018 •

edited

Loading

danilobuerger commented Dec 8, 2018

wontonst commented Dec 10, 2018

cjroth commented Dec 31, 2018 •

edited

Loading

hamidhadi commented Jan 2, 2019

danilobuerger commented Jan 2, 2019

hamidhadi commented Jan 2, 2019 •

edited

Loading

danibonilha commented Jan 4, 2019

mjniday left a comment

modeinnovations commented Jan 17, 2019

Freddy03h commented Jan 17, 2019

cpojer left a comment

facebook-github-bot left a comment

react-native-bot commented Feb 4, 2019

Fix regression of VirtualizedList jumpy header #22025

Fix regression of VirtualizedList jumpy header #22025

Conversation

danilobuerger commented Oct 30, 2018

Test Plan:

Release Notes:

analysis-bot left a comment

Choose a reason for hiding this comment

analysis-bot Oct 30, 2018

Choose a reason for hiding this comment

llostris commented Dec 7, 2018 • edited Loading

danilobuerger commented Dec 8, 2018

wontonst commented Dec 10, 2018

cjroth commented Dec 31, 2018 • edited Loading

hamidhadi commented Jan 2, 2019

danilobuerger commented Jan 2, 2019

hamidhadi commented Jan 2, 2019 • edited Loading

danibonilha commented Jan 4, 2019

mjniday left a comment

Choose a reason for hiding this comment

modeinnovations commented Jan 17, 2019

Freddy03h commented Jan 17, 2019

cpojer left a comment

Choose a reason for hiding this comment

facebook-github-bot left a comment

Choose a reason for hiding this comment

react-native-bot commented Feb 4, 2019

llostris commented Dec 7, 2018 •

edited

Loading

cjroth commented Dec 31, 2018 •

edited

Loading

hamidhadi commented Jan 2, 2019 •

edited

Loading