-
Notifications
You must be signed in to change notification settings - Fork 217
fix(l10n): Update cms localization to reuse strings #19575
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
Conversation
|
With this change, will it mean that all strings that have the same content will map to a single translation, no matter the context? E.g., if multiple buttons/experiments/entrypoints have "Continue", they will all map to the same translation even if some locale may use a different verb depending on context? |
I believe so yes. Its going from To The ftl id did contain the context of the string, or at least some details on how its used. This approach caused some issues since its alot of duplicate strings. |
|
@bcolsson This new approach would reduce the number of unique strings quite significantly and be much simpler/drier, but I wonder if it might be an issue if different page elements share an FTL id? With the new approach, headings/input label/buttons that have the same English text would share the same FTL id across all customizations/pages/elements. My hunch is that different customizations could safely share strings if the content is identical, but only if they are for the same page + page element. |
Thanks for calling that out and that does make sense. Looking at what already exists in previous extracts that didn't seem like a problem, but considering there is the chance that could potentially be an issue with further iterations perhaps we should take some of this in account (while still aiming to reduce redundant strings to improve velocity / reduce localizer burden). Looking at the content that's previously been extracted, it seems like differentiating on the element (e.g. button, header, subject, etc.) would give us some flexibility to accommodate languages that may need different translations based on whether a header vs. button while still significantly reducing the number of strings. It'd also have the advantage of providing the context in which the string is used. I'd still like to share across different page types to keep the amount of redundant strings lower. Could we perhaps change the approach to a hybrid, something like: |
|
@vpomerleau @bcolsson I've updated this to use |
vpomerleau
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates! This strikes a good balance between reducing duplication and allowing localization flexibility if tweaks are needed for different page elements. Had a look at the example output, and this LGTM. With the component name included in the id, the comments could possibly be omitted now
| } | ||
|
|
||
| /** | ||
| * Sanitize entire FTL content to remove hidden Unicode characters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
polish: mismatch between comment and function
|
|
||
| /** | ||
| * Generate FTL ID with l10nId prefix, component name, field name, and hash | ||
| * Generate FTL ID using only the hash of the string content |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
polish: update comment to align with the change to include component hash + substring is 8 char not 12
Because
This pull request
fxa-<hash of text>Issue that this pull request solves
Closes: https://mozilla-hub.atlassian.net/browse/FXA-12539
Checklist
Notes
Here is an example of the PR created by Github now hhttps://github.com/vbudhram/fxa-cms-l10n/pull/16