[HOLD for payment 2024-06-20] [$250] Web - IOU - Currency listing page displays empty when clicked on currency

[kbecciv]

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: 1.4.63-0
Reproducible in staging?: y
Reproducible in production?: n
Issue found when executing PR: #38892
Issue reported by: Applause - Internal Team

Action Performed:

1. Tap on a chat --- tap the FAB > track expense
2. Enter an amount > change currency to RP> tap next
3. Tap track expense (add a merchant when required)
4. Tap on newly created expense to open expense details
5. Tap amount to edit the currency
6. Notice the currency page takes a minute to load

Expected Result:

Currency listing page should be displayed with available currencies

Actual Result:

The currency listing page is displayed empty. When user taps on screen the currency list appears

Workaround:

n/a

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

RPReplay_Final1714710485.MP4

Bug6453814_1713455722598.Screen_Recording_2024-04-18_at_18.53.39.1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015e149d6e5bb867bf
• Upwork Job ID: 1782471211809525760
• Last Price Increase: 2024-05-22
• Automatic offers:
• nkdengineer | Contributor | 102517477

Issue Owner

Current Issue Owner: @Christinadobrzyn

[melvin-bot]

Triggered auto assignment to @deetergp (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[kbecciv]

Issue is reproduced Only on MacOS/ Safari

[deetergp]

Wow, that really is specific to Safari! It works just fine in Chrome. I wonder if this TS migration isn't the culprit #40100

[deetergp]

This seems pretty edge-case-y. I'm going to demote it as not-a-blocker.

[bernhardoj]

I have locally reverted #40100 but still happens.

[deetergp]

Going to mark this as an external bug.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @Christinadobrzyn (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.

[melvin-bot]

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

[nkdengineer]

Proposal

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

Currency listing page displayed empty. When user scroll up or down the listing page, the available currencies will appear.

What is the root cause of that problem?

Here, we have the logic to scrollToIndex if the focused index changes in useArrowKeyFocusManager.

In useArrowKeyFocusManager here, we call onFocusedIndexChange in an useEffect, so it will be called in first render, even if the focusedIndex did not change.

This first render of useArrowKeyFocusManager (and scrolling) happens even before the SectionList here finishes rendering (and onLayout called), so the scrolling will interfere with the SectionList rendering, causing the blank page to show.

This is a known issue and avoided that by only scrolling to the focused index after the onLayout was already called here. But due to the first rendering in useArrowKeyFocusManager, scrolling is still called before onLayout.

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

In useArrowKeyFocusManager here, we need to call onFocusedIndexChange when the focused index actually changes (not on first render). We can use usePrevious or firstRenderRef (like here) for this.

const prevIsFocusedIndex = usePrevious(focusedIndex);

useEffect(() => {
    if (prevIsFocusedIndex === focusedIndex) {
        return;
    }
    onFocusedIndexChange(focusedIndex)
}, [focusedIndex, prevIsFocusedIndex]);

What alternative solutions did you explore? (Optional)

In scrollToIndex, early return if onLayout of SectionList wasn't called yet. To do this, we need to store isInitialSectionListRender in a ref like isInitialSectionListRenderRef too and use it, also we need to make sure to set isInitialSectionListRenderRef to false before this line to make sure the initial scroll still works fine.

An alternative is to still reuse isInitialSectionListRender state, but we should use animated: false (or add a new param) to distinguish the first scroll. So we always allow the first scroll, if it's not the first scroll, isInitialSectionListRender is true, then we early return and don't scroll.

[rushatgabhane]

cc: @deetergp

[melvin-bot]

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

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[nkdengineer]

@rushatgabhane this PR is ready for preview

[Christinadobrzyn]

monitoring #42816

[deetergp]

PR was merged today 🎉

[Christinadobrzyn]

Just a heads up - I'm going to be ooo until June 24th. I'm not going to assign anyone new but if you need a new BZ teammate for any reason please feel free to ask for one to be assigned here.

[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 1.4.82-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: IOU - Currency listing page displays empty when clicked on currency #42816

If no regressions arise, payment will be issued on 2024-06-20. 🎊

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

• @rushatgabhane requires payment through NewDot Manual Requests
• @nkdengineer requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@rushatgabhane] The PR that introduced the bug has been identified. Link to the PR:
• [@rushatgabhane] 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:
• [@rushatgabhane] A discussion in #expensify-bugs 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:
• [@rushatgabhane] Determine if we should create a regression test for this bug.
• [@rushatgabhane] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@Christinadobrzyn] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[rushatgabhane]

1. The PR that introduced the bug has been identified. Link to the PR: Pay Header improvements #18883

2. 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: https://github.com/Expensify/App/pull/18883/files#r1641867143

3. A discussion in #expensify-bugs 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: N.A.

4. Determine if we should create a regression test for this bug. N.A. This is caught by normal QA steps

5. If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again - N.A.

[rushatgabhane]

@Christinadobrzyn please attach payment summary whenever you're back

[Christinadobrzyn]

Payouts due:

• Contributor: $250 @nkdengineer (paid in Upwork - https://www.upwork.com/nx/wm/workroom/37416123/details)
• Contributor+: $250 @rushatgabhane (paid through NewDot)

Upwork job is here.

Closing this but @rushatgabhane please make sure to submit for payment through NewDot!

[JmillsExpensify]

$250 approved for @rushatgabhane