Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Web - IOU - Previously used currency appears for a brief moment for request/ split money #3560

Closed
kavimuru opened this issue Jun 11, 2021 · 8 comments · Fixed by #3577
Closed
Assignees
Labels
Daily KSv2 Engineering Reviewing Has a PR in review

Comments

@kavimuru
Copy link

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Expected Result:

Regardless of the previously selected currency, the currency should match user's IP location

Actual Result:

Previously selected currency flashes for a brief moment, then currency of user's IP location appears

Action Performed:

  1. Go to staging.expensify.cash and login
  2. Send an IOU request with a currency that does not match user's IP location
  3. Open the IOU model again by clicking "Request money" or "split bill"

Workaround:

Unknown

Platform:

Where is this issue occurring?

Web ✔️
iOS ✔️
Android ✔️
Desktop App ✔️
Mobile Web ✔️

Version Number: 1.0.68-0
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Bug5109090_Recording__82.mp4

Expensify/Expensify Issue URL:

View all open jobs on Upwork

@MelvinBot
Copy link

Triggered auto assignment to @HorusGoul (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@isagoico
Copy link

Issue reproducible today during KI retests

@Julesssss
Copy link
Contributor

I think the fix here would be to show the loading icon while we retrieve currency. The iou key already has an isLoading key for this.

@HorusGoul
Copy link
Contributor

@Julesssss Just tried to do that, but the iou.loading seems to be for a different thing.

We could fix this by returning a promise from the PersonalDetails.fetchCurrencyPreferences() call and then have a state key that we can use to display the loading icon.

@Julesssss
Copy link
Contributor

Yeah, we reuse the iou.loading key in a few places to avoid duplication, but the IOU Modal is the right place for this IMO. Alternatively we could have a new key/pair under the key: isRetreivingCurrency or something similar.

As for managing state, we prefer to drive everything from Onyx, hence the above keys. Here's some additional context: https://expensify.slack.com/archives/C01GTK53T8Q/p1623064998057400

@Julesssss
Copy link
Contributor

The other problem is that functions that occur within the 'Action' files, should not return anything. Components should act upon data from Onyx only -- our offline-first architecture.

@HorusGoul
Copy link
Contributor

I see! I was able to get it to work using Onyx:

Screen.Recording.2021-06-14.at.13.10.44.mov

I'll open a PR with the fix.

Thanks for the guidance, @Julesssss! 😊

@Julesssss
Copy link
Contributor

Nice, that looks ideal!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 Engineering Reviewing Has a PR in review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants