Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.36k stars 2.78k forks source link

[HOLD #12054] [$1000] Conversation DM bounces, flickers, and lags when scrolling downward in a long chat #2545

Closed isagoico closed 10 months ago

isagoico commented 3 years ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Expected Result:

Scrolling through a chat DM should be smooth (doesn't bounce, flicker, or lag) regardless of conversation length.

Actual Result:

The chat DM bounces when scrolling downward in a long conversation.

Action Performed:

  1. Open the Expensify.cash app on an Android device
  2. Navigate to a chat DM that has a long conversation of back and forth.
  3. Scroll upwards for a few seconds.
  4. Stop scrolling upward, and then slowly scroll downard at a reading speed.

Note: lag and flickering behavior is especially worsened if you:

Workaround:

You can still scroll, but it's a poor UX and the scroll should be smooth.

Platform:

Where is this issue occurring? All platforms

Web iOS Android Desktop App Mobile Web

Version Number: 1.0.65-0

Notes/Photos/Videos:

https://user-images.githubusercontent.com/44479856/115792143-beb73900-a397-11eb-9183-c9f9dc35da61.mp4

Expensify/Expensify Issue URL: https://www.upwork.com/jobs/~0140ca576a3b244cff

View all open jobs on Upwork


From @quinthar https://expensify.slack.com/archives/C01GTK53T8Q/p1619067337008500

ISSUE: scrolling backwards seems to work great, but when you scroll forwards it Jitters all over the place. It's very hard to describe, but it's perfectly reproducible on android. Just go into a long conversation, scroll back a few pages, and then scroll forward slowly, and it bounces all over the place.

MelvinBot commented 3 years ago

Triggered auto assignment to @mateocole (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

MelvinBot commented 3 years ago

Triggered auto assignment to @ctkochan22 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

ctkochan22 commented 3 years ago

Labels look good

mallenexpensify commented 3 years ago

@ctkochan22 bouncing back to you - do you think this can be worked on by a Contributor?

ctkochan22 commented 3 years ago

@mallenexpensify Shoot sorry. Yes I think so.

mallenexpensify commented 3 years ago

Thanks @ctkochan22 , in the future, if a contributor can work on an issue apply the External label so it can be posted to Upwork. Adding it now.

MelvinBot commented 3 years ago

Triggered auto assignment to @michaelhaxhiu (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

michaelhaxhiu commented 3 years ago

Will post today.

michaelhaxhiu commented 3 years ago

Just tested on my Android, and I was able to reproduce successfully. Posted to Upwork as of right now ✅ .

MelvinBot commented 3 years ago

Triggered auto assignment to @Julesssss (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

Julesssss commented 3 years ago

CC @kidroca, as he had some ideas for these improvements here. Perhaps you would be interested in this issue?

kidroca commented 3 years ago

I don't think this is related to cache.

When I studied the effects of virtualization I came to a similar situation when I had a large chat: https://github.com/Expensify/Expensify.cash/issues/2629#issuecomment-832516464

My theory (short version) For big chats you reach the virtualization window boundary As you scroll down an item is removed from the top Another item is added on the bottom Native, does not provide getItemLayout to the flat list It has to put the item, render it and then shift content if it has to (only when scrolling down)

Implementing a fast getItemLayout would probably be hard. There's an open ticket about a replacement of the FlatList component which might solve the problem here entirely

In theory this should be recreatable on iOS as well, maybe the list handles better there as the devices are beefier. Any chance someone could try this on the oldest iPhone supported?

michaelhaxhiu commented 3 years ago

Any chance someone could try this on the oldest iPhone supported?

This would be good to ask in #expensify-open-source I think, as we have more eyes and exposure there. Can you post it there @kidroca? I agree with the idea of seeing if old iOS devices are included in the scope of the issue.

rdjuric commented 3 years ago

@kidroca @michaelhaxhiu Here's a test running on an old iPhone 5s.

https://user-images.githubusercontent.com/53711423/122099933-f6b38900-cde8-11eb-85a5-ec7a9b97f9ec.mov

It's from the build in the app store, so probably a few versions behind our main. Feels smooth on iOS and doesn't flicker.

michaelhaxhiu commented 3 years ago

@rdjuric thank you! Can you try once more, but this time expand our keyboard and scroll downward slowly? If it behaves the same, no need to record another vid.

rdjuric commented 3 years ago

@michaelhaxhiu Sorry! Forgot about the expanded keyboard 😅

https://user-images.githubusercontent.com/53711423/122101860-2d8a9e80-cdeb-11eb-872f-c0458940cc47.mp4

The issue does happens on iOS. I was also able to reproduce it while scrolling up the chat, not only down, seems to happen whenever the FlatList render new items. It stops flickering when we approach the end (or beginning) of the chat, as the FlatList doesn't need to render any new items.

michaelhaxhiu commented 3 years ago

Any takers on this issue so far?

kidroca commented 3 years ago

IMO this should wait the result of #2985 I don't see an easy fix and this might result in double or unnecessary work if the chat scrolling implementation is to be rehauled Proper Bidirectional scrolling implementation should fix the current issue

michaelhaxhiu commented 3 years ago

Applying the planning label till https://github.com/Expensify/Expensify.cash/issues/2985 is released.

michaelhaxhiu commented 3 years ago

Still waiting on https://github.com/Expensify/Expensify.cash/issues/2985

michaelhaxhiu commented 3 years ago

Still waiting on https://github.com/Expensify/Expensify.cash/issues/2985

michaelhaxhiu commented 3 years ago

Still waiting on https://github.com/Expensify/Expensify.cash/issues/2985

MelvinBot commented 3 years ago

@Julesssss, @michaelhaxhiu it looks like no one is assigned to work on this job. Please double the price or add a comment stating why the job isn't being doubled.

michaelhaxhiu commented 3 years ago

Still blocked on the same GH.

michaelhaxhiu commented 3 years ago

Still on hold.

michaelhaxhiu commented 3 years ago

Rory started working on https://github.com/Expensify/App/issues/2985 and will post an update sometime early this week.

michaelhaxhiu commented 3 years ago

Awaiting an update from Rory on https://github.com/Expensify/App/issues/2985, which Jeremy requested yesterday (September 8 2021).

michaelhaxhiu commented 3 years ago

Rory is making good progress on bi-directional scrolling, and working with Kidroca - https://github.com/Expensify/App/issues/2985

Julesssss commented 3 years ago

Issue is blocked currently, not overdue

michaelhaxhiu commented 2 years ago

Heads up I'm OOO for the next 1-2 weeks, and going to keep this one assigned. If you need to payout the contributor, feel free to unassign me and re-apply the external label -- that'll auto assign a new CM

kadiealexander commented 2 years ago

Please refer to this post for updated information on the n6 hold, we've raised the bonus to $250 for all issues where a PR is created before the N6 hold is lifted.

Julesssss commented 2 years ago

Issue is still on hold due as we're awaiting the outcome of this issue: https://github.com/Expensify/App/issues/2985

Julesssss commented 2 years ago

No change

Julesssss commented 2 years ago

Issue is still on hold due as we're awaiting the outcome of this issue: #2985

Julesssss commented 2 years ago

It looks like a solution is not far away in the parent issue.

michaelhaxhiu commented 2 years ago

Still keeping this one in the ol' holster until parent GH is complete.

Julesssss commented 2 years ago

Still waiting on the above

michaelhaxhiu commented 2 years ago

Same-same

michaelhaxhiu commented 2 years ago

Luca & Rory are making gradual progress on the blocker GH. 🤞

michaelhaxhiu commented 2 years ago

Still blocked.

michaelhaxhiu commented 2 years ago

Blocked still

michaelhaxhiu commented 2 years ago

Hold

Julesssss commented 2 years ago

Still held

michaelhaxhiu commented 2 years ago

still holding on https://github.com/Expensify/App/issues/2985

michaelhaxhiu commented 2 years ago

Not overdue

michaelhaxhiu commented 2 years ago

Still on hold

michaelhaxhiu commented 2 years ago

Still holding

michaelhaxhiu commented 2 years ago

still holding

michaelhaxhiu commented 2 years ago

This is holding on birectional scrolling still.

trjExpensify commented 1 year ago

Didn't we complete the bidirectional scrolling portion of the issue, so can't this come off hold now or are we actually held on comment linking or something?