Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added count param to phrase param and return plural form accordingly #48229

Merged
merged 48 commits into from
Sep 26, 2024
Merged
Changes from 1 commit
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
736b250
update TranslationBase and make few changes
ZhenjaHorbach Aug 28, 2024
9d42d55
fix conflicts
ZhenjaHorbach Aug 28, 2024
e830bb5
update TranslationBase
ZhenjaHorbach Aug 28, 2024
09f51d4
update translations PART-1
ZhenjaHorbach Aug 29, 2024
a237886
update translations PART-2
ZhenjaHorbach Aug 29, 2024
d0bf65b
update translations PART-3
ZhenjaHorbach Aug 29, 2024
41f0334
update translations PART-4
ZhenjaHorbach Aug 29, 2024
078a5ad
Merge branch 'main' into pluralizing_localization-v2
ZhenjaHorbach Aug 31, 2024
b70f5e4
update TranslationBase type
ZhenjaHorbach Aug 31, 2024
f31c7f2
fix conflicts
ZhenjaHorbach Sep 3, 2024
af67cd0
fix conflicts
ZhenjaHorbach Sep 3, 2024
eab923d
fix conflicts
ZhenjaHorbach Sep 5, 2024
5d1a70c
update new translations
ZhenjaHorbach Sep 5, 2024
41a3199
update TranslationBaseValue
ZhenjaHorbach Sep 5, 2024
8ebf751
revert some changes
ZhenjaHorbach Sep 5, 2024
273c8e1
revert some changes
ZhenjaHorbach Sep 5, 2024
d63aa0d
update type arguments
ZhenjaHorbach Sep 5, 2024
4b7eb6d
fix conflicts
ZhenjaHorbach Sep 9, 2024
57d8b7f
fix ts issues
ZhenjaHorbach Sep 9, 2024
83481bf
fix ts issue with no string values
ZhenjaHorbach Sep 11, 2024
554611c
fix conflicts
ZhenjaHorbach Sep 11, 2024
7c24bb4
update types for translations
ZhenjaHorbach Sep 11, 2024
d3028b6
refactor TranslationBase
ZhenjaHorbach Sep 11, 2024
100feae
refactor some code
ZhenjaHorbach Sep 11, 2024
87fd61d
fix conflicts
ZhenjaHorbach Sep 12, 2024
101fd03
refactor types for translations
ZhenjaHorbach Sep 12, 2024
90af317
update types
ZhenjaHorbach Sep 12, 2024
362c264
fix issue with no object values
ZhenjaHorbach Sep 12, 2024
8f58c9e
fix ts issue in localize
ZhenjaHorbach Sep 12, 2024
04c8f76
fix comments
ZhenjaHorbach Sep 12, 2024
bd485f9
fix conflicts
ZhenjaHorbach Sep 13, 2024
5d73e02
fix conflicts
ZhenjaHorbach Sep 14, 2024
0002662
fix conflicts
ZhenjaHorbach Sep 17, 2024
4e0d494
fix conflicts
ZhenjaHorbach Sep 18, 2024
2b84c98
fix conflicts
ZhenjaHorbach Sep 19, 2024
2175302
update package-lock
ZhenjaHorbach Sep 19, 2024
d7f49e5
update package-lock x2
ZhenjaHorbach Sep 19, 2024
090af99
fix comments
ZhenjaHorbach Sep 19, 2024
e278f17
fix conflicts
ZhenjaHorbach Sep 19, 2024
be492c0
fix conflicts
ZhenjaHorbach Sep 20, 2024
94d170b
fix conflicts
ZhenjaHorbach Sep 24, 2024
ffcd48e
update branch
ZhenjaHorbach Sep 25, 2024
480bdda
fix some comments
ZhenjaHorbach Sep 25, 2024
1ebfb54
update readme
ZhenjaHorbach Sep 25, 2024
27b7056
update readme x2
ZhenjaHorbach Sep 25, 2024
7638a4c
update localize comment
ZhenjaHorbach Sep 25, 2024
12b3b41
fix comments
ZhenjaHorbach Sep 26, 2024
a36a304
refactor plural forms
ZhenjaHorbach Sep 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix some comments
ZhenjaHorbach committed Sep 25, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit 480bdda7b2641d8c40946bd14357101b772bbd84
21 changes: 21 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -619,7 +619,28 @@ Some pointers:
key to the translation file and use the arrow function version, like so:
`nameOfTheKey: ({amount, dateTime}) => "User has sent " + amount + " to you on " + dateTime,`.
This is because the order of the phrases might vary from one language to another.
- When working with translations that involve plural forms, it's important to handle different cases correctly.

For example:
- zero: Used when there are no items **(optional)**.
- one: Used when there's exactly one item.
- few: Used for a small number of items **(optional)**.
- many: Used for larger quantities **(optional)**.
- other: A catch-all case for other counts or variations.

Here’s an example of how to implement plural translations:

messages: () => ({
zero: 'No messages',
one: 'One message',
few: (count) => `${count} messages`,
many: (count) => `You have ${count} messages`,
other: (count) => `You have ${count} unread messages`,
})

In your code, you can use the translation like this:

`translate('common.messages', {count: 1});`
----

# Deploying
5 changes: 4 additions & 1 deletion src/languages/en.ts
Original file line number Diff line number Diff line change
@@ -3661,7 +3661,10 @@ const translations = {
rate: 'Rate',
addRate: 'Add rate',
trackTax: 'Track tax',
deleteRates: ({count}: DistanceRateOperationsParams) => `Delete ${Str.pluralize('rate', 'rates', count)}`,
deleteRates: () => ({
one: 'Delete rate',
other: () => `Delete rates`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a function?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it will be easy to implement, and we need to pass the count somehow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But same as the others, you could just have a string here right?

Copy link
Contributor Author

@ZhenjaHorbach ZhenjaHorbach Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes
I also thought about it 5 minutes ago 😅
I will merge changes soon

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it will work!

}),
enableRates: ({count}: DistanceRateOperationsParams) => `Enable ${Str.pluralize('rate', 'rates', count)}`,
disableRates: ({count}: DistanceRateOperationsParams) => `Disable ${Str.pluralize('rate', 'rates', count)}`,
enableRate: 'Enable rate',
5 changes: 4 additions & 1 deletion src/languages/es.ts
Original file line number Diff line number Diff line change
@@ -3711,7 +3711,10 @@ const translations = {
rate: 'Tasa',
addRate: 'Agregar tasa',
trackTax: 'Impuesto de seguimiento',
deleteRates: ({count}: DistanceRateOperationsParams) => `Eliminar ${Str.pluralize('tasa', 'tasas', count)}`,
deleteRates: () => ({
one: 'Eliminar tasa',
other: () => `Eliminar tasas`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

}),
enableRates: ({count}: DistanceRateOperationsParams) => `Activar ${Str.pluralize('tasa', 'tasas', count)}`,
disableRates: ({count}: DistanceRateOperationsParams) => `Desactivar ${Str.pluralize('tasa', 'tasas', count)}`,
enableRate: 'Activar tasa',
12 changes: 6 additions & 6 deletions src/languages/types.ts
Original file line number Diff line number Diff line change
@@ -57,11 +57,11 @@ type FlattenObject<TObject, TPrefix extends string = ''> = {
/**
* Retrieves a type for a given key path (calculated from the type above)
*/
type TranslationValue<TObject, TPath extends string> = TPath extends keyof TObject
? TObject[TPath]
: TPath extends `${infer TKey}.${infer TRest}`
? TKey extends keyof TObject
? TranslationValue<TObject[TKey], TRest>
type TranslationValue<TObject, TKey extends string> = TKey extends keyof TObject
? TObject[TKey]
: TKey extends `${infer TPathKey}.${infer TRest}`
? TPathKey extends keyof TObject
? TranslationValue<TObject[TPathKey], TRest>
: never
: never;

@@ -85,7 +85,7 @@ type FlatTranslationsObject = {
/**
* Determines the expected parameters for a specific translation function based on the provided translation path
*/
type TranslationParameters<TPath extends TranslationPaths> = FlatTranslationsObject[TPath] extends (...args: infer Args) => infer Return
type TranslationParameters<TKey extends TranslationPaths> = FlatTranslationsObject[TKey] extends (...args: infer Args) => infer Return
? Return extends PluralForm
? Args[0] extends undefined
? [PluralParams]
34 changes: 16 additions & 18 deletions src/libs/Localize/index.ts
Original file line number Diff line number Diff line change
@@ -80,37 +80,35 @@ const translationCache = new Map<ValueOf<typeof CONST.LOCALES>, Map<TranslationP
* phrase and stores the translated value in the cache and returns
* the translated value.
*/
function getTranslatedPhrase<TPath extends TranslationPaths>(
function getTranslatedPhrase<TKey extends TranslationPaths>(
language: 'en' | 'es' | 'es-ES',
path: TPath,
phraseKey: TKey,
fallbackLanguage: 'en' | 'es' | null,
...parameters: TranslationParameters<TPath>
...parameters: TranslationParameters<TKey>
): string | null {
// Get the cache for the above locale
const cacheForLocale = translationCache.get(language);

// Directly access and assign the translated value from the cache, instead of
// going through map.has() and map.get() to avoid multiple lookups.
const valueFromCache = cacheForLocale?.get(path);
const valueFromCache = cacheForLocale?.get(phraseKey);

// If the phrase is already translated, return the translated value
if (valueFromCache) {
return valueFromCache;
}

const translatedPhrase = translations?.[language]?.[path];
const translatedPhrase = translations?.[language]?.[phraseKey];

if (translatedPhrase) {
if (typeof translatedPhrase === 'function') {
/**
*
* is Plain object is for checking if the phraseTranslated output
* is an object then further check if it include the count param or not
* OR before checking the plain object output, we can check if we have the count
* param in parameters
*
* If the result of `translatedPhrase` is an object, check if it contains the 'count' property
* to handle pluralization logic.
* Alternatively, before evaluating the translated result, we can check if the 'count' parameter
* exists in the passed parameters.
*/
const translateFunction = translatedPhrase as unknown as (...parameters: TranslationParameters<TPath>) => string | PluralForm;
const translateFunction = translatedPhrase as unknown as (...parameters: TranslationParameters<TKey>) => string | PluralForm;
const translateResult = translateFunction(...parameters);

if (typeof translateResult === 'string') {
@@ -121,7 +119,7 @@ function getTranslatedPhrase<TPath extends TranslationPaths>(
if (phraseObject && typeof phraseObject === 'object' && 'count' in phraseObject && typeof phraseObject.count === 'number') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to check 'count' in phraseObject since if count is not defined, typeof would return undefined and this code would work anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need some additional clarification to prevent Property 'count' does not exist on type 'object'.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we change the type of phraseObject to allow for an optional count?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can do it
But I'm not sure that is a good idea update TranslationParameters type for count
Plus we use such a condition to write count key into phraseObject

Снимок экрана 2024-09-25 в 22 35 02 Снимок экрана 2024-09-25 в 22 34 35

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I'm not sure that is a good idea update TranslationParameters type for count

Why?

Plus we use such a condition to write count key into phraseObject

Which we could remove if we added it as an optional param, no?

const pluralRule = new Intl.PluralRules(language).select(phraseObject.count);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See here


const pluralResult = pluralRule in translateResult && translateResult[pluralRule];
const pluralResult = translateResult[pluralRule];
if (pluralResult) {
if (typeof pluralResult === 'string') {
return pluralResult;
@@ -133,11 +131,11 @@ function getTranslatedPhrase<TPath extends TranslationPaths>(
return translateResult.other(phraseObject.count);
}

throw new Error(`Invalid plural form for '${path}'`);
throw new Error(`Invalid plural form for '${phraseKey}'`);
}

// We set the translated value in the cache only for the phrases without parameters.
cacheForLocale?.set(path, translatedPhrase);
cacheForLocale?.set(phraseKey, translatedPhrase);
return translatedPhrase;
}

@@ -146,18 +144,18 @@ function getTranslatedPhrase<TPath extends TranslationPaths>(
}

// Phrase is not found in full locale, search it in fallback language e.g. es
const fallbackTranslatedPhrase = getTranslatedPhrase(fallbackLanguage, path, null, ...parameters);
const fallbackTranslatedPhrase = getTranslatedPhrase(fallbackLanguage, phraseKey, null, ...parameters);

if (fallbackTranslatedPhrase) {
return fallbackTranslatedPhrase;
}

if (fallbackLanguage !== CONST.LOCALES.DEFAULT) {
Log.alert(`${path} was not found in the ${fallbackLanguage} locale`);
Log.alert(`${phraseKey} was not found in the ${fallbackLanguage} locale`);
}

// Phrase is not translated, search it in default language (en)
return getTranslatedPhrase(CONST.LOCALES.DEFAULT, path, null, ...parameters);
return getTranslatedPhrase(CONST.LOCALES.DEFAULT, phraseKey, null, ...parameters);
}

/**

Unchanged files with check annotations Beta

function getOriginalMessage<T extends ReportActionName>(reportAction: OnyxInputOrEntry<ReportAction<T>>): OriginalMessage<T> | undefined {
if (!Array.isArray(reportAction?.message)) {
return reportAction?.message ?? reportAction?.originalMessage;

Check failure on line 218 in src/libs/ReportActionsUtils.ts

GitHub Actions / Changed files ESLint check

'originalMessage' is deprecated. Used in old report actions before migration. Replaced by using getOriginalMessage function
}
return reportAction.originalMessage;

Check failure on line 220 in src/libs/ReportActionsUtils.ts

GitHub Actions / Changed files ESLint check

'originalMessage' is deprecated. Used in old report actions before migration. Replaced by using getOriginalMessage function
}
function isExportIntegrationAction(reportAction: OnyxInputOrEntry<ReportAction>): boolean {
// HACK ALERT: We're temporarily filtering out any reportActions keyed by sequenceNumber
// to prevent bugs during the migration from sequenceNumber -> reportActionID
if (String(reportAction.sequenceNumber) === key) {

Check failure on line 596 in src/libs/ReportActionsUtils.ts

GitHub Actions / Changed files ESLint check

'sequenceNumber' is deprecated. Used in old report actions before migration. Replaced by reportActionID
Log.info('Front-end filtered out reportAction keyed by sequenceNumber!', false, reportAction);
return true;
}
}
function getCardIssuedMessage(reportAction: OnyxEntry<ReportAction>, shouldRenderHTML = false) {
const assigneeAccountID = (reportAction?.originalMessage as IssueNewCardOriginalMessage)?.assigneeAccountID;

Check failure on line 1701 in src/libs/ReportActionsUtils.ts

GitHub Actions / Changed files ESLint check

'originalMessage' is deprecated. Used in old report actions before migration. Replaced by using getOriginalMessage function
const assigneeDetails = PersonalDetailsUtils.getPersonalDetailsByIDs([assigneeAccountID], currentUserAccountID ?? -1)[0];
const assignee = shouldRenderHTML ? `<mention-user accountID="${assigneeAccountID}"/>` : assigneeDetails?.firstName ?? assigneeDetails.login ?? '';
getNumberOfMoneyRequests,
getOneTransactionThreadReportID,
getOriginalMessage,
getParentReportAction,

Check failure on line 1757 in src/libs/ReportActionsUtils.ts

GitHub Actions / Changed files ESLint check

'getParentReportAction' is deprecated. Use Onyx.connect() or withOnyx() instead

Check failure on line 1757 in src/libs/ReportActionsUtils.ts

GitHub Actions / Changed files ESLint check

'getParentReportAction' is deprecated. Use Onyx.connect() or withOnyx() instead
getRemovedFromApprovalChainMessage,
getReportAction,
getReportActionHtml,
[customField, fieldName, fieldValue, updateRecord],
);
const renderMap: Record<string, JSX.Element> = {

Check failure on line 163 in src/pages/workspace/accounting/netsuite/import/NetSuiteImportCustomFieldEdit.tsx

GitHub Actions / Changed files ESLint check

'JSX' is deprecated. Use `React.JSX` instead of the global `JSX` namespace
mapping: renderSelection,
};