Updating Translate to Manage Pluralization

[brandonhenry]

Details

Fixed Issues

$ #38614
PROPOSAL: #38614 (comment)

Tests

1. Navigate to a chat room or report that contains messages with pluralized text related to the maximum number of participants (e.g., "You've selected the maximum number (2) of participants.").

2. Verify that the pluralized text is displayed correctly based on the count (e.g., "You've selected the maximum number (2) of participants." for count 2, and "You've selected the maximum number (5) of participants." for count 5).

3. Change the app's language to a different supported language (e.g., Spanish) and verify that the pluralized text is correctly translated and displayed based on the count.

4. Navigate to a chat room or report that contains messages with pluralized text related to expense counts (e.g., "2 expenses, 1 scanning").

5. Verify that the pluralized text is displayed correctly based on the count (e.g., "1 expense" for count 1, "2 expenses, 1 scanning" for counts 2 expenses and 1 scanning receipt).

6. Change the app's language to a different supported language (e.g., Spanish) and verify that the pluralized text is correctly translated and displayed based on the counts.

7. Navigate to a workspace settings page that shows the "Selected" count for members (e.g., "2 selected").

8. Verify that the pluralized text is displayed correctly based on the count (e.g., "1 selected" for count 1, "5 selected" for count 5).

9. Change the app's language to a different supported language (e.g., Spanish) and verify that the pluralized text is correctly translated and displayed based on the count.

10. Navigate to a workspace settings page that displays options related to distance rates (e.g., "Delete 2 rates", "Enable 3 rates", "Disable 1 rate", "Are you sure you want to delete these 4 rates?").

11. Verify that the pluralized text is displayed correctly based on the count for each option.

12. Change the app's language to a different supported language (e.g., Spanish) and verify that the pluralized text is correctly translated and displayed based on the count for each option.

• Verify that no errors or warnings related to pluralization appear in the JavaScript console.
• Verify that the pluralized text is displayed correctly based on the count across different contexts (maximum participants, expense counts, member selection, distance rates).

Offline tests

1. Put the device in offline mode or disconnect from the internet.
2. Navigate to chat rooms, reports, and workspace settings pages that contain pluralized text based on the examples mentioned in the "Tests" section.
3. Verify that the pluralized text is still displayed correctly based on the count, even in offline mode.
4. Change the app's language to a different supported language and verify that the pluralized text is still correctly translated and displayed based on the count in offline mode.

• Verify that the pluralized text is displayed correctly based on the count, even in offline mode, across different contexts (maximum participants, expense counts, member selection, distance rates).

QA Steps

1. Navigate to a chat room or report that contains messages with pluralized text.
2. Verify that the pluralized text is displayed correctly based on the count.
3. Test with different counts (e.g., 0, 1, 2, 3, 10) to ensure the correct plural form is used for each case.
4. Change the app's language to each supported language and verify that the pluralized text is correctly translated and displayed based on the count in each language.
5. Verify that no errors or warnings related to pluralization appear in the JavaScript console.
6. Test on different platforms (Android, iOS, web) to ensure consistency in pluralization behavior.

• Verify that the pluralized text is displayed correctly based on the count.
• Verify that the pluralized text is properly cached and retrieved from the cache when revisiting the same screen.
• [ ]

PR Author Checklist

• I linked the correct issue in the ### Fixed Issues section above
• I wrote clear testing steps that cover the changes made in this PR
  • I added steps for local testing in the Tests section
  • I added steps for the expected offline behavior in the Offline steps section
  • I added steps for Staging and/or Production testing in the QA steps section
  • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• I included screenshots or videos for tests on all platforms
• I ran the tests on all platforms & verified they passed on:
  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop
• I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• I followed proper code patterns (see Reviewing the code)
  • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
  • I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
  • I verified that comments were added to code that is not self explanatory
  • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
  • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
  • I verified the JSDocs style guidelines (in STYLE.md) were followed
• If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• I followed the guidelines as stated in the Review Guidelines
• I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
• I verified that if a function's arguments changed that all usages have also been updated correctly
• If any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
• If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
• If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
  • I verified that all the inputs inside a form are aligned with each other.
  • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

[jayeshmangwani]

@brandonhenry When PR will be ready for review, please add some unit tests for the Pluralization

[brandonhenry]

Will do @jayeshmangwani !

[brandonhenry]

Just wanted to leave an update here that I was working through some other PRs so haven't had that much time to put here. All done on those so this one will get my full attention this week

[brandonhenry]

thanks for you patience here team. had a lot of back and forth and with local build issues and other things going on, this one kind of fell behind.

I've refactored this a lot into a place I think looks good. mind taking a look? @jayeshmangwani @iwiznia

[melvin-bot]

@jayeshmangwani Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

[jayeshmangwani]

@brandonhenry Above changes will crash the app when we pass the conciergePlaceholderOptions array to localize. Can you test on your side and let us know if the crash is happening on your side, too?

iOS:

iOS-crash-localize.mov

web:

[jayeshmangwani]

these lines of code crashing after the changes:

App/src/pages/home/report/ReportActionCompose/ReportActionCompose.tsx

Lines 201 to 208 in 0b2f55f

	// Placeholder to display in the chat input.
	const inputPlaceholder = useMemo(() => {
	if (includesConcierge) {
	if (userBlockedFromConcierge) {
	return translate('reportActionCompose.blockedFromConcierge');
	}
	return translate('reportActionCompose.conciergePlaceholderOptions')[conciergePlaceholderRandomIndex];

[jayeshmangwani]

Please update the localize test that includes the pluralized examples

[brandonhenry]

sure thing @jayeshmangwani - Let me check again now, thanks for the callout!

[brandonhenry]

so I did see the issue. fixing this and will have this back before EOD

[brandonhenry]

Still working through issues here

[brandonhenry]

@iwiznia @jayeshmangwani so the reason this was failing is because we for some reason have an array for this specific translation phrase. Every other translation in this file is a string. I think it should always be a string

[brandonhenry]

@jayeshmangwani @iwiznia

Alrighty, updated the function to handle that one off array translation. I also updated it so that we didn't have to alter phraseKey at all. Now, if you want to have multiple forms for a translation, you just return an object of Record<LDMLPluralRule, string> in en.ts and you are good to go.

example:

sayHello: (_count) => ({
          other: 'Say hello!',
          one: 'Say hello!!',
          two: 'Say hello!!!',
}),

we still need to pass in count for all the logic to work its magic under the hood. i don't think liner will like the unused variable 🤔

This works now though, no crashes or weirdness and the tests works

translate('reportActionsView.sayHello', {count: 1});

[brandonhenry]

Made a lot of progress yesterday. Changes pointed out more typing issues. Fixing those up, super close!

[jayeshmangwani]

Thanks for the update

[blazejkustra]

Currently these types don't work as intended:

translate('common.addressLine') // Should error without parameters
translate('common.personalAddress', {invalid: 'test'}) // Should error with parameters

[brandonhenry]

Taking a look now! I made quite a few changes and finally got errors for missing types in en.ts. Cleaned those up and we're just about there

[jayeshmangwani]

@brandonhenry Please let me know if there is anything I can help with here

[iwiznia]

This is taking forever to get done.... @brandonhenry can you take it to the finish line at most next week please?

[brandonhenry]

@iwiznia yes for sure! my apologies, main reason is focusing the smaller PRs i was working on that also seem to be taking forever due to review times...

[brandonhenry]

@iwiznia @jayeshmangwani I'm so sorry 😭 i am living in Houston and we got hit really bad by hurricane beryl. No power since Sunday night. Will keep updated as I also have no service so have to drive out and get it

This is really close to done as I worked on it all weekend hoping to finish before beryl but...

[Osmon11]

OMG this is epic...

[brandonhenry]

@Osmon11 indeed. Power is back online as of today! Cleaning this up today / tomorrow

Cc. @iwiznia @jayeshmangwani

[jayeshmangwani]

Cleaning this up today / tomorrow

@brandonhenry can we please take this PR to the finish line today or tomorrow?

[brandonhenry]

@jayeshmangwani @iwiznia

Edit: I'm requesting that this one be transferred to someone else. In hindisght, I should have requested that as soon as I saw we need to change the typing to make this work. My lack of understanding of the system since this is my first expensify ticket, the 3 weeks of power outage / setback and working a 43+ hour full time job set me up for failure here...

The code is 90% done in my opinion but my lack of understanding the Expensify codebase fully + my lack of time to invest in doing so is holding this one back. My apologies but I'm available if you have any questions about the current work. Thank you for your understanding

[twisterdotcom]

@shubham1206agra is going to take over as contributor here.

[shubham1206agra]

Reference

[shubham1206agra]

@iwiznia Can you close this PR?