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

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

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

We think that this bug might be related to #wave-collect - Release 2

Edited by proposal-police: This proposal was edited at 2024-11-01 22:11:15 UTC.

Proposal

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

Verifying page does not show offline indication when the user is offline.

What is the root cause of that problem?

Currently, to display the offline indicator on wider screens, we need to use the shouldShowOfflineIndicatorInWideScreen prop, which is missing in this instance.

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

we can pass shouldShowOfflineIndicatorInWideScreen here if we want to show the offline indicator in wider screens

Optionally we can add offlineIndicatorStyle={styles.mtAuto} so that in android, the indicator stays below the content

What alternative solutions did you explore? (Optional)

If we decide to show the FullPageOfflineBlockingView, as suggested in another proposal to wrap this code block in FullPageOfflineBlockingView , I believe this is not correct. Once the bank is verified, we display the verified state/waitlist state, so there"s no need to block that view. There are no pending requests now we are just waiting for the user to take action.

Let"s only wrap the code block below in FullPageOfflineBlockingView, as a write request is being made when it"s pressed, and the response cannot be anticipated.

Optional: Although there is no need to block the view while it is in a loading state, if we decide to do so, let"s just wrap this component in FullPageOfflineBlockingView.

Alternative 2
If we want to allow users to use the plus icon on the select bank account page, we can wrap only the loading state in the FullPageOfflineBlockingView.

Sounds like it could come from #51407.
The proposal makes sense to me, going to label External to get a second pair of eyes on it.

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

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

reviewing...

Proposal

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

Verifying page does not show offline indication when user is offline

What is the root cause of that problem?

We did not wrap the WorkspaceExpensifyCardBankAccounts with FullPageOfflineBlockingView

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

We need to wrap the content inside the page with FullPageOfflineBlockingView, this will return a blocking view when the user if offline:

We do the same for connect bank account page:

<FullPageOfflineBlockingView>
                {isInVerificationState && renderVerificationStateView()}
                {!isInVerificationState && (
                    <View style={styles.flex1}>
                        <Text style={[styles.mh5, styles.mb3]}>{translate("workspace.expensifyCard.chooseExistingBank")}</Text>
                        {renderBankOptions()}
                        <MenuItem
                            icon={Expensicons.Plus}
                            title={translate("workspace.expensifyCard.addNewBankAccount")}
                            onPress={handleAddBankAccount}
                        />
                    </View>
                )}
                </FullPageOfflineBlockingView>

What alternative solutions did you explore? (Optional)

@ahmedGaber93 can you also please review my proposal ^, i think my proposal causes consistency across the application

@Nodebrute Thanks for the proposal

Your solution not work with me, unless I remove !shouldUseNarrowLayout && from the below condition.

I think your RCA is not complete.

@ahmedGaber93 I believe we don"t display offline indicators in the Right Hand Panel (RHP). Since this is a part of the RHP, I think we"re not supposed to show an offline indicator here.

@ahmedGaber93 Could you please confirm the expected behavior here? Also, which offline pattern is this related to?

@francoisl What do you think about the expected behavior here

1. display OfflineIndicator at the bottom of screen
2. display FullPageOfflineBlockingView

@ahmedGaber93 @francoisl for wide screen we never show the offline indicator on the RHP, also you can see that the central Pane already shows this indicator, in our case we need to block the view using FullPageOfflineBlockingView

for wide screen we never show the offline indicator on the RHP

Hmm! I am not sure about that, Let"s get confirmation from @Expensify/design for #51885 (comment)

Proposal Updated
#51885 (comment)

@francoisl What do you think about the expected behavior here

1. display OfflineIndicator at the bottom of screen
2. display FullPageOfflineBlockingView

I think 2 is appropriate in this instance. But let"s see if the other @Expensify/design"ers agree

I think that sounds right to me, but will let others chime in as well.

Agree here. @twilight2294 happy to pay for your solution, but as you aren"t doing the work, it will be $125 for you, not the full $250.

@twisterdotcom, @koko57 decided to go with my solution #51885 (comment)

Wow, sorry. I cannot keep up. Okay, in which case, that would be for you.

@twisterdotcom , I was the first to suggest the use or FullPageOfflineBlockingView, @Nodebrute first suggested to use shouldShowOfflineIndicatorInWideScreen and then edited their solution after i added mine:

I suggested the use of FullPageOfflineBlockingView, and @Nodebrute wrote their alternative solution based on it, @ahmedGaber93 can you also confirm this please :)_

@twilight2294 your solution wasn’t fully correct. Just suggesting FullPageOfflineBlockingView isn’t a complete fix on its own. I"ve thoroughly researched when it"s appropriate to block the user and when it"s not. Also, I noticed you posted your solution here while you still had pending actions on this PR. This goes against guidelines, as contributors shouldn"t submit proposals on issues when they have assigned issues or PRs awaiting their action https://expensify.slack.com/archives/C01GTK53T8Q/p1717003214375549, Thank you!

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

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

• fix: wrap the page in FullPageOfflineBlockingView #51947

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

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

• @koko57 does not require payment (Contractor)
• @ahmedGaber93 requires payment automatic offer (Reviewer)

@ahmedGaber93 @twisterdotcom 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]

Payment Summary:

• @koko57 does not require payment (Contractor)
• @ahmedGaber93 paid $250 here (Reviewer)
• @Nodebrute paid $125 here (Contributor)

Wow, sorry. I cannot keep up. Okay, in which case, that would be for you.

@twisterdotcom friendly bump ^ #51885 (comment)

Accepted.

@francoisl @twisterdotcom @koko57 @ahmedGaber93 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!

BugZero Checklist:

• [Contributor] Classify the bug:

• [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: https://github.com/Expensify/App/pull/51407/files#r1843618525

• [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: N/A

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

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

Precondition:

Test:

1. Create new Workspace
2. Go to Settings > Workspace > More features
3. Enable Expensify Card
4. Go offline
5. Go to Expensify Card page, click Issue New card
6. Verify the Choose Bank Account page should display the full page oflline blocking view.
7. Try the same with workspaces that already have the Bank account verified or in verifying state - you should see the modal with this state instead

Do we agree 👍 or 👎