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.

Sign up for GitHub

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

Jump to bottom

Save preferred locale in NVP #4055

Merged
merged 8 commits into from
Jul 21, 2021
Merged

Save preferred locale in NVP #4055

merged 8 commits into from
Jul 21, 2021

Conversation

luacmartins
Copy link
Contributor

@luacmartins luacmartins commented Jul 14, 2021 •
edited
Loading

cc @chiragsalian

Related Web-E PR: https://github.com/Expensify/Web-Expensify/pull/31508

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/167702

Tests

  1. Open app in two different devices, e.g. web and desktop and login with the same user.
  2. On web, go to Settings > Preferences and change the Language.
  3. Confirm that the language changes on Desktop too and vice-versa.

QA Steps

Steps above.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

web.mov

Mobile Web

mobile-web.mov

Desktop

desktop.mov

iOS

ios.mov

Android

android.mov

@luacmartins luacmartins self-assigned this Jul 14, 2021
@luacmartins luacmartins requested a review from a team July 19, 2021 22:24
@MelvinBot MelvinBot requested review from thienlnam and removed request for a team July 19, 2021 22:24
@luacmartins luacmartins marked this pull request as ready for review July 19, 2021 22:25
@luacmartins luacmartins requested a review from a team as a code owner July 19, 2021 22:25
@MelvinBot MelvinBot requested review from Dal-Papa and removed request for a team July 19, 2021 22:25
@luacmartins luacmartins removed the request for review from Dal-Papa July 19, 2021 22:26
@luacmartins luacmartins changed the title Save preferred locale in NVP [HOLD Web-E PR #31508] Save preferred locale in NVP Jul 19, 2021
thienlnam
thienlnam reviewed Jul 20, 2021
View reviewed changes
Copy link
Contributor

@thienlnam thienlnam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good, going to test this tommorow

@luacmartins luacmartins changed the title [HOLD Web-E PR #31508] Save preferred locale in NVP Save preferred locale in NVP Jul 20, 2021
@luacmartins
Copy link
Contributor Author

Hold removed here

@github-actions
Copy link
Contributor

github-actions bot commented Jul 20, 2021 •
edited
Loading

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@luacmartins
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

thienlnam
thienlnam requested changes Jul 20, 2021
View reviewed changes
Copy link
Contributor

@thienlnam thienlnam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm testing it but the changes don't seem to be syncing between my web and desktop apps. Even if I change it Spanish on Desktop, and refresh on Web, the Web is still all in english

@luacmartins
Copy link
Contributor Author

Are you seeing any console or network errors?

@thienlnam
Copy link
Contributor

Hmm looks like I'm getting some CORS errors, might just be my development setup

Screen Shot 2021-07-20 at 10 51 53 AM

@luacmartins
Copy link
Contributor Author

Maybe this might help? I'm not sure what solved it for me.

@thienlnam
Copy link
Contributor

Hmm still not able to get it to work but since it's a cors issue and you were able to get it to work successfully, we'll have it test again during QA

thienlnam
thienlnam approved these changes Jul 21, 2021
View reviewed changes
Copy link
Contributor

@thienlnam thienlnam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving code changes

@thienlnam thienlnam merged commit 7e395c5 into main Jul 21, 2021
@thienlnam thienlnam deleted the cmartins-preferred-locale branch July 21, 2021 23:30
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@thienlnam thienlnam restored the cmartins-preferred-locale branch July 23, 2021 15:32
@OSBotify OSBotify mentioned this pull request Jul 23, 2021
Closed
60 tasks
@OSBotify
Copy link
Contributor

🚀 Deployed to staging in version: 1.0.79-5🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production in version: 1.0.80-2🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@luacmartins luacmartins deleted the cmartins-preferred-locale branch August 19, 2021 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thienlnam thienlnam thienlnam approved these changes

Assignees

@luacmartins luacmartins

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@luacmartins @thienlnam @OSBotify