[HOLD for payment 2025-01-02] Debug Mode - "Create" button flash after creating a violation

[IuliiaHerets]

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: v9.0.66-0
Reproducible in staging?: Y
Reproducible in production?: Can't check as no Debug Mode on production
Email or phone of affected tester (no customers): dave0123seife@gmail.com
Issue reported by: Applause Internal Team

Action Performed:

1. Go to https://staging.new.expensify.com/
2. Enable debug mode in Troubleshoot
3. Open an expense thread (create one if you don't have any)
4. Enter debug view then check on "View transaction" tab
5. Go to "violations" > Create a violation and save it

Expected Result:

"Create" button does not flash after creating a violation

Actual Result:

"Create" button flash after creating a violation

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6673733_1732322393345.Screen_Recording_2024-11-23_at_3.31.20_at_night.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @slafortune

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @slafortune (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]

💬 A slack conversation has been started in #expensify-open-source

[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.

[DylanDylann]

cc @pac-guerreiro

[mountiny]

This is a debug feature only so not a blocker. @DylanDylann @pac-guerreiro please follow up on this one from your PR cc @puneetlath in case you want to take over as a CME too cc @fabioh8010

[pac-guerreiro]

Hi! I’m Pedro Guerreiro from Callstack - expert contributor group. I’d like to work on this task!

[pac-guerreiro]

@IuliiaHerets I wasn't able to reproduce this behaviour on latest main, can you test it again? 😄

Screen.Recording.2024-11-25.at.20.48.16.mp4

[DylanDylann]

I still can reproduce this bug. @pac-guerreiro Could you please try again with some consecutive create?

Screen.Recording.2024-11-26.at.09.52.37.mov

[DylanDylann]

Another bug I see on the debug feature:

App/src/pages/Debug/ReportAction/DebugReportActionCreatePage.tsx

Line 117 in 3f4e93e

Debug.setDebugData(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {[parsedReportAction.reportActionID]: parsedReportAction});

we use the set method but we don't add the previous data. It caused when adding new report action, all previous report actions will be removed

[IuliiaHerets]

@pac-guerreiro Tester also can still reproduce the issue in the latest build

Screen.Recording.2024-11-26.at.3.26.36.at.night.mp4

[pac-guerreiro]

Another bug I see on the debug feature:

App/src/pages/Debug/ReportAction/DebugReportActionCreatePage.tsx

Line 117 in 3f4e93e

Debug.setDebugData(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {[parsedReportAction.reportActionID]: parsedReportAction});

we use the set method but we don't add the previous data. It caused when adding new report action, all previous report actions will be removed

@DylanDylann nice catch! I think it should be addressed on a separate issue, can you create it?

[pac-guerreiro]

@IuliiaHerets @DylanDylann

I'll try it again, but it's not an easy one 😅 Could you also try creating a report action to confirm if it also happens?

[pac-guerreiro]

Today I didn't have the time to test this again. Tomorrow I'll take another look at it!

[IuliiaHerets]

@pac-guerreiro Issue is reproducible, build v9.0.67-1

Screen.Recording.2024-11-27.at.1.20.29.at.night.mp4

[melvin-bot]

@slafortune, @blimpich, @pac-guerreiro, @DylanDylann Eep! 4 days overdue now. Issues have feelings too...

[JKobrynski]

I tried reproducing this issue today and I wasn't able to get it to flash as clearly as it does on the demo, but I think that it flashed for a split second for me 2 or 3 times. I will be taking a look into the code to try to fix that but it might be difficult due to problems with reproduction.

[JKobrynski]

Today I tested a solution which gets rid of the ScrollView that wrapped the FlatList with scrollEnabled={false}, so there is only one scrollable component, I also wrapped renderItem method with useCallback, but none of these solutions fixed that flashing effect.

My plan for the further investigation is to check how this component is rendered in DebugTransactionPage and look for the source of this bug there.

[JKobrynski]

Proposal

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

"Create" button is flashing after creating a new transaction violation in debug mode.

What is the root cause of that problem?

According to my investigation, this is caused by FlatList. Even replacing it with FlashList doesn't help. To me, it looked like FlashList made it occur less often, but I might be wrong, and it doesn't really matter as the bug still reproduces.

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

As the contains a ScrollView that wraps a FlatList with scrollEnabled={false} we can get rid of the FlatList component and replace it with a standard data.map((item) => renderItem(item)) solution. This is not one of the super long and data heavy lists, so removing the non-scrollable FlatList should not impact the performance. I tested this solution and I couldn't reproduce the flashing effect anymore. It was also still as smooth as before.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

N/A

What alternative solutions did you explore? (Optional)

1. FlashList

As mentioned above, I tried replacing FlatList with FlashList but it didn't fix the issue. It seemed to me like the issue happens less often, but it didn't fix it.

2. Getting rid of ScrollView

This component is quite a small one, and it has a FlatList with scrollEnabled={false} nested right inside a ScrollView. I tried replacing the ScrollView with a View component and enabling scroll on the FlatList. The component looked the same after that change, however the bug still remained unfixed.

3. useCallback

I thought this flashing effect might be caused by some unnecessary rerender caused by some prop/param update, so I wrapped the renderItem method with useCallback. It also didn't help, the flash was still happening.

[blimpich]

Weird bug. I'm good with the above solution if we make sure to add a comment explaining why we made the changes we did. Seems like a bug that wouldn't really be able to be prevented with a unit test and also would be easy to forget about and have a regression come up later.

[JKobrynski]

The PR is ready for review!

[pac-guerreiro]

@DylanDylann @IuliiaHerets I'm back from my vacation, so I'll take it from here 😄

[pac-guerreiro]

I'm able to reproduce this issue on track/submit expense flow as well:

Screen.Recording.2024-12-12.at.14.47.05.mp4

I suspect this issue is caused by OnyxTabNavigator. I just asked internally if someone has experienced this before and can help with it 😄

[pac-guerreiro]

@puneetlath @IuliiaHerets

Last friday (12/13/24)

• I did some trial and error to understand what could be causing OnyxTabNavigator to re-render the page contents, but I didn't manage to find the cause.

Today:

• I created my own tab navigator, DebugTabNavigator, which mimics OnyxTabNavigator behaviour and layout but uses a StackNavigator instead of a MaterialTopTabNavigator and it fixes the problem.

[pac-guerreiro]

@puneetlath @IuliiaHerets

Yesterday I addressed the eslint errors on the PR but it seems that they still persist 😅

I'll address them in a moment and let you know!

[pac-guerreiro]

@puneetlath @IuliiaHerets

All remaining issues addressed! Now waiting on @DylanDylann to do his magic 🪄

[pac-guerreiro]

@puneetlath @IuliiaHerets

Today I resolved all conflicts in the PR 😄

[pac-guerreiro]

@puneetlath @IuliiaHerets

Today I fixed the app crash reported by @DylanDylann!

I'll be away until January 2nd but someone should take care of my work 😄 Happy holidays! 🎉

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

• replace FlatList with .map solution #53655

If no regressions arise, payment will be issued on 2025-01-02. 🎊

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

• @JKobrynski does not require payment (Contractor)
• @pac-guerreiro does not require payment (Contractor)
• @DylanDylann requires payment (Needs manual offer from BZ)

[melvin-bot]

@DylanDylann @slafortune @DylanDylann 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]