Localization for Amount, Numbers and Dates. #3021

parasharrajat · 2021-05-19T23:08:12Z

Please Review @iwiznia.

Details

Fixed Issues

Fixes #2349
Fixes #3038

Tests / QA Steps

Localization #2349

Open E.cash
All the times and phone numbers should be localized.

Expensify SMS Domain #3038

Create and account in e.cash with a Phone number
Do not set a name or last name in the account.
Log in as another account
Create a new group chat that includes the phone number account.
@expensify.sms should not be visible in any area of the app.

Tested On

Screenshots

Web / Desktop

translations.mp4

IOS

Android

1622059575660.mp4

…harrajat/translations

src/libs/Permissions.js

parasharrajat · 2021-05-24T22:54:58Z

Please let me know If it's going in the right direction. I personally looked at all the pages and added changes. If I am missing some parts, please mention that. Thanks.

There are few places where we are already using localization IOUConfirmationList. There are few errors on the IOU details pages so I am not unable to open them. But I will check the code if changes are required there.

Jag96

This is looking good to me so far, but I'll let @iwiznia comment since he's got more context here

iwiznia · 2021-05-25T19:51:16Z

There are few errors on the IOU details pages so I am not unable to open them

Oh damn, what errors? Also, make sure to regularly pull main and merge it into your branch, since it is possible some of those are already fixed.

parasharrajat · 2021-05-25T20:02:00Z

@iwiznia when I click Pay its was crashing the app. I have reported it and I think its being fixed. I will pull in the latest changes and check again.

iwiznia

Looking good

iwiznia · 2021-05-25T20:25:50Z

src/pages/DetailsPage.js

@@ -71,20 +74,20 @@ const DetailsPage = ({personalDetails, route, translate}) => {
                                />
                            </View>
                            <Text style={[styles.displayName, styles.mt1, styles.mb6]} numberOfLines={1}>
-                                {details.displayName
-                                    ? details.displayName
-                                    : null}


So, before when details.displayName was falsy, we would render null, now we would render details.displayName, is that what we want? Are there uninteded consequences of that? Mostly, I am wondering why we had this ternary condition here if there are no other consequences.

I need to pull latest changes and I think thus this ternary would change. Why I did this as I believe we will never has a user who don't have a handle. In other words, can you imagine chating with a user who has no name(login | phone number). So if we are ended up null then we are not doing it correctly. Anyways, this issues has been fixed recently. I will pull the changes.

I have updated it so return as null when there is no displayName.

src/libs/Permissions.js

src/pages/DetailsPage.js

parasharrajat · 2021-05-26T19:50:26Z

Ready for review. Also pulled latest changes from Main.

Co-authored-by: Ionatan Wiznia <iwiznia@hotmail.com>

iwiznia · 2021-05-26T20:00:42Z

Code looks good, can you test in all platforms and remove WIP? I will do a final review then.

parasharrajat · 2021-05-27T20:27:57Z

Updated. Ready to review

iwiznia · 2021-05-27T20:39:56Z

@parasharrajat did you test on ios? I am not seeing the video for it.

@Jag96 do you want to do one last review?

parasharrajat · 2021-05-27T20:41:32Z

Yeah, I tested it, I will update the video soon.

iwiznia · 2021-05-28T16:15:04Z

Oh, @Jag96 is out of office, @parasharrajat if you upload the ios video test, I'll merge.

parasharrajat · 2021-05-28T16:27:07Z

I am having issues with my mac system but I am try to recover it and hope it will work.

iwiznia · 2021-05-31T19:46:28Z

Any luck with that @parasharrajat?

parasharrajat · 2021-06-01T11:23:13Z

@iwiznia I'll update you today.

Jag96

Looks good, just one question about the format in some places. In the Header, I see we don't show the + and the area code, but in other places we do (example below). Is there a reason it isn't the same in both places?

iwiznia · 2021-06-01T19:42:19Z

If I had to guess is because that's text generated in the server and not in the client. Since we don't have I18n in the server, I don't think we can fix that right now.

parasharrajat · 2021-06-01T21:40:32Z

Yeah. I checked the main data and see that this message is coming from backend. But I can run a search for PH number on text and replace all the instances, if you want this?

iwiznia · 2021-06-01T21:59:52Z

No, I don't think we want that. We will need to solve this at some point when we add internationalization to the backend.

@Jag96 any other concerns? If not, do you mind merging this?

Jag96

No, I don't think we want that. We will need to solve this at some point when we add internationalization to the backend.

👍

OSBotify · 2021-06-01T22:09:23Z

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

parasharrajat added 8 commits May 20, 2021 04:35

chg: localize numbers

ebdb7ce

Merge branch 'main' of github.com:Expensify/Expensify.cash into paras…

e3eba26

…harrajat/translations

Merge branch 'main' of github.com:Expensify/Expensify.cash into paras…

199b5f1

…harrajat/translations

Merge branch 'master' into parasharrajat/translations

47c7646

Phone Number Localization

404381a

fix: currency amount in IOUBadge

989b31c

localization for chat message rows.

71cd0ea

localize phone numbers

f349f5c

parasharrajat commented May 24, 2021

View reviewed changes

src/libs/Permissions.js Outdated Show resolved Hide resolved

parasharrajat changed the title ~~Localization for Amount, Numbers and Dates.~~ [WIP] Localization for Amount, Numbers and Dates. May 24, 2021

parasharrajat marked this pull request as ready for review May 24, 2021 22:51

parasharrajat requested a review from a team as a code owner May 24, 2021 22:51

MelvinBot requested review from Jag96 and removed request for a team May 24, 2021 22:51

Jag96 requested a review from iwiznia May 24, 2021 23:58

Jag96 reviewed May 25, 2021

View reviewed changes

iwiznia reviewed May 25, 2021

View reviewed changes

pull latest changes

027ac6c

parasharrajat mentioned this pull request May 26, 2021

LHN - Phone number accounts without name set are showing @Expensify.sms in group chats #3038

Closed

parasharrajat added 3 commits May 26, 2021 22:51

remove unnecessary changes

dd39906

nullify name in detailsPage when no DisplayName

d4ec7d4

fix: removed @expensify.sms from login

ad470f3

parasharrajat force-pushed the parasharrajat/translations branch from d9cd22b to ad470f3 Compare May 26, 2021 19:17

iwiznia reviewed May 26, 2021

View reviewed changes

src/pages/DetailsPage.js Outdated Show resolved Hide resolved

Update src/pages/DetailsPage.js

40908d6

Co-authored-by: Ionatan Wiznia <iwiznia@hotmail.com>

parasharrajat changed the title ~~[WIP] Localization for Amount, Numbers and Dates.~~ Localization for Amount, Numbers and Dates. May 26, 2021

iwiznia approved these changes May 27, 2021

View reviewed changes

Jag96 reviewed Jun 1, 2021

View reviewed changes

Jag96 approved these changes Jun 1, 2021

View reviewed changes

Jag96 merged commit 6cf79ac into Expensify:main Jun 1, 2021

OSBotify mentioned this pull request Jun 1, 2021

Deploy Checklist: Expensify.cash 2021-06-01 #3279

Closed

9 tasks

parasharrajat deleted the parasharrajat/translations branch November 20, 2023 13:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Localization for Amount, Numbers and Dates. #3021

Localization for Amount, Numbers and Dates. #3021

parasharrajat commented May 19, 2021 •

edited

Loading

parasharrajat commented May 24, 2021

Jag96 left a comment

iwiznia commented May 25, 2021

parasharrajat commented May 25, 2021

iwiznia left a comment

iwiznia May 25, 2021

parasharrajat May 25, 2021

parasharrajat May 26, 2021

parasharrajat commented May 26, 2021 •

edited

Loading

iwiznia commented May 26, 2021

parasharrajat commented May 27, 2021 •

edited

Loading

iwiznia commented May 27, 2021

parasharrajat commented May 27, 2021

iwiznia commented May 28, 2021

parasharrajat commented May 28, 2021

iwiznia commented May 31, 2021

parasharrajat commented Jun 1, 2021

Jag96 left a comment

iwiznia commented Jun 1, 2021

parasharrajat commented Jun 1, 2021

iwiznia commented Jun 1, 2021

Jag96 left a comment

OSBotify commented Jun 1, 2021

Localization for Amount, Numbers and Dates. #3021

Localization for Amount, Numbers and Dates. #3021

Conversation

parasharrajat commented May 19, 2021 • edited Loading

Details

Fixed Issues

Tests / QA Steps

Localization #2349

Expensify SMS Domain #3038

Tested On

Screenshots

Web / Desktop

IOS

Android

parasharrajat commented May 24, 2021

Jag96 left a comment

Choose a reason for hiding this comment

iwiznia commented May 25, 2021

parasharrajat commented May 25, 2021

iwiznia left a comment

Choose a reason for hiding this comment

iwiznia May 25, 2021

Choose a reason for hiding this comment

parasharrajat May 25, 2021

Choose a reason for hiding this comment

parasharrajat May 26, 2021

Choose a reason for hiding this comment

parasharrajat commented May 26, 2021 • edited Loading

iwiznia commented May 26, 2021

parasharrajat commented May 27, 2021 • edited Loading

iwiznia commented May 27, 2021

parasharrajat commented May 27, 2021

iwiznia commented May 28, 2021

parasharrajat commented May 28, 2021

iwiznia commented May 31, 2021

parasharrajat commented Jun 1, 2021

Jag96 left a comment

Choose a reason for hiding this comment

iwiznia commented Jun 1, 2021

parasharrajat commented Jun 1, 2021

iwiznia commented Jun 1, 2021

Jag96 left a comment

Choose a reason for hiding this comment

OSBotify commented Jun 1, 2021

parasharrajat commented May 19, 2021 •

edited

Loading

parasharrajat commented May 26, 2021 •

edited

Loading

parasharrajat commented May 27, 2021 •

edited

Loading