[$500] Chat - The chat shakes when you swipe down

[lanitochka17]

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

---

Version Number: 1.4.1.4
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Navigate to https://staging.new.expensify.com/
2. Log in
3. Open a conversation with a lot of one word messages and some emojis in a chat
4. Swipe up and down

Expected Result:

The swiping should be smooth

Actual Result:

The chat shakes when you swipe down

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6284556_1700515578151.Shake.1.mp4

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a404d0c8e8de3687
• Upwork Job ID: 1726717224301903872
• Last Price Increase: 2023-12-11

[melvin-bot]

Triggered auto assignment to @sophiepintoraetz (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01a404d0c8e8de3687

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @sobitneupane (External)

[shahinyan11]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Chat - The chat shakes when you swipe down

What is the root cause of that problem?

While scrolling, every time the scroll indicator gets close to the bottom, the Report.getNewerActions is called which switches loader by changing the prop isLoadingNewerReportActions. And in ReportActionsList component comes a change in container style associated with changing of the isLoadingNewerReportActions prop in this part of code. ( It adds or remove paddingTop: CONST.CHAT_HEADER_LOADER_HEIGHT to the container so that the content moves up and the loader becomes visible ) . It Is the main cause of shaking of the content

What changes do you think we should make in order to solve the problem?

We can change this part of code like this - isLoadingNewerReportActions && scrollingVerticalOffset.current <= CONST.CHAT_HEADER_LOADER_HEIGHT ? styles.chatContentScrollViewWithHeaderLoader : {} .

Adding scrollingVerticalOffset.current <= CONST.CHAT_HEADER_LOADER_HEIGHT in check helps us avoid adding a loader if the scroll position is such that the loader won't be visible anyway.

What alternative solutions did you explore? (Optional)

Alternatively we can

1. I suggest removing this part of the code and setting the paddingTop: CONST.CHAT_HEADER_LOADER_HEIGHT always
2. fix _.throttle function in ReportActionsView component and increase throttle time.
   What is wrong and need to be fixed :
   Now the code is written incorrectly and the loadNewerChats function is called without throttling. It is wrapped in useMemo which has deeps which changes after every call of Report.getNewerActions and the loadNewerChats function re-defines again ( Which makes using the _.throttle this way pointless ).

[brunovjk]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The problem involves noticeable shaking in the chat view when engaging in specific actions, like opening multiline text in edit mode and scrolling up.

What is the root cause of that problem?

The issue arises because loadNewerChats is invoked prematurely, before the user scrolls to the very end, leading to unnecessary adjustments in the container style within the ReportActionsList component, rendering the loading circle (paddingTop: CONST.CHAT_HEADER_LOADER_HEIGHT) unnecessarily.

App/src/pages/home/report/ReportActionsList.js

Line 466 in d40c017

onStartReachedThreshold={0.75}

If the user doesn't reach the end of the scroll, the loading circle is not visible anyway, making the adjustments redundant.

What changes do you think we should make in order to solve the problem?

Set onStartReachedThreshold to zero, so that loadNewerChats will only be called when the user gets to the end of the scroll (chat).

    onStartReached={loadNewerChats}
    onStartReachedThreshold={0}

What alternative solutions did you explore? (Optional)

[melvin-bot]

@sobitneupane, @sophiepintoraetz Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[sophiepintoraetz]

@sobitneupane - can you take a look at the proposals, please?

[sobitneupane]

Thanks for the proposal @shahinyan11

I suggest removing this part of the code and setting the paddingTop: CONST.CHAT_HEADER_LOADER_HEIGHT always

I don't think we can do that. If we always set paddingTop: CONST.CHAT_HEADER_LOADER_HEIGHT, won't we have unwanted padding when Activity Indicator is hidden?

I don't think even increasing throttle solves the issue completely.

[sobitneupane]

Thanks for the proposal @brunovjk

@brunovjk Can you please add more details on 3rd method in your proposal.

Regarding "Create a designated space for the loading circle, ensuring it appears and disappears without affecting existing content.", It might create unwanted padding when Activity Indicator is hidden resulting bad user experience as discussed here.

[shahinyan11]

I don't think we can do that. If we always set paddingTop: CONST.CHAT_HEADER_LOADER_HEIGHT, won't we have unwanted padding when Activity Indicator is hidden?

What I mean is that the display loader should not change the height of the content. I thought that the padding was small and would not look ugly.
I can an offer to move loader to top or something else. But if you don't like it. I think that firstly a design solution is needed and after that development. Because there is no way to do it without changing the design.

I don't think even increasing throttle solves the issue completely.

This will not solve the problem completely. this just helps so that it doesn’t happen again several times within a second

[shahinyan11]

Proposal

Updated

I found a new solution that helps avoid changing the UI. I changed old two solutions as alternative and added one new solution as the main

[sophiepintoraetz]

Waiting for feedback from @sobitneupane

[brunovjk]

Sorry for the delay @sobitneupane, I understand, I had the same concerns about solution 2, great that you confirmed it.

I'm going to do some tests now on solution 3 and get back to you with details as soon as possible.

Thanks.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@sobitneupane @sophiepintoraetz this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[shahinyan11]

@sobitneupane Could you please take a look at my updated proposal and give some feedback. It's already been 5 days since my update?

[sobitneupane]

Thanks for the update @shahinyan11. I was waiting for @brunovjk's update. I will review your proposal asap. Thank you for the patience.

[brunovjk]

@brunovjk Can you please add more details on 3rd method in your proposal.

@sobitneupane after testing the "Scroll Height Increase" option, it was observed that the issue persisted, and the chat shaking was merely transferred to the scrollbar, making the user experience less favorable. Upon further investigation, the problem was traced to the onStartReachedThreshold value in the ReportActionsList component.

[brunovjk]

Proposal

Updated

[sobitneupane]

Will review the proposal by EOD.

[brunovjk]

I tried to reproduce the issue now, and it seems that not even the circle loading is appearing.

[shahinyan11]

Will review the proposal by EOD.

@sobitneupane when to expect updates from you

[sobitneupane]

@shahinyan11 I was reviewing the proposals. But looks like the issue is no longer reproducible.

[shahinyan11]

@sobitneupane and what steps did you take ? Or should we do something ?

[shahinyan11]

@sobitneupane The behavior changed due to adding following code || hasNewestReportAction in condition in this condition .

[sobitneupane]

The change was introduced by this PR. And the issue is no longer reproducible. I don't think any action is required in the issue.

[shahinyan11]

The issue is no longer reproducible but the question arises. When this loader may appear ? If there still exists some cases when the loader may appear, the issue still actual and if not, then why not remove the loader component from there ?

[sobitneupane]

@shahinyan11 I think we should close the issue for now. We can always reopen the issue if the problem resurfaces.

cc: @sophiepintoraetz

[melvin-bot]

@sobitneupane @sophiepintoraetz this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[melvin-bot]

@sobitneupane, @sophiepintoraetz Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[sophiepintoraetz]

Ah, I missed this comment from being OOO, agreed.