[$250] IOU-Submit expense on foreign currency, total briefly greyed out on top

IuliiaHerets commented 1 week 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.28-0 Reproducible in staging?: Y Reproducible in production?: Y Issue reported by: Applause Internal Team

Action Performed:

Launch app
Open 1:1 chat with no prior conversation
Create 2 submit expense in local currency
From report page, create one more expense in local currency
Note total is not greyed out on top
Create a expense in foreign currency
Note total is briefly greyed out on top

Expected Result:

Submit expense on foreign currency, total briefly must not be greyed out on top.

Actual Result:

Submit expense on foreign currency, total briefly greyed out on top.

Workaround:

Unknown

Platforms:

[x] Android: Native
[x] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/35e10933-c7b0-4d0d-b7a5-880552b4097f

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021831692684862783385
Upwork Job ID: 1831692684862783385
Last Price Increase: 2024-09-12

Issue Owner

Current Issue Owner: @allgandalf

melvin-bot[bot] commented 1 week ago

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

IuliiaHerets commented 1 week ago

@muttmuure 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

nyomanjyotisa commented 1 week ago

Proposal

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

IOU-Submit expense on foreign currency, total briefly greyed out on top

What is the root cause of that problem?

We greyed the total value while waiting total update here

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

If we don't want to greyed it out we change that code to the following

<Text
    numberOfLines={1}
    style={[styles.taskTitleMenuItem, styles.alignSelfCenter]}
>
    {formattedTotalAmount}
</Text>

What alternative solutions did you explore? (Optional)

IMO we should display a loading state on the total value while waiting the total updated if isOnline, and greyed it out only if isOffline

Tony-MK commented 1 week ago

Proposal

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

Submit expense on foreign currency, total briefly greyed out on top

What is the root cause of that problem?

When the isTotalUpdated is false because one of the transactions has a different currency from the workspace, the styles.offlineFeedback.pending is applied briefly before the total is updated.

https://github.com/Expensify/App/blob/2d1f8cd1cb072f0b9c9672effa65199727553ab9/src/components/ReportActionItem/MoneyReportView.tsx#L166

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

We should check if the user is offline together with isTotalUpdated so it is applied only when the user is offline.

Let's change the style of the Text component for the total with the isOffline condition, as shown below.

\\ Get `isOffline`
const {isOffline} = useNetwork();

...

\\ Use it in the style of the `Text` component
style={[styles.taskTitleMenuItem, styles.alignSelfCenter, isOffline && !isTotalUpdated && styles.offlineFeedback.pending]}

Krishna2323 commented 1 week ago

Edited by proposal-police: This proposal was edited at 2023-10-04T15:33:00Z.

Proposal 

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

IOU-Submit expense on foreign currency, total briefly greyed out on top

What is the root cause of that problem?

styles.offlineFeedback.pending style is applied when isTotalUpdated is false. https://github.com/Expensify/App/blob/3e5da7810fdc48ce391db3ed3aa7dab86a59c490/src/components/ReportActionItem/MoneyReportView.tsx#L166
What changes do you think we should make in order to solve the problem? 
To be consistent with other pages like MoneyRequestPreviewContent, we should show Pending... when isTotalUpdated is false.
To do that, we should first remove the offline styling from the amount text here: https://github.com/Expensify/App/blob/3e5da7810fdc48ce391db3ed3aa7dab86a59c490/src/components/ReportActionItem/MoneyReportView.tsx#L166
Wrap the amount row (with title text) with OfflineWithFeedback and pass !isTotalUpdated ? "update" : undefined as the value for pendingAction prop.
We should update the formattedTotalAmount constant to: https://github.com/Expensify/App/blob/3e5da7810fdc48ce391db3ed3aa7dab86a59c490/src/components/ReportActionItem/MoneyReportView.tsx#L56 TO:
```
const formattedTotalAmount = !isTotalUpdated ? translate('iou.fieldPending') : CurrencyUtils.convertToDisplayString(totalDisplaySpend, report.currency);
```

[!NOTE]
We might also need minor style changes.

What alternative solutions did you explore? (Optional)

In offline mode, instead of Pending... we can use Total will be updated when you'll be back online.

Result

https://github.com/user-attachments/assets/1bc9b6b8-698b-4248-9e3f-81db9eae0bb8

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 5 days ago

@muttmuure, @allgandalf Eep! 4 days overdue now. Issues have feelings too...

allgandalf commented 5 days ago

I will review the proposals tomorrow

melvin-bot[bot] commented 3 days ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

allgandalf commented 19 hours ago

@Expensify/design , need your inputs here:

When we have two IOU's we show the total at the top right of the report:

When we submit 3rd expense with different currency, for a brief time the total is grayed out:

https://github.com/user-attachments/assets/af9b3bc7-cacc-43a2-870a-cad04207fa01

What should be done in this case? should we show a loading state here or some message that total is loading ?

dubielzyk-expensify commented 4 hours ago

Ah interesting. Yeah I'd prefer for us to do something more loading-y. Either ... or --. I forget where we do something similar, but perhaps other @Expensify/design remembers or have better suggestions

Expensify / App