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
4.03k stars 3.03k forks source link

[$250] Localize onboarding tasks and messages #56361

Open roryabraham opened 1 week ago

roryabraham commented 1 week ago

Problem

When we added the new onboarding flows, it seems like none of the in-product text was localized to support multiple languages. I don't know if this was discussed as an exception due to #urgency, but it's against our guidelines.

Solution

Localize all the onboarding tasks and messages

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021886815983481151203
  • Upwork Job ID: 1886815983481151203
  • Last Price Increase: 2025-02-11
Issue OwnerCurrent Issue Owner: @s77rt
melvin-bot[bot] commented 1 week ago

Job added to Upwork: https://www.upwork.com/jobs/~021886815983481151203

melvin-bot[bot] commented 1 week ago

Triggered auto assignment to @anmurali (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

melvin-bot[bot] commented 1 week ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @eVoloshchak (External)

Shahidullah-Muffakir commented 1 week ago

🚨 Edited by proposal-police: This proposal was edited at 2025-02-05 23:35:50 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Onboarding tasks and messages are not translated

What is the root cause of that problem?

New feature

What changes do you think we should make in order to solve the problem?

  1. Add translations keys for all the titles, description and messages in the en.ts and es.ts files. Here in the onboarding add tasks https://github.com/Expensify/App/blob/beb95b0f07d0fa98bf8c9b90b615418d13ddf426/src/languages/en.ts#L1818 and https://github.com/Expensify/App/blob/beb95b0f07d0fa98bf8c9b90b615418d13ddf426/src/languages/es.ts#L1821 for example for the createWorkspaceTask:

        tasks: {
            createWorkspaceTask: {
                title: 'Create a workspace',
                description: (workspaceSettingsLink) => '*Create a workspace* to track expenses, scan receipts, chat, and more.\n' +
                    '\n' +
                    'Here’s how to create a workspace:\n' +
                    '\n' +
                    '1. Click *Settings*.\n' +
                    '2. Click *Workspaces* > *New workspace*.\n' +
                    '\n' +
                    `*Your new workspace is ready!* [Check it out](${workspaceSettingsLink}).`,
            }
        }
  2. now replace all the titles, description and messages with the newly created translation keys.for example for the createWorkspaceTask: https://github.com/Expensify/App/blob/e36d8123c261144553c5e1f7d9704c3dfd477c81/src/CONST.ts#L105-L118

    const createWorkspaceTask: OnboardingTask = {
    type: 'createWorkspace',
    autoCompleted: true,
    title: Localize.translateLocal('onboarding.tasks.createWorkspaceTask.title'),
    description: ({ workspaceSettingsLink }) => Localize.translateLocal('onboarding.tasks.createWorkspaceTask.description', { workspaceSettingsLink }),
    };

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

NA

What alternative solutions did you explore? (Optional)

NA

https://github.com/user-attachments/assets/449d0af8-b061-4eb1-af63-9929e3745af7

mohitrustagi commented 1 week ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Localize onboarding tasks and messages.

What is the root cause of that problem?

Hardcoded strings for OnboardingTask and OnboardingMessage types in src/CONST.ts file.

What changes do you think we should make in order to solve the problem?

Create text in en.ts and es.ts. Use translate function for the types OnboardingMessage and OnboardingTask defined in src/CONST.ts.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

N/A

What alternative solutions did you explore? (Optional)

N/A

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

melvin-bot[bot] commented 1 week ago

πŸ“£ @mohitrustagi! πŸ“£ Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
mohitrustagi commented 1 week ago

Contributor details Your Expensify account email: mohit.rustagi@live.in Upwork Profile Link: Upwork

melvin-bot[bot] commented 1 week ago

βœ… Contributor details stored successfully. Thank you for contributing to Expensify!

melvin-bot[bot] commented 6 days ago

@eVoloshchak, @anmurali, @roryabraham Eep! 4 days overdue now. Issues have feelings too...

melvin-bot[bot] commented 6 days ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @s77rt (External)

roryabraham commented 6 days ago

@s77rt please review proposals on this issue. Thanks!

s77rt commented 5 days ago

@Shahidullah-Muffakir Thanks for the proposal.

we need to consider the the links like workspaceSettingsLink, adminsRoomLink and others, while adding the translation key, I used # to separate it, so then we can use it in the Task view

I don't see the point in adding this # separator. A translation key must be static (no variables modifiers) and its value can be a function that takes the dynamic parameters e.g.:

https://github.com/Expensify/App/blob/48520ecc0f6389f72c8c136984893c4a99f79fc0/src/languages/en.ts#L3600

s77rt commented 5 days ago

@mohitrustagi Thanks for the proposal but can you please elaborate on what needs to be done after translating the strings. For instance we can't directly use translate/translateLocal because in CONST as the name states we define a constant and its value won't change if the user changes his preferred language.

Shahidullah-Muffakir commented 5 days ago

@Shahidullah-Muffakir Thanks for the proposal.

we need to consider the the links like workspaceSettingsLink, adminsRoomLink and others, while adding the translation key, I used # to separate it, so then we can use it in the Task view

I don't see the point in adding this # separator. A translation key must be static (no variables modifiers) and its value can be a function that takes the dynamic parameters e.g.:

App/src/languages/en.ts

Line 3600 in 48520ec

balanceWillBeSettledOn: ({settlementDate}: SettlementDateParams) => Balance will be settled on ${settlementDate},

Thanks for reviewing the proposal. I understand your point.

My idea is to store the translation key itself in the db and separate any links (like workspaceSettingsLink) using # or another delimiter. Then, while displaying it in the TaskView, we can extract the link from report.description and pass it to the translate function.

Another approach would be to store the description as an object with description and links as properties. However, this could cause issues since report.description is currently typed as a string.

s77rt commented 5 days ago

@Shahidullah-Muffakir We don't need and we shouldn't store any data in the translation key. The translation value is the place for data and for this case the value is most likely to be a function (that returns a string).

Shahidullah-Muffakir commented 5 days ago

@s77rt As we are storing the the task description in the report?.description which is a string, https://github.com/Expensify/App/blob/8aa6de85bc039755d1119fdb30c6a93208d2815d/src/components/ReportActionItem/TaskView.tsx#L136

Now instead of storing the whole task description, I suggest to store only the translation key and link(if available). and while displaying it, extract the translation key and links and pass it to the translate function.

// Step 1: Split the string
const [translationKey, workspaceSettingsLink] = report?.description.split("#");

// Step 2: Translate the description
const translatedDescription = translate(translationKey, { workspaceSettingsLink });

for this case the value is most likely to be a function (that returns a string).

Not sure, how can we store a func there in the report?.description

s77rt commented 5 days ago

@Shahidullah-Muffakir The report. description should be a string. At this point we should have already translated the keys. However this means that if you change the language after creating the task report, the messages won't be re-translated.

Can you check if the task report is returned by the BE at any point or if it's only a local report. Note the report id, logout, login and see if you can still access the task report

Shahidullah-Muffakir commented 5 days ago

@s77rt Yes, each time a user clicks on a task (e.g. 'Create a Workspace'), the OpenReport API is called, just like it is for regular reports. It returns the description we created after the onboarding flow as a string.

Image
melvin-bot[bot] commented 5 days ago

πŸ“£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πŸ’Έ

s77rt commented 5 days ago

Got it! We may just translate the strings once.

cc @roryabraham Would it work if we only translate the strings when creating the tasks? i.e. if your preferred language is Spanish the reports will be in Spanish, however if you switch the language to English the messages will still be in Spanish.

This limitation is due to the report.description being expected to be a plain string that is rendered as is and not a translation key. Technically it's possible to fix that but it will increase complexity and I'm not sure if it's worth it.

Shahidullah-Muffakir commented 5 days ago

Another option is to store both the English and Spanish descriptions together, separated by a delimiter (e.g., ||). Then, when displaying the description, we can split the string and show the appropriate translation based on the user's locale. πŸ˜„

s77rt commented 5 days ago

@Shahidullah-Muffakir We shouldn't go with such workarounds. For instance we can't tell if a delimiter is really a delimiter or part of the text and similarly we can't tell if a string is a transaltion key or the text itself

roryabraham commented 4 days ago

Got it! We may just translate the strings once.

cc @roryabraham Would it work if we only translate the strings when creating the tasks? i.e. if your preferred language is Spanish the reports will be in Spanish, however if you switch the language to English the messages will still be in Spanish.

This limitation is due to the report.description being expected to be a plain string that is rendered as is and not a translation key. Technically it's possible to fix that but it will increase complexity and I'm not sure if it's worth it.

Yes, I think this limitation is ok for now. We don't need to translate a report description after it's been saved in the database.

roryabraham commented 4 days ago

If we really wanted to implement a solution without that limitation, we'd have to update the back-end to add a new field like report.localizedDescriptionKey or something. Or maybe use ChatGPT to translation report.description on the fly in the back-end. Either way, such changes are out-of-scope for this issue

s77rt commented 3 days ago

@Shahidullah-Muffakir Can you please update your proposal based on the latest update (that is the translation is only required when creating the report) https://github.com/Expensify/App/issues/56361#issuecomment-2655636375

Shahidullah-Muffakir commented 3 days ago

@s77rt I have updated the proposal.

s77rt commented 3 days ago

@Shahidullah-Muffakir Thanks for the update. The proposal looks good to me.

πŸŽ€ πŸ‘€ πŸŽ€ C+ reviewed Link to proposal

melvin-bot[bot] commented 3 days ago

Current assignee @roryabraham is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.