[Tracking] Pagination issues

[bfitzexpensify]

HOLD on #16660

This is a tracking issue for a number of different pagination issues:

• [$16000] Pagination seems broken for chats (please see #12054) #7860
• Web - Chat - Unable to scroll to the beginning of a conversation with a user who has a lot of messages #12006
• [HOLD #12054] [$1000] Conversation DM bounces, flickers, and lags when scrolling downward in a long chat #2545

[bfitzexpensify]

PRs to fix these:

• Fix Inverted VirtualizedList's baseline measurement for items' offset (1st of 4 problems that cause #1254) necolas/react-native-web#2412
• Fix artificial scroll limit expanding on scroll to un-measured area (2nd of 4 problems that cause #1254) necolas/react-native-web#2413
• Fix VirtualizedList skips items for offset measuring when the user scrolls very fast (3rd of 4 problems that cause #1254) necolas/react-native-web#2414
• Prevent VirtualizedList to save offsets equal to zero if item is not the first item of list (4th of 4 problems that cause #1254) necolas/react-native-web#2415

[bfitzexpensify]

Update from Lucio in #7860 (comment):

Update: right now I'm testing if the problems and solutions stated in #7860 (comment) still apply to the #7860 (comment).

[bfitzexpensify]

Latest update from Lucio in #7860 (comment):

Update: I am still applying the react-native's VirtualizedList new changes to react-native-web, there are many files to copy and paths to resolve.

@roryabraham did Lucio's issue here in that issue get resolved?

[roryabraham]

Hey @LucioChavezFuentes, I just went through and reviewed all your pull requests (sorry for the delay, btw). Overall they look great, but 3/4 of them should be made against the upstream React Native instead of react-native-web. Can you please:

• Open three new PRs against React Native instead of React Native web, according to the review comments I left in:
  • Fix artificial scroll limit expanding on scroll to un-measured area (2nd of 4 problems that cause #1254) necolas/react-native-web#2413
  • Fix VirtualizedList skips items for offset measuring when the user scrolls very fast (3rd of 4 problems that cause #1254) necolas/react-native-web#2414
  • Prevent VirtualizedList to save offsets equal to zero if item is not the first item of list (4th of 4 problems that cause #1254) necolas/react-native-web#2415
• Once you've completed the previous step, can you also please open duplicate pull requests against the Expensify fork of React Native, so that we can benefit from these changes sooner (without having to wait for upstream review + release)?
  • Note: I just created a new branch in our fork called Expensify-0.71.0-alpha1, please use that as the base for pull requests in our fork.
• Update Fix Inverted VirtualizedList's baseline measurement for items' offset (1st of 4 problems that cause #1254) necolas/react-native-web#2412 according to the review I left there, then open a duplicate PR against our react-native-web fork?

Thanks, and let me know if there's anything else I can do to support you? Let's keep pushing to get this done ASAP 🚀

[roryabraham]

@LucioChavezFuentes please link the PRs you open against the React Native upstream repo here 🙇🏼

[LucioChavezFuentes]

I made the PRs in react-native:

1. Prevent the tail-spacer gets negative values in VirtualizedList on scrolling toward new items with dynamic height. facebook/react-native#35413
2. fix items skip measure when items are expensive to render facebook/react-native#35414
3. Prevent VirtualizedList saves items with offsets equal to or below zero and lengths equal to zero facebook/react-native#35415

[bfitzexpensify]

We're still moving here, melvin

[roryabraham]

@LucioChavezFuentes just as a heads-up, we are working on upgrading our fork to RN 0.71, and I'll ping you in here as soon as that's done. Then we'd like duplicate pull requests for the upstream created against our fork (so that we can benefit from the changes without waiting for Meta's review + release process)

[roryabraham]

Our React Native fork has been upgraded to 0.71.0, but we still need to publish it

[roryabraham]

@LucioChavezFuentes Can you provide an update to let us know the status of all the upstream PRs?

Also, I'm still not seeing any pull requests from you in either our React Native fork or our react-native-web fork

[LucioChavezFuentes]

Hi @roryabraham, I tried to explain where I am heading here.

Currently I am working on the root cause of the first problem of my proposal. The first problem exists because Web's onLayout changes layout values when the element measured have transformations while RN's onLayout ignores transformations and returns the same layout values as if the element has no transformation at all.
VirtualizedList is built based on Native's onLayout behavior so it breaks with the current Web's onLayout behavior.

I am looking for a solution to make Web’s onLayout behave the same as its native counterpart by reverting transformations measures while still using ‘getBoundClientRect’. Apparently, I can't use ‘offsetTop’ because react-native-web had used it previously and later replaced it to fix the following issues: 1037 1151.

Also, I'm still not seeing any pull requests from you in either our React Native fork or our react-native-web fork

My proposal is applicable only on Web. If there is urgency in fixing issue #7860 we can merge my solutions in react-native-web fork. But these solutions may not be merged in upstream repositories as they don’t fix the root issues.

I can not reproduce the problems stated in my proposal in Native platforms therefore I don't think uploading the solutions to react-native fork will help, except for Problem 2 but I still don't know its root issue and it has the lowest impact.

[roryabraham]

@LucioChavezFuentes Thanks for the update here. Let us know if we can support you at all. We may be migrating away from VirtualizedList to a different list component in the future, but if you can address the root cause of the onLayout incompatibility between React Native and react-native-web we will still pay out for this. We'd rather only merge solutions in our fork of react-native-web that we expect with a reasonable degree of confidence will be accepted in the upstream.

[roryabraham]

Still on HOLD but we're upgrading RN to 0.72 presently and will follow-up with RNW 0.19 soon thereafter

[roryabraham]

Still on HOLD

[roryabraham]

PR to upgrade to RNW 0.19 is here

[bfitzexpensify]

Looks like we're still working through #24482

[roryabraham]

We are very close on the RNW upgrade. Being actively discussed in slack daily

[LucioChavezFuentes]

@roryabraham, I noticed that React-Native-Web has been updated and already deployed to production. I test the list and it works great. Can we say this issue has been concluded?

[roryabraham]

Wow, sorry for the long delay and thanks very much for all your patience here @LucioChavezFuentes. Looks like we can pay out the other half of the bounty here at long last 🎉

[melvin-bot]

Current assignee @bfitzexpensify is eligible for the NewFeature assigner, not assigning anyone new.

[roryabraham]

Marking this as a new feature just for accounting reasons

[roryabraham]

@bfitzexpensify let's pay this out ASAP

[bfitzexpensify]

Nice one! Remainder of the contract has been paid out.

[LucioChavezFuentes]

Thank you so much!!