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

Log in - Language picker selection should be remembered after you log in or sign up #4999

Closed
isagoico opened this issue Sep 1, 2021 · 7 comments
Assignees
Labels
Daily KSv2 Engineering Improvement Item broken or needs improvement. Reviewing Has a PR in review

Comments

@isagoico
Copy link

isagoico commented Sep 1, 2021

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


Action Performed:

  1. Navigate to staging.new.expensify.com
  2. In the language picker select spanish
  3. Log in or sign up

Expected Result:

Language selection should still be the same after login in or signing up

Actual Result:

After log in or sign up the language is displayed in english. In case of the existing user, it will always change to the language that was previously selected in preferences.

Workaround:

N/A.

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.0.91-0

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

image

Expensify/Expensify Issue URL:

View all open jobs on GitHub


From @roryabraham https://expensify.slack.com/archives/C01GTK53T8Q/p1630453024244700

NEW FEATURE (internal): When you choose a locale in the signup and login pages, your choice should be remembered after you sign up or log in.

@MelvinBot
Copy link

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

@MonilBhavsar MonilBhavsar added the Improvement Item broken or needs improvement. label Sep 2, 2021
@MonilBhavsar
Copy link
Contributor

When we change the preferred language in the login page, we update that in Onyx, but we don't update that in NVP.
The statement here gets the NVP(previous preference) and overwrites the Onyx data.

I tried changing the above statement to manually make API call to get preferred language from NVP and update it, if it's different from selected one. And finally set the preferred language in Onyx.

@roryabraham
Copy link
Contributor

@MonilBhavsar Am I correct in understanding that your proposal is something like this?

  1. From the login/signup page, update the NVP_PREFERRED_LOCALE in Onyx.
  2. Once the user logs in, set the NVP_PREFERRED_LOCALE in the API to match their choice made in Onyx, instead of reading the NVP from Onyx and overwriting the choice made in Onyx.

Because I think that might work (and could even be external).

However, we need to be careful and only set the preferred locale NVP if the user switched to a non-standard locale before logging in or signing up. Basically we want to make sure we're avoiding this situation:

  1. An existing user sets a non-standard locale such as es.
  2. They go to New Expensify on a new device, and go through the login form in English
  3. We overwrite their previously set non-standard locale to the standard en.
    • Instead, we should check if the locale set in Onyx is the default en, and only if it is not should we set the NVP.

With that qualification I think this proposal looks good. Feel free to implement it yourself or make it external. Up to you, but I think it might be less work at this point to just keep it internal (since we have already found a solution we think will work).

@MonilBhavsar
Copy link
Contributor

MonilBhavsar commented Sep 2, 2021

  1. From the login/signup page, update the NVP_PREFERRED_LOCALE in Onyx.

We already update that in Onyx here, Also we have a call to update the NVP in setLocale(), but we can't do since we don't have a token.

  1. Once the user logs in, set the NVP_PREFERRED_LOCALE in the API to match their choice made in Onyx, instead of reading the NVP from Onyx and overwriting the choice made in Onyx.

Yes exactly, I think we can do that in Authscreen and replace the current statement that overwrites with Onyx data

Instead, we should check if the locale set in Onyx is the default en, and only if it is not should we set the NVP.

So, we don't set the NVP if we they have chosen en, right? Currently we don't have any check for this, but can add that.

Yes, I'll handle this as I am trying to make myself familiar with this repo

@roryabraham
Copy link
Contributor

So, we don't set the NVP if we they have chosen en, right? Currently we don't have any check for this, but can add that.

Yep, that sounds correct!

@MonilBhavsar MonilBhavsar added the Reviewing Has a PR in review label Sep 2, 2021
@MelvinBot
Copy link

@MonilBhavsar Whoops! This issue is 2 days overdue. Let's get this updated quick!

@MonilBhavsar
Copy link
Contributor

Closing it as it's deployed to prod

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

No branches or pull requests

4 participants