Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.55k stars 2.9k forks source link

[HOLD for payment 2021-11-22] Missing spanish translations in VBA flow (Company types and routing and bank account number example) - Reported by: @Santhosh-Sellavel #5992

Closed isagoico closed 2 years ago

isagoico commented 3 years ago

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. Set app to spanish
  2. Navigate to bank account flow > Manually set bank account
  3. Check the image example above the routing and bank account number
  4. Enter test credentials and proceed to company step
  5. Click on the company type dropdown

Expected Result:

All page should be correctly translated to spanish.

Actual Result:

The example image in add bank account modal is not translated along with the company type options in Company step

Workaround:

None needed.

Platform:

Where is this issue occurring?

Version Number: 1.1.8-8

Reproducible in staging?: Yes Reproducible in production?: Yes

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

Notes/Photos/Videos: Any additional supporting documentation

image image

Expensify/Expensify Issue URL:

Issue reported by: @Santhosh-Sellavel Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1634748908001600

View all open jobs on GitHub

MelvinBot commented 3 years ago

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

flodnv commented 3 years ago

Thanks for the report!

As I expected, this is an image. @iwiznia what should we do here? Do we have an existing way to handle "translated images"?

iwiznia commented 3 years ago

Oh this is a new one, we have not needed this till now. Assuming we need an image and can't do this with text (which I am not sure if we can or not in this specific case), this is what we could do:

PrashantMangukiya commented 3 years ago

Proposed Solution: (If considered as External)

Current Code :

https://github.com/Expensify/App/blob/6d504d0c1b8f9af1242e17c701b18f1242f133ae/src/pages/ReimbursementAccount/BankAccountStep.js#L19

https://github.com/Expensify/App/blob/6d504d0c1b8f9af1242e17c701b18f1242f133ae/src/pages/ReimbursementAccount/BankAccountStep.js#L259-L263

https://github.com/Expensify/App/blob/6d504d0c1b8f9af1242e17c701b18f1242f133ae/src/pages/ReimbursementAccount/CompanyStep.js#L244-L251

Solution:

As we do not need to use example check image on multiple pages, so we can implement simple solution as under: i.e. We have to put language specific image within asset folder i.e. example-check-image-en.png and example-check-image-es.png and update code within /src/pages/ReimbursementAccount/BankAccountStep.js as under:

// import exampleCheckImage from '../../../assets/images/example-check-image.png';   // *** Old code
import exampleCheckImageEn from '../../../assets/images/example-check-image-en.png'; // *** New code
import exampleCheckImageEs from '../../../assets/images/example-check-image-es.png'; // *** New code

constructor(props) { 
  this.getExampleCheckImage = this.getExampleCheckImage.bind(this);   // *** Add this line
}

// Add this function
getExampleCheckImage() {
    switch (this.props.preferredLocale) {
        case 'en':
            return exampleCheckImageEn;
        case 'es':
            return exampleCheckImageEs;
        default:
            return exampleCheckImageEn;
    }
}

<Image
    resizeMode="contain"
    style={[styles.exampleCheckImage, styles.mb5]}
    // source={exampleCheckImage}           // *** Old code
    source={this.getExampleCheckImage()}   // *** New code
/>

Update code within src/pages/ReimbursementAccount/CompanyStep.js file to localise incorporationTypes as under:

<ExpensiPicker
    label={this.props.translate('companyStep.companyType')}
    // items={_.map(CONST.INCORPORATION_TYPES, (label, value) => ({value, label}))}   // *** OLD 
    items={_.map(this.props.translate('companyStep.incorporationTypes'), (label, value) => ({value, label}))}  // *** New
    onChange={value => this.clearErrorAndSetValue('incorporationType', value)}
    value={this.state.incorporationType}
    placeholder={{value: '', label: '-'}}
    hasError={this.getErrors().incorporationType}
/>

Add incorporationTypes within src/languages/en.js as under:

  companyStep: {
       ...,
       incorporationTypes: {
          LLC: 'LLC',
          CORPORATION: 'Corp',
          PARTNERSHIP: 'Partnership',
          COOPERATIVE: 'Cooperative',
          SOLE_PROPRIETORSHIP: 'Sole Proprietorship',
          OTHER: 'Other',
      },
  },

Add incorporationTypes within src/languages/es.js as under:

  companyStep: {
       ...,
        incorporationTypes: {
            LLC: 'LLC',
            CORPORATION: 'Corp',
            PARTNERSHIP: 'Camaradería',
            COOPERATIVE: 'Cooperativa',
            SOLE_PROPRIETORSHIP: 'Propietario único',
            OTHER: 'Otra',
        },
  },
MelvinBot commented 3 years ago

Triggered auto assignment to @NicMendonca (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

MelvinBot commented 3 years ago

Triggered auto assignment to @timszot (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

NicMendonca commented 3 years ago

Upwork job post: https://www.upwork.com/jobs/~01b30b6babc6764eb4

iwiznia commented 3 years ago

@PrashantMangukiya we would like a solution more similar to this one.

Do you know if this is possible?

We import the image name coming from the translation like we import the image now: import image from '../../../assets/images/'+imageNameFromLocale; (I am unsure about this, I assume we can import things on the fly, but not really sure)

PrashantMangukiya commented 3 years ago

@iwiznia let me think about it. I will post message if found any possible solution that way. Thanks.

PrashantMangukiya commented 3 years ago

I did some research and came to know that, import can not be dynamic i.e. we can not use variable within import statement.

If image loaded from remote url i.e. network/cloud image then we can think other solution to load it dynamically.

Here is other possible solution we can use for image: Create new file src/pages/ReimbursementAccount/exampleCheckImage.js as under:

import exampleCheckImageEn from '../../../assets/images/example-check-image-en.png';
import exampleCheckImageEs from '../../../assets/images/example-check-image-es.png';

const images = {
    en: exampleCheckImageEn,
    es: exampleCheckImageEs,
};

function exampleCheckImage(languageKey = 'en') {
    return images[languageKey];
}

export default exampleCheckImage;

Update code within /src/pages/ReimbursementAccount/BankAccountStep.js file as under (it looks clean)

// import exampleCheckImage from '../../../assets/images/example-check-image.png';   // Old code
import exampleCheckImage from './exampleCheckImage';  // New code

<Image
    resizeMode="contain"
    style={[styles.exampleCheckImage, styles.mb5]}
    // source={exampleCheckImage}   // Old code
    source={exampleCheckImage(this.props.preferredLocale)}  // New Code
/>

I will keep thinking and post message if found any other solution.

iwiznia commented 3 years ago

Oh that's a bummer about dynamically importing files. I like the proposed solution. Can I ask you to also add a blurb in the Readme under Internationalization saying that to include localized images, we can copy what this new file is doing? @timszot if you agree, please let @NicMendonca to hire @PrashantMangukiya.

PrashantMangukiya commented 3 years ago

@iwiznia thanks, yes I will add some grammar within readme under Internationalization section.

To implement image solution we need example-check-image-es.png with Spanish text. So someone from graphics team need to provide me Spanish image. Note: Spanish image should be same Size and Quality of english image used at present, So it looks consistent.

English image already there so we will rename it i.e. example-check-image.png to example-check-image-en.png

Here is Final Solution:

Create new file src/pages/ReimbursementAccount/exampleCheckImage.js as under:

import exampleCheckImageEn from '../../../assets/images/example-check-image-en.png';
import exampleCheckImageEs from '../../../assets/images/example-check-image-es.png';

const images = {
    en: exampleCheckImageEn,
    es: exampleCheckImageEs,
};

function exampleCheckImage(languageKey = 'en') {
    return images[languageKey];
}

export default exampleCheckImage;

Update code within /src/pages/ReimbursementAccount/BankAccountStep.js file as under (it looks clean)

// import exampleCheckImage from '../../../assets/images/example-check-image.png';   // Old code
import exampleCheckImage from './exampleCheckImage';  // New code

<Image
    resizeMode="contain"
    style={[styles.exampleCheckImage, styles.mb5]}
    // source={exampleCheckImage}   // Old code
    source={exampleCheckImage(this.props.preferredLocale)}  // New Code
/>

Update code within src/pages/ReimbursementAccount/CompanyStep.js file to localise incorporationTypes as under:

<ExpensiPicker
    label={this.props.translate('companyStep.companyType')}
    // items={_.map(CONST.INCORPORATION_TYPES, (label, value) => ({value, label}))}   // *** OLD 
    items={_.map(this.props.translate('companyStep.incorporationTypes'), (label, value) => ({value, label}))}  // *** New
    onChange={value => this.clearErrorAndSetValue('incorporationType', value)}
    value={this.state.incorporationType}
    placeholder={{value: '', label: '-'}}
    hasError={this.getErrors().incorporationType}
/>

Add incorporationTypes within src/languages/en.js as under:

  companyStep: {
       ...,
       incorporationTypes: {
          LLC: 'LLC',
          CORPORATION: 'Corp',
          PARTNERSHIP: 'Partnership',
          COOPERATIVE: 'Cooperative',
          SOLE_PROPRIETORSHIP: 'Sole Proprietorship',
          OTHER: 'Other',
      },
  },

Add incorporationTypes within src/languages/es.js as under:

  companyStep: {
       ...,
        incorporationTypes: {
            LLC: 'LLC',
            CORPORATION: 'Corp',
            PARTNERSHIP: 'Camaradería',
            COOPERATIVE: 'Cooperativa',
            SOLE_PROPRIETORSHIP: 'Propietario único',
            OTHER: 'Otra',
        },
  },
iwiznia commented 3 years ago

To implement image solution we need example-check-image-es.png with Spanish text. So someone from graphics team need to provide me Spanish image. Note: Spanish image should be same Size and Quality of english image used at present, So it looks consistent.

cc @Expensify/design can you provide this?

Update code within src/pages/ReimbursementAccount/CompanyStep.js file to localise incorporationTypes as under:

Isn't that a different issue? Also I think it would not solve it (but there's conversations about that in some other issue).

shawnborton commented 3 years ago

What are the spanish words for "Routing Number" and "Account Number"?

PrashantMangukiya commented 3 years ago

Update code within src/pages/ReimbursementAccount/CompanyStep.js file to localise incorporationTypes as under:

Yes localise incorporationTypes is different things, but it was within title of this issue so I suggested solution.

Suggested solution to localise incorporationTypes is working as expected (I tested it) Because within onyx they are storing keys instead of value, so when language changed it will be able to maintain selected option.

Here is screen record for localise incorporationTypes solution:

https://user-images.githubusercontent.com/7823358/138926771-1e6d8b7a-11d6-4928-9c9a-04f9d1665bf6.mp4

Please let me know if there is a separate issue to localise incorporationTypes, so I can suggest solution there.

iwiznia commented 3 years ago

Suggested solution to localise incorporationTypes is working as expected (I tested it) Because within onyx they are storing keys instead of value, so when language changed it will be able to maintain selected option.

Oh nice! @PrashantMangukiya I think we will need a migration of the already saved data (with values) to the new data (with keys), right? Otherwise users that already set it would have it misconfigured?

What are the spanish words for "Routing Number" and "Account Number"?

@shawnborton Good question, I think it would be Número de ruta and Número de cuenta (they should be capitalized as written and ideally the ones in english too).

PrashantMangukiya commented 3 years ago

Oh nice! @PrashantMangukiya I think we will need a migration of the already saved data (with values) to the new data (with keys), right? Otherwise users that already set it would have it misconfigured?

Yes if past data saved with companyType value then it needs to migration. At present VBA flow logic is already using Key to store companyType (I checked within local storage), So not sure any old data generated with value etc.

Also In my solution I suggested translation for dropdown ui element only, so save and other api related logic remain unchanged.

shawnborton commented 3 years ago

@michelle-thompson do you want to grab this one? I think you created the original version for the check image.

timszot commented 3 years ago

@NicMendonca let's hire @PrashantMangukiya for this as I agree with the solution.

NicMendonca commented 3 years ago

@PrashantMangukiya hired! 👋

PrashantMangukiya commented 3 years ago

@NicMendonca Thanks. Accepted offer on Upwork.

As soon as design team will provide Spainsh image as mentioned in https://github.com/Expensify/App/issues/5992#issuecomment-951944756 I will proceed to submit pr.

PrashantMangukiya commented 3 years ago

CC @shawnborton @timszot @iwiznia can someone help me.

To implement image solution we need example-check-image-es.png with Spanish text. So someone from graphics team need to provide me Spanish image. Note: Spanish image should be same Size and Quality of english image used at present, So it looks consistent.

English image already there so we will rename it i.e. example-check-image.png to example-check-image-en.png

I am waiting for example-check-image-es.png Spanish image as mentioned in my proposal here https://github.com/Expensify/App/issues/5992#issuecomment-951944756 But I did not receive Spanish image yet.

If someone can provide me Spanish image example-check-image-es.png then I will proceed to submit pr soon.

Thanks in advance.

iwiznia commented 3 years ago

@shawnborton do you have that image?

mallenexpensify commented 3 years ago

Assigned to @PrashantMangukiya

@shawnborton Can you answer Ioni's question above "Do you have that image?"

michelle-thompson commented 3 years ago
bank details - spanish

Apologies for the delay, here's the Spanish translation!

PrashantMangukiya commented 3 years ago

@michelle-thompson thanks for the Spanish image. I will prepare and submit pr asap.

NicMendonca commented 3 years ago

☝️ PR deployed to staging

botify commented 3 years ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 1.1.14-4 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2021-11-22. :confetti_ball:

NicMendonca commented 2 years ago

@PrashantMangukiya paid!

@Santhosh-Sellavel can you please accept the offer for this job here so I can pay for reporting this issue? Thank you!

Santhosh-Sellavel commented 2 years ago

@NicMendonca Accepted!

NicMendonca commented 2 years ago

@Santhosh-Sellavel thank you! Paid!

Santhosh-Sellavel commented 2 years ago

@NicMendonca Contract is still open, end it when you have time thanks!