Repository navigation
fix(MultiSelector): translate the default count text - #9017
HelloOjasMutreja wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR Analysis Report
The exact-head preview is not available yet. Generated by PR Enrichment workflow | View current CI run |
|
AI review status for this pull request.
|
There was a problem hiding this comment.
Thanks for moving the count text into the catalog; changes are requested before this lands. The English message {count, number} selected makes count a plain number, and the catalog check (check:i18n-catalog) rejects any shipped translation that turns it into a plural, so the plural forms the changeset promises can only come from app overrides: adding your test's own French message to fr-FR.json fails that check. Declaring the source as {count, plural, one {# selected} other {# selected}} keeps the English output identical and lets translators supply plural forms. Please also correct the changeset's "English output is unchanged" (1,200 selections now read 1,200 selected, and untranslated locales use their own digits) and refresh the formatValue @default in the JSDoc.
[Automated review]
| "description": "Polite screen-reader announcement of how many MultiSelector options are selected out of the total, spoken after each toggle. Live-region text, never shown on screen; example: `3 of 12 selected`." | ||
| }, | ||
| "@astryx.multiSelector.triggerCount": { | ||
| "defaultMessage": "{count, number} selected", |
There was a problem hiding this comment.
{count, number} gives count a number contract, so check:i18n-catalog rejects a shipped translation that adds plural forms. Your test's French message fails it in fr-FR.json with argument "count" has incompatible kind: expected number; found plural. A plural source keeps the English output identical and lets translators pluralize:
"defaultMessage": "{count, plural, one {# selected} other {# selected}}",|
Pushed the three changes:
|
There was a problem hiding this comment.
Thanks for the follow-up; changes are still requested before this lands. The latest commit put the plural form on TableSelectionToolbar's @astryx.table.selection.selectedCount (line 831 of en.json) rather than on @astryx.multiSelector.triggerCount (line 1647), which is still {count, number} selected, so shipped MultiSelector translations still cannot add plural forms and Table translations must now be plural. Please restore {count, number} selected on the Table key and declare triggerCount as {count, plural, one {# selected} other {# selected}}; the changeset's plural sentence is then accurate. The formatValue JSDoc and the changeset's number-formatting note are already fixed.
[Automated review]
| }, | ||
| "@astryx.table.selection.selectedCount": { | ||
| "defaultMessage": "{count, number} selected", | ||
| "defaultMessage": "{count, plural, one {# selected} other {# selected}}", |
There was a problem hiding this comment.
This rewrites TableSelectionToolbar's selectedCount, not the MultiSelector message: @astryx.multiSelector.triggerCount (line 1647) is still {count, number} selected, so a French plural translation of it still fails check:i18n-catalog, while a number-shaped Table translation now fails instead. Restore {count, number} selected here and put the plural on line 1647:
"defaultMessage": "{count, plural, one {# selected} other {# selected}}",|
Closing as a duplicate: #9020 landed the same MultiSelector count text fix on main. |
Fixes #8980.
With
triggerDisplay="count"and noformatValue, the trigger text was the literal template${n} selected, so it read "2 selected" under every locale and had no plural forms. The component's other strings, including the selection-count announcement, already go throught().The default text now comes from a new message,
@astryx.multiSelector.triggerCount({count, number} selected), withcountas a parameter so a locale can add plural forms.formatValuestill replaces it, and English output is unchanged. Onlyen.jsongains the key; other locales fall back to English until translated, as for any new message, and the catalog check passes.Tests: a new case renders under a French override with a plural message and expects "2 sélectionnés"; it fails against the old literal. Typecheck, eslint,
check:i18n-catalogand the changeset coverage check pass locally. One MultiSelector test, "uses a bottom sheet and stays open while toggling options", fails locally with and without this change.This is a text change, so there is no before/after image.