[LOW] [Splits] [$1000] Mweb-IOU-Previously selected currency is shown for an second in send money confirmation page.

[kavimuru]

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.31-1
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause internal team
Slack conversation:

Action Performed:

1. Go to https://staging.new.expensify.com/
2. Tab fab---- send money
3. Enter an amount
4. Tap next
5. Select a contact
6. Tap on amount section
7. Change the currency
8. Tap save
9. Perform step 7 and step 8 repeatedly and note currency display in send money confirmation page

This issue also happens in the request flow:

1. Open the app
2. Go to FAB > Request money
3. Change currency to UAH
4. Close the app and re-open
5. Go to FAB > Request money
6. BUG: UAH is briefly shown before showing user's local currency

Expected Result:

Previously selected currency must not be shown for an second in send money confirmation page.

Actual Result:

Previously selected currency is shown for an second in send money confirmation page.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6354245_1706160961488.previous.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a75f53fa7369a183
• Upwork Job ID: 1750501967791529984
• Last Price Increase: 2024-03-07

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @MitchExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

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

[JordanLevy19]

Proposal

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

When clicking on the currency icon to change the currency, and after selecting the new currency, there's a flickering or delay with the currency symbol switching between the old currency and the newly selected currency within a 2-second interval. This seems to suggest that there is an issue with the state management of the currency selection.

What is the root cause of that problem?

The issue may stem from a combination of asynchronous state updates and the React rendering cycle, which can momentarily display outdated content

Delayed Currency Update:

In NewRequestAmountPage.js, the currency is determined and then passed to MoneyRequestAmountForm. If there is any delay or asynchronous operation in updating this prop, the child component could render with outdated currency information initially and then update once the new currency prop is received.

const currency = CurrencyUtils.isValidCurrencyCode(currentCurrency) ? currentCurrency : iou.currency;

State Management in Form:

The MoneyRequestAmountForm component has a complex state management mechanism that includes multiple useState and useEffect hooks, which could lead to a lag when updating the currency symbol.

useEffect(() => {
    if (!currency || !_.isNumber(amount)) {
        return;
    }
    initializeAmount(amount);
}, [selectedTab, amount]);

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

Optimize State Updates in MoneyRequestAmountForm:

Ensure that the state updates dependent on the currency prop are synchronized. Use the useReducer hook to manage the state more predictably, which could help prevent the flickering caused by multiple useState and useEffect updates.

Stabilize Currency Symbol Updates:

Modify the TextInputWithCurrencySymbol component to handle currency changes more gracefully. This could involve implementing a loading state that shows a spinner or a placeholder until the currency symbol is confirmed to be updated to prevent showing the wrong symbol even momentarily.

Review Navigation Flow:

In NewRequestAmountPage, ensure that navigation and state updates related to currency selection are completed before rendering the MoneyRequestAmountForm.

[MitchExpensify]

I don't think this fits into any wave or VIP right now

[situchan]

@MitchExpensify I think this should be fixed as bad user experience. This also happens at the start of money request flow.
Please check this video: (clearly noticeable on android)

Screen.Recording.2024-01-28.at.7.06.10.PM.mov

[MitchExpensify]

After sleeping on this I think I agree and this seems wave 5 worthy cc @dylanexpensify

[situchan]

Thanks. We can simplify repro step like this:

1. Open the app
2. Go to FAB > Request money
3. Change currency to UAH
4. Close the app and re-open
5. Go to FAB > Request money
6. BUG: UAH is briefly shown before showing user's local currency

[MitchExpensify]

Added that to the OP @situchan !

Let's hunt down some proposals here, what d'you think on the priority level @dylanexpensify ?

[melvin-bot]

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

[melvin-bot]

@MitchExpensify, @situchan Huh... This is 4 days overdue. Who can take care of this?

[MitchExpensify]

@situchan what do you think about this proposal? #35147 (comment)

[situchan]

@JordanLevy19 can you please update proposal to fix #35147 (comment)? And I am not convinced about your solutions. Please provide more details with permalink.

[JordanLevy19]

Hey @situchan - more specific, updated proposal with a fix below! Thank You!

Proposal

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

Users experience a noticeable delay when changing the currency in the IOU request flow. Specifically, after selecting a new currency from the IOURequestStepCurrency and returning to the MoneyRequestAmountForm, the currency symbol updates, but not instantly. Instead, there's a delay where the old currency symbol is visible before it switches to the new one.

What is the root cause of that problem?

The way the currency symbol's update is handled within the state management and corresponding logic. When a user selects a new currency, the component responsible for displaying the currency symbol - BaseTextInputWithCurrencySymbol - does not immediately respond to the state change. Instead, it waits for the next rendering cycle to receive the updated currency prop and then displays the new symbol.

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

Direct modifications to the BaseTextInputWithCurrencySymbol.js

State Management for Currency Symbol

We introduce a localized state within the BaseTextInputWithCurrencySymbol component to manage the currency symbol. The state is directly tied to the selectedCurrencyCode prop to ensure immediate updates.
Upon the selection of a new currency, this state is updated to reflect the change, triggering a re-render of the currency symbol without waiting for the parent component's state to refresh.

UseEffect Hook for Immediate Updates

We leverage the useEffect hook within BaseTextInputWithCurrencySymbol to listen for changes to the selectedCurrencyCode prop. When a change is detected, the localized currency symbol state is updated, which in turn updates the UI instantly.

Code

function BaseTextInputWithCurrencySymbol(props) {
const {fromLocaleDigit} = useLocalize();
const styles = useThemeStyles();

// State to hold the dynamically updated currency symbol
const [localizedCurrencySymbol, setLocalizedCurrencySymbol] = useState(CurrencyUtils.getLocalizedCurrencySymbol(props.selectedCurrencyCode));

// Effect to update the currency symbol when the selected currency code changes
useEffect(() => {
    setLocalizedCurrencySymbol(CurrencyUtils.getLocalizedCurrencySymbol(props.selectedCurrencyCode));
}, [props.selectedCurrencyCode]);
...

}

Permalinks below:

IOURequestStepAmount.js
App/src/pages/iou/request/step/IOURequestStepAmount.js

MoneyRequestAmountForm.js
App/src/pages/iou/steps/MoneyRequestAmountForm.js

BaseTextInputWithCurrencySymbol.js
App/src/components/TextInputWithCurrencySymbol/BaseTextInputWithCurrencySymbol.js

[JordanLevy19]

Proposal

[Updated] - #35147 (comment)

[melvin-bot]

@MitchExpensify @situchan 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!

[situchan]

Thanks. I will provide feedback tomorrow or on Monday

[dylanexpensify]

replied via dm!

[MitchExpensify]

@cristipaval or perhaps @neil-marcellini seeing as you are close to this wave, would either of you be able to take this on? I'm considering doubling to entice more proposals

[melvin-bot]

@MitchExpensify, @situchan Huh... This is 4 days overdue. Who can take care of this?

[MitchExpensify]

Gonna hold on making any moves on this (e.g. doubling) until I see how it falls into the new quarterly release plan

[trjExpensify]

Send money is a IOU feature, so I haven't moved this to #wave-collect, but rather #vip-split

[melvin-bot]

Upwork job price has been updated to $1000

[MitchExpensify]

doubled to help find a solution here https://expensify.slack.com/archives/C01GTK53T8Q/p1709783101385489

[jeremy-croff]

Thanks. We can simplify repro step like this:

1. Open the app
2. Go to FAB > Request money
3. Change currency to UAH
4. Close the app and re-open
5. Go to FAB > Request money
6. BUG: UAH is briefly shown before showing user's local currency

I cannot reproduce it anymore with these steps.

[shahinyan11]

Proposal

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

Mweb-IOU-Previously selected currency is shown for an second in send money confirmation page.

What is the root cause of that problem?

Here we update IOU currency in Onyx data then navigate to MoneyRequestConfirmPage. And because moving between screens works faster than updating Onyx data and re-rendering of MoneyRequestConfirmPage the old interface is displayed briefly

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

1. Add ability to send currency as a route param here. As done for the lower route ( MONEY_REQUEST_CURRENCY )
2. Send currency as route params here

Navigation.navigate(ROUTES.MONEY_REQUEST_CONFIRMATION.getRoute(iouType, reportID, currency));

3. Use currency from route.params instead props.iou.currency everywhere in MoneyRequestConfirmPage component. But using only here also enough to fix problem

What alternative solutions did you explore? (Optional)

We can remove the previous screen ( in this it is MoneyRequestConfirmPage ) from the navigation stack to force it to recreate which will help us avoid displaying the old UI.

1. Add new shouldRecreate parameter for goBack function and add below code here.

    if(shouldRecreate){
        navigationRef.current?.dispatch(StackActions.pop(2));
        navigate(fallbackRoute);
        return;
    }

2. Change this line with below code

Navigation.goBack(ROUTES.MONEY_REQUEST_CONFIRMATION.getRoute(iouType, reportID), false, false, true);

[MitchExpensify]

@shahinyan11 were you able to reproduce this on Android's latest version?

[shahinyan11]

@MitchExpensify Yes.

Screen.Recording.2024-03-08.at.23.57.46.mov

[MitchExpensify]

Thanks!

@situchan - How does @shahinyan11 's proposal look to you?

[jeremy-croff]

Proposal

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

Sometimes, the previous currency is shown for a brief moment before it updates between the edit amount screen and the confirmation screen when sending an IOU.

What is the root cause of that problem?

We see on slower devices, such as mobile or on a slowed-down browser with throttling, that this is a race condition between the rerendering of the component that receives the new currency prop and the animation of the modal stack navigator. This is because, on the edit amount screen, we save the currency at the same time we navigate back.

note: this is virtually unreproducible on faster laptops because the speed at which it can set the onyx variable and receive the new prop is faster than the animation.

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

There are a few ways we can facilitate passing back the selected currency context to the previous screen so that there's no delay. Either through a callback that set the parent state or directly passing the value as a param and using the param.

I choose the param route and would implement it specifically as the following:

1. From the Amount edit screen, we update Navigation.goBack to Navigation.navigate for the confirmation screen here so that we can pass params.
2. Then, we could directly pass the updated currency as a route parameter. (We need to update the ROUTES.ts as well with the new param)
3. We would now add the code to listen to query parameters on the confirmation screen. And we want to add some default logic:
   const routeCurrency = lodashGet(route, 'params.currency', '');
   This is how we can now retrieve the new currency we receive as an optional param.

const currency = CurrencyUtils.isValidCurrencyCode(routeCurrency) ? routeCurrency : iou.currency

This step validates the route param so that users cannot mess it up accidentally. Otherwise, it falls back to the IOU reports currency from Onyx so that all existing flows work as expected.

What alternative solutions did you explore? (Optional)

We can also pass a callback that updates the parent state in the confirm screen and triggers it from the amount screen when they save. This avoids route params altogether, but I preferred following existing patterns more.

[situchan]

reviewing latest proposals

[situchan]

Expecting update early next week

[MitchExpensify]

@situchan We really need an update here after this amount of time. Can you please provide one in the next 24 hours? Otherwise I'll need to reassign to another C+

[situchan]

The 2nd issue I reported here was fixed in #36845.

I am not able to reproduce 1st issue because of loading:

Screen.Recording.2024-03-19.at.11.12.47.AM.mov

@shahinyan11 @jeremy-croff can you please confirm?

[shahinyan11]

@situchan Yes. This is no longer reproducible

[situchan]

@MitchExpensify we can close this