[HOLD for payment 2024-11-29] [$250] mWeb - Inbox- When creating WS via FAB and using device back button, rest of chats disappear

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.55-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5141100&group_by=cases:section_id&group_order=asc&group_id=229067
Issue reported by: Applause - Internal Team

Action Performed:

1. Open the staging.new.expensify.com website
2. Sign in with new account
3. Complete the onboarding process
4. Tap on the FAB and select "New Workspace"
5. Once redirected to workspace details page, use the device back button to return to inbox
6. Verify that all the chats, including the new workspace chat, are displayed in inbox

Expected Result:

When the user creates a new workspace via FAB and returns to inbox using device back button, all the chats should be visible

Actual Result:

When creating a new workspace via FAB, and returning to inbox using device back button, the rest of the chats disappear and only the workspace chat is visible until navigating to a different category and returning

Workaround:

Unknown

Platforms:

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

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6649332_1730206763889.WS_FAB.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021856428632364438612
• Upwork Job ID: 1856428632364438612
• Last Price Increase: 2024-11-12
• Automatic offers:
• brunovjk | Reviewer | 104913598

Issue Owner

Current Issue Owner: @abekkala

[melvin-bot]

Triggered auto assignment to @abekkala (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[lanitochka17]

@abekkala FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[bernhardoj]

Proposal

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

After create a new workspace from FAB and go back, the LHN only shows 1 chat.

What is the root cause of that problem?

The LHN actually has all the chats, but they are all overlapping each other. Normally, each item has a translateY value.

But when the issue happens, all the translate are 0. It's handled by flashlist.
https://github.com/Shopify/flash-list/blob/f6723c4ad7a8ba84f0c9d4b2b636f4073c1f9410/src/FlashList.tsx#L511
https://github.com/Shopify/flash-list/blob/f6723c4ad7a8ba84f0c9d4b2b636f4073c1f9410/src/native/config/PlatformHelper.web.ts#L11-L17

The values (x & y) are coming from parentProps. FlashList uses recyclerlistview which the parentProps value comes from. The x & y value is from the recyclerlistview layout manager which holds each item rect.
https://github.com/Flipkart/recyclerlistview/blob/678c3cde10499787387e7cd79ee9ef358d04426f/src/core/RecyclerListView.tsx#L688-L700

When an item is updated, it will update the layout manager with the new item size and then trigger a re-render of the list, only if it's still mounted (tracked by _isMounted variable).
https://github.com/Flipkart/recyclerlistview/blob/678c3cde10499787387e7cd79ee9ef358d04426f/src/core/RecyclerListView.tsx#L735-L741
https://github.com/Flipkart/recyclerlistview/blob/678c3cde10499787387e7cd79ee9ef358d04426f/src/core/RecyclerListView.tsx#L559-L564

In our case, when we create the new workspace from FAB, there are a few things happened:

1. We are navigated to the workspace detail page. This also changes the bottom tab navigator screen from Home (LHN) to Settings.
2. A new optimistic expense chat is added, triggering the items update
3. However, after the bottom tab screen is changed to Settings, the size of each item is always 0. That's because any bottom tab screen that is not focused has a display: none applied to it, just like stack navigator
4. 0 is saved as the item size to the layout manager
5. The FlashList list is somehow unmounted which causes RecyclerListView to be unmounted too, setting _isMounted to false

When going back, the list will render each item with the saved size first, 0, so all x and y are 0. Then, each item will trigger a size update to the real size, but when trying to re-render the list, it fails because _isMounted is still false (from step 5) because the constructor isn't triggered again.
https://github.com/Flipkart/recyclerlistview/blob/678c3cde10499787387e7cd79ee9ef358d04426f/src/core/RecyclerListView.tsx#L559-L564

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

Set _isMounted to true in componentDidMount
https://github.com/Flipkart/recyclerlistview/blob/678c3cde10499787387e7cd79ee9ef358d04426f/src/core/RecyclerListView.tsx#L241-L243

What alternative solutions did you explore? (Optional)

I think the reason we check for mounted status is because we debounce the _queueStateRefresh. Instead of using _isMounted to make sure the debounced function isn't triggered when the component is already unmounted, we can cancel the debounce when unmount.

componentWillUnmount() {
    this.refreshRequestDebouncer.cancel();
}

[melvin-bot]

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

[melvin-bot]

@abekkala Still overdue 6 days?! Let's take care of this!

[abekkala]

@lanitochka17 is this due to focus mode?

[lanitochka17]

@abekkala Preferences were set as "Most Recent" at that moment

[melvin-bot]

@abekkala this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

@abekkala Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

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

[melvin-bot]

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

[brunovjk]

Hey @bernhardoj, Thanks for the proposal, it makes sense to me, I can reproduce the issue across all browser-related platforms (web, desktop, mWeb Android, mWeb iOS). However, I wasn’t able to test your solution—I modified the RecyclerListView.tsx file in the node_modules folder where the recyclerlistview library is stored, but the issue persists. Do you think we need to try something else to test it? Maybe share a test branch? Thanks!

[bernhardoj]

You need to change the node_modules/recyclerlistview/dist/reactnative/core/RecyclerListView.js.

[brunovjk]

Thanks for clarifying, @bernhardoj. Your proposal makes sense to me and tested well in a quick test:

before_change

main.mov

after_change

proposal.mov

However, I’m not 100% sure about potential regressions or side effects. We’ll need to do more thorough testing in the next step. Have you thought about the best way to proceed? How should we implement this change so others can test it properly? Does that make sense? Thanks again!

[bernhardoj]

How should we implement this change so others can test it properly?

Sorry, I don't quite get this one. I don't think there is any difference with other cases of adding patches and upstream PR.

[brunovjk]

Ok, thanks for clarifying :D With that, I think we can proceed with @bernhardoj’s proposal.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @brunovjk 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[grgia]

Thanks for your proposal @bernhardoj !

[bernhardoj]

PR is ready. Added the upstream PR link on the PR.

cc: @brunovjk

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.65-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix lhn list only shows 1 item when going back after create new ws from FAB #52675

If no regressions arise, payment will be issued on 2024-11-29. 🎊

For reference, here are some details about the assignees on this issue:

• @bernhardoj requires payment through NewDot Manual Requests
• @brunovjk requires payment automatic offer (Reviewer)

[melvin-bot]

@brunovjk @abekkala @brunovjk The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[abekkala]

PAYMENT SUMMARY NOV 29

• Fix:@bernhardoj [$250] Payment via NewDot Manual Requests
• PR Review: @brunovjk [$250]offer

[bernhardoj]

Requested in ND.

[JmillsExpensify]

$250 approved for @bernhardoj

[brunovjk]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other: The issue is a side effect of how the third-party library (recyclerlistview) handles component mounting states. Upstream PR: Fix _queueStateRefresh not working after previously being frozen by react-freeze Flipkart/recyclerlistview#787

Where bug was reported:

• 2a. Reported on production
• 2b. Reported on staging (deploy blocker)
• 2c. Reported on both staging and production
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other: Applause - Internal Team

• [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

  Link to comment: This bug stems from a third-party dependency (recyclerlistview) and not from a PR authored by contributors to this repository

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

  Link to discussion:

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

Regression Test Proposal

1. Log in with a new user account.
2. Create a new workspace via the FAB button.
3. Navigate back to the inbox using the back button (browser/device-specific).
4. Verify all chats, including the new workspace chat, are displayed in the inbox.
5. Navigate between tabs and confirm the chat list updates without errors.
6. Trigger additional list updates (e.g., new chat creation) and validate rendering.

Do we agree 👍 or 👎

[abekkala]

@brunovjk payment sent and contract ended - thank you! 🎉