Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.54k stars 2.89k forks source link

Android - Expense - In offline, pay button greyed out in preview but not in header #48183

Closed lanitochka17 closed 2 months ago

lanitochka17 commented 2 months ago

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: 9.0.25 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team

Action Performed:

  1. Launch app
  2. Tap fab -- new workspace
  3. Navigate to LHN
  4. Open workspace chat
  5. Create a submit expense
  6. Open the expense
  7. Note in preview and detail page header pay button is not greyed out
  8. Go offline
  9. Note in preview and detail page header pay button is not greyed out
  10. Create a submit expense
  11. Note in preview pay button is greyed out and in detail page header pay button is not greyed out

Expected Result:

In offline, pay button must be greyed out in both preview and in header

Actual Result:

In offline, pay button greyed out in preview but not in header

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/15762485-305d-43fd-8f34-017e94e73fb8

View all open jobs on GitHub

melvin-bot[bot] commented 2 months ago

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

FitseTLT commented 2 months ago

Expected behaviour. In preview, the preview parent component is grayed because there is a request added optimistically so because the button is inside it, it will be gray.

nyomanjyotisa commented 2 months ago

Edited by proposal-police: This proposal was edited at 2024-08-28 16:43:06 UTC.

Proposal

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

In offline, pay button greyed out in preview but not in header

What is the root cause of that problem?

Currently, we greyed the money report header only when all the report actions on the opened report are pending

And we greyed the money report preview when one or more report actions on the report are pending

It causes the inconsistency

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

Greyed the money request header if isOffline is true and moneyRequestReport?.pendingFields?.preview exist like we use here

Update this code to the following

<View style={[styles.pt0, (!!moneyRequestReport?.pendingFields?.preview && isOffline) && styles.offlineFeedback.pending]}>

RESULT image

What alternative solutions did you explore? (Optional)

Alternative 1 Wrap the View on MoneyReportHeader with OfflineWithFeedback here

        <OfflineWithFeedback
            pendingAction={moneyRequestReport?.pendingFields?.preview}
            needsOffscreenAlphaCompositing
        >

Alternative 2 Create a function to check if any of the report actions are pending

function hasPendingReportAction(reportActions: ReportAction[]): boolean {
    return reportActions.some(action => action.pendingAction !== undefined);
}

If any of the report actions are pending and isOffline is true, greyed the money request header

const hasPendingReportAction = ReportActionsUtils.hasPendingReportAction(reportActions)
...
<View style={[styles.pt0, (hasPendingReportAction && isOffline) && styles.offlineFeedback.pending]}>

Alternative 3 on OfflineWithFeedback for header, if reportPendingAction empty use report?.pendingFields?.preview for the pending action here

                        <OfflineWithFeedback
                            pendingAction={reportPendingAction ?? report?.pendingFields?.preview}
                            errors={reportErrors}
                            shouldShowErrorMessages={false}
                            needsOffscreenAlphaCompositing
                        >
melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

@strepanier03 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

strepanier03 commented 2 months ago

I'll review again today, I see there is a comment about expected behavior above so I'll confirm that as well.

strepanier03 commented 2 months ago

I think the comment by @FitseTLT is accurate because both Android/Web and Mac/Chrome have the same behavior as the OP comment.

I'm going to close this out and it can be reopened if something changes.