Apply localization to all hard coded text in the Expensify.cash app #2408

michaelhaxhiu · 2021-04-15T14:15:49Z

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

Upwork job - https://www.upwork.com/jobs/~01ad599888651ad174

Expected Result:

We are looking to apply localization to the Expensify.cash app, so that all hard coded text is replaced with locale-aware components/ methods. This project will require (1) Identifying all places that display hardcoded texts in the app, and (2) Using the withLocalize HOC in this PR #2208 to make all those texts support localized versions.

Actual Result:

Today, all text in the Expensify.cash app is hardcoded in English. Here are a few examples for reference:

Platform:

This should apply to all platforms listed:

Expensify/Expensify Issue URL: https://github.com/Expensify/Expensify/issues/152153

Other considerations:

There may be a few areas where it doesn't make sense to translate hardcoded text, which are important to highlight as a part of this project. For example, we should not update the hard corded text found here:

Click avatar (above chat list) > Profile > Set my time zone manually > see dropdown list of regions

The text was updated successfully, but these errors were encountered:

michaelhaxhiu · 2021-04-16T17:28:55Z

Posted to Upwork (job link in GH body) and awaiting proposals.

eVoloshchak · 2021-04-18T17:51:57Z

Proposal

In every component we will use withLocalize HOC from components/withLocalize.js;
I can see that all localization library setup is already done, there are two localization files in "languages" directory;
Since there is no Spanish translation, the es.js locale file will contain placeholder english localization from en.js (to avoid "missing translation" errors);
In each component translation function will be used as such:
const { translations: { translate } } = withLocalize();
And all of the instances of Phrase will be substituted with {translate('phrase)}

Question

Am I correct in thinking that all of the files that need localization are contained in "components" and "pages" directories?
Or is there such files in other directories too?

iwiznia · 2021-04-19T18:34:45Z

In every component we will use withLocalize HOC from components/withLocalize.js;

Not every component, just every component that displays hardcoded english text.

Since there is no Spanish translation, the es.js locale file will contain placeholder english localization from en.js (to avoid "missing translation" errors);

Don't do this, we are not translating the app to spanish yet (or any other language) but also we don't want to leave garbage in the translation files.

In each component translation function will be used as such:
const { translations: { translate } } = withLocalize();

Huh? Why? That's not how you use the higher order component, its usage is similar to the withOnyx higher order component.

And all of the instances of Phrase will be substituted with {translate('phrase)}

Just to clarify, what you pass to the translate method is not the full english text, but the key to the translation.

Am I correct in thinking that all of the files that need localization are contained in "components" and "pages" directories?

Yes, I think that's correct.

eVoloshchak · 2021-04-19T19:37:56Z

Don't do this, we are not translating the app to spanish yet (or any other language) but also we don't want to leave garbage in the translation files.

Got it, thank you for clarification

Huh? Why? That's not how you use the higher order component, its usage is similar to the withOnyx higher order component.

My bad, I confused HOC with hook. I remember being slightly confused seeing withLocalize in "components" directory, thinking it was a hook. I got it now. I also see the "compose" function from redux.

iwiznia · 2021-04-19T19:41:41Z

Can you list out the places we will update? After reviewing that I will approve your proposal.

eVoloshchak · 2021-04-19T21:40:58Z

Can you list out the places we will update? After reviewing that I will approve your proposal.

Here is the list of components that need to be updated:

Components

    AttachmentPicker (native)
    TextInputFocusable
    BaseUpdateAppModal
    AttachmentModal
    ConfirmModal
    HeaderWithCloseButton
    IOUConfirmationList
    OptionsSelector
    RenderHTML
    UnreadActionIndicator
    VideoChatButtonAndMenu
    WelcomeText

Pages
    home
        EmojiPickerMenu
        ReportActionCompose
        ReportActionContextMenu
        ReportActionItemFragment
        ReportActionsView
        ReportTypingIndicator
        SidebarScreen
    iou
        IOUParticipantsRequest
        IOUParticipantsSplit
        IOUAmountPage
        IOUModal
    setings
        LoginField
        ProfilePage
        AddSecondaryLoginPage
        InitialSettingsPage
        PasswordPage
        PaymentsPage
        PreferencesPage
    signin
        LoginFormNarrow
        LoginFormWide
        SignInPageLayoutNarrow
        SignInPageLayoutWide
        TermsOnly
        TermsWithLicenses
        ChangeExpensifyLoginLink
        PasswordForm
        ResendValidationForm

    DetailsPage
    NewChatPage
    NewGroupPage
    NotFound
    ReportParticipantsPage
    SearchPage
    SetPasswordPage

I have another question: what should we do with the text labels in defaultProps or in variables set outside of the react components? These won't be able to use 'translate' function. Should i move all of the default props and constants inside the corresponding react componets?

iwiznia · 2021-04-19T22:59:22Z

Some of those don't seem to have any hardcoded text:

TextInputFocusable
HeaderWithCloseButton
EmojiPickerMenu
ReportActionItemFragment
ReportActionsView

I don't see InitialSettingsPage did you mean InitialPage?

I have another question: what should we do with the text labels in defaultProps or in variables set outside of the react components? These won't be able to use 'translate' function. Should i move all of the default props and constants inside the corresponding react componets?

Great question! I think what you need to do there is basically move the translation to the render method. So, for example here you would:

Make the default prop value an empty string
In the render method, where we use it, do something like: this.props.placeholderText || translate(keyHere)

That way the translation is moved from initialization to the render method and it can get updated whenever the locale changes.
Does that make sense?

eVoloshchak · 2021-04-20T06:22:07Z

Some of those don't seem to have any hardcoded text:

TextInputFocusable - lines 182, 190
HeaderWithCloseButton - line 66
EmojiPickerMenu - line 104
ReportActionItemFragment - lines 68-82
ReportActionsView- line 303

I don't see InitialSettingsPage did you mean InitialPage?

Yes, exactly. Component with dislay name 'InitialSettingsPage' inside the src/pages/settings/InitialPage.js

That way the translation is moved from initialization to the render method and it can get updated whenever the locale changes. Does that make sense?

Yes, it makes perfect sense. Thank you for clarification

P.S.
There is also variables defined outside the component`s scope, for instance CONTEXT_ACTIONS in ReportActionContextMenu and messages and pronouns in CONST.js. How should we handle these?

iwiznia · 2021-04-20T17:50:12Z

Oh, sorry about that. You are right about all those having texts, I missed it. The only one I am having doubts is this one, is that actually printed in screen for the user to see?

There is also variables defined outside the component`s scope, for instance CONTEXT_ACTIONS in ReportActionContextMenu and messages and pronouns in CONST.js. How should we handle these?

Yeah, for those you will need to subscribe to the PREFERRED_LOCALE onyx key and call the translate method directly.

@michaelhaxhiu go ahead and hire @eVoloshchak in upwork.

parasharrajat · 2021-04-20T17:58:02Z

@eVoloshchak

There are also variables defined outside the component`s scope, for instance, CONTEXT_ACTIONS in ReportActionContextMenu and messages and pronouns in CONST.js. How should we handle these?

These should be used somewhere in any component and I think you can call the translate from the HOC there. can't we?

michaelhaxhiu · 2021-04-20T18:01:45Z

@eVoloshchak Hired you on Upwork for the job, and assigned you to this GH. ✅ Feel free to begin drafting a PR for the proposed solution.

Given that this is your first time working on an Expensify.cash job, I wanted to highlight the [Contributor Guidelines]
(https://github.com/Expensify/Expensify.cash/blob/master/CONTRIBUTING.md) in case you haven't read it yet!

iwiznia · 2021-04-20T18:39:35Z

These should be used somewhere in any component and I think you can call the translate from the HOC there. can't we?

I think that won't work since we will have the text, not the key.

parasharrajat · 2021-04-20T18:44:13Z

so can't we set the key as CONST values and thus they would be passed down.
Like

const CONTEXT_ACTIONS = [
    // Copy to clipboard
    {
        text: 'Copy to Clipboard', ==> Key here
        icon: ClipboardIcon,
        successText: 'Copied!',
        successIcon: Checkmark,
        shouldShow: true,

iwiznia · 2021-04-20T18:49:04Z

True, maybe that's better.

eVoloshchak · 2021-04-21T16:30:26Z

The only one I am having doubts is this one, is that actually printed in screen for the user to see?

Judging by the source code it is visible to user on the screen, but I will have to double check when I actually launch the application.

Feel free to begin drafting a PR for the proposed solution. Given that this is your first time working on an Expensify.cash job, I wanted to highlight the [Contributor Guidelines]
(https://github.com/Expensify/Expensify.cash/blob/master/CONTRIBUTING.md) in case you haven't read it yet!

Starting to work on the PR, thank you for the guidelines

so can't we set the key as CONST values and thus they would be passed down.

Got it

michaelhaxhiu · 2021-04-23T13:58:45Z

@eVoloshchak how's the PR coming along so far? Feel free to provide daily updates as you go so that we can have a gauge of your progress.

eVoloshchak · 2021-04-23T16:20:15Z

@eVoloshchak how's the PR coming along so far? Feel free to provide daily updates as you go so that we can have a gauge of your progress.

I have applied localization to a majority of components/pages, I have 4 of them left to be translated. I expect to finish them in a couple of hours, after that i will start testing on different platforms/devices. If tests are successfull, I'll submit a PR today.

iwiznia · 2021-04-26T21:41:57Z

I reverted the PR (left a comment in the PR itself)

eVoloshchak · 2021-04-29T19:27:50Z

I reverted the PR (left a comment in the PR itself)

I've resolved some issues with the PR, but I still have a couple of questions. I've asked them here: #2562

michaelhaxhiu · 2021-05-03T17:44:21Z

What is the latest on this one? I see the PR is merged. Is there another outstanding PR?

iwiznia · 2021-05-03T21:40:03Z

@michaelhaxhiu we reverted the PR ad @eVoloshchak was waiting on some answers from me which they did not get till today since I was OOO.

iwiznia · 2021-05-05T21:20:50Z

@eVoloshchak can you confirm you are working on this?

eVoloshchak · 2021-05-06T16:55:11Z

@eVoloshchak can you confirm you are working on this?

Yes, I'm working on this, planning to submit a PR in a couple of hours.

P.S: submitted a new PR.
#2718

michaelhaxhiu · 2021-05-10T12:27:50Z

Nice work @eVoloshchak, looks like that PR is still being reviewed right now. I'll check back in on this later today to see if we've finalized all comments and merged.

Julesssss · 2021-05-11T13:06:47Z

Hi @eVoloshchak, I think this PR has caused a regression with the IOU Modal, but this wasn't possible for you to test, so no worries 🙂

I'm seeing a crash here which doesn't occur on previous commits:

Uncaught TypeError: this.textInput.focus is not a function
    at IOUAmountPage.componentDidMount (IOUAmountPage.js:60)
    at commitLifeCycles (react-dom.development.js:19814)
    at commitLayoutEffects (react-dom.development.js:22803)
    at HTMLUnknownElement.callCallback (react-dom.development.js:188)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:237)
    at invokeGuardedCallback (react-dom.development.js:292)
    at commitRootImpl (react-dom.development.js:22541)
    at unstable_runWithPriority (scheduler.development.js:653)
    at runWithPriority$1 (react-dom.development.js:11039)
    at commitRoot (react-dom.development.js:22381)

As the feature is currently behind a beta, you must perform the following change to reproduce the issue. Either navigate to Permissions.js, comment out this line and return true -- or apply this diff.txt with git apply path/to/diff.txt

michaelhaxhiu · 2021-05-11T15:02:18Z

Re-opening this GH until the regression is fixed, which @Julesssss linked above.

iwiznia · 2021-05-11T19:21:42Z

There's also this other error: Error: addSecondaryLoginPage.password was not found in the default language which we need to fix too.
@eVoloshchak are you on it?

iwiznia · 2021-05-11T20:59:19Z

I am unsure if this change is what caused this, I think it did not.

eVoloshchak · 2021-05-11T21:05:59Z

There's also this other error: Error: addSecondaryLoginPage.password was not found in the default language which we need to fix too.
@eVoloshchak are you on it?

I have fixed the error with addSecondaryLoginPage.password, ready to submit a PR.
Do I just create a new PR referencing this issue?
I can see that there is already a pull request that resolves this issue. Should I carry on with fixing it?

iwiznia · 2021-05-11T21:30:27Z

Do I just create a new PR referencing this issue?

Yep and assign me for review

I can see that there is already a pull request that resolves this issue. Should I carry on with fixing it?

Nope, someone else is handling it, so no need.

eVoloshchak · 2021-05-11T21:44:30Z

Yep and assign me for review

I'm not sure that I can assign reviewers for my commit, couldn't find it anywhere. Isn't it available only to the owner of the repository?

iwiznia · 2021-05-11T21:45:49Z

You can't add a reviewer? Odd.. anyway, I see the PR above, so reviewing it now.

iwiznia · 2021-06-07T19:03:05Z

Reopening since a couple of issues were found

michaelhaxhiu · 2021-06-07T19:11:04Z

How long ago were the issues found @iwiznia? This contributor has already been compensated for the job and so I'm thinking it may be better to open a fresh GH.

iwiznia · 2021-06-07T19:14:34Z

I know that we already paid, but the issues were there since the beginning and I expect @eVoloshchak to fix them.

michaelhaxhiu · 2021-06-07T19:26:54Z

Ah, I understand! Ok let's keep this GH going in that case 👍

iwiznia self-assigned this Apr 15, 2021

michaelhaxhiu added Exported Daily KSv2 labels Apr 16, 2021

michaelhaxhiu assigned eVoloshchak Apr 20, 2021

iwiznia removed their assignment Apr 22, 2021

eVoloshchak mentioned this issue Apr 23, 2021

Apply localization to all hard coded text #2562

Merged

5 tasks

Luke9389 closed this as completed in #2562 Apr 26, 2021

iwiznia reopened this Apr 26, 2021

eVoloshchak mentioned this issue May 6, 2021

Apply localization to all hard coded text #2718

Merged

5 tasks

iwiznia closed this as completed in #2718 May 10, 2021

michaelhaxhiu reopened this May 11, 2021

eVoloshchak mentioned this issue May 11, 2021

Fix translation typo on AddSecondaryLoginPage #2808

Merged

5 tasks

iwiznia closed this as completed in #2808 May 11, 2021

iwiznia reopened this Jun 7, 2021

eVoloshchak mentioned this issue Jun 8, 2021

Apply localization to all hard coded text in the Expensify.cash app #3435

Merged

Jag96 closed this as completed in #3435 Jun 9, 2021

Apply localization to all hard coded text in the Expensify.cash app #2408

Apply localization to all hard coded text in the Expensify.cash app #2408

Comments

michaelhaxhiu commented Apr 15, 2021 • edited Loading

Expected Result:

Actual Result:

Platform:

Other considerations:

michaelhaxhiu commented Apr 16, 2021

eVoloshchak commented Apr 18, 2021

Proposal

Question

iwiznia commented Apr 19, 2021

eVoloshchak commented Apr 19, 2021

iwiznia commented Apr 19, 2021

eVoloshchak commented Apr 19, 2021 • edited Loading

iwiznia commented Apr 19, 2021

eVoloshchak commented Apr 20, 2021 • edited Loading

iwiznia commented Apr 20, 2021

parasharrajat commented Apr 20, 2021

michaelhaxhiu commented Apr 20, 2021

iwiznia commented Apr 20, 2021

parasharrajat commented Apr 20, 2021

iwiznia commented Apr 20, 2021

eVoloshchak commented Apr 21, 2021

michaelhaxhiu commented Apr 23, 2021

eVoloshchak commented Apr 23, 2021

iwiznia commented Apr 26, 2021

eVoloshchak commented Apr 29, 2021

michaelhaxhiu commented May 3, 2021 • edited Loading

iwiznia commented May 3, 2021

iwiznia commented May 5, 2021

eVoloshchak commented May 6, 2021 • edited Loading

michaelhaxhiu commented May 10, 2021

Julesssss commented May 11, 2021

michaelhaxhiu commented May 11, 2021

iwiznia commented May 11, 2021

iwiznia commented May 11, 2021

eVoloshchak commented May 11, 2021

iwiznia commented May 11, 2021

eVoloshchak commented May 11, 2021 • edited Loading

iwiznia commented May 11, 2021

iwiznia commented Jun 7, 2021

michaelhaxhiu commented Jun 7, 2021

iwiznia commented Jun 7, 2021

michaelhaxhiu commented Jun 7, 2021

michaelhaxhiu commented Apr 15, 2021 •

edited

Loading

eVoloshchak commented Apr 19, 2021 •

edited

Loading

eVoloshchak commented Apr 20, 2021 •

edited

Loading

michaelhaxhiu commented May 3, 2021 •

edited

Loading

eVoloshchak commented May 6, 2021 •

edited

Loading

eVoloshchak commented May 11, 2021 •

edited

Loading