Skip to content

Wrap up translating all strings #1325

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

Merged
merged 7 commits into from
Feb 8, 2025
Merged

Wrap up translating all strings #1325

merged 7 commits into from
Feb 8, 2025

Conversation

PIG208
Copy link
Member

@PIG208 PIG208 commented Feb 5, 2025

Fixes: #277

@PIG208 PIG208 force-pushed the pr-l10n branch 2 times, most recently from 28486b4 to 1d6275a Compare February 5, 2025 00:46
Copy link
Collaborator

@chrisbobbe chrisbobbe left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

@chrisbobbe chrisbobbe requested a review from gnprice February 5, 2025 22:32
@chrisbobbe chrisbobbe added the integration review Added by maintainers when PR may be ready for integration label Feb 5, 2025
Copy link
Member

@gnprice gnprice left a comment

Choose a reason for hiding this comment

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

Thanks! Comments below.

This also doesn't yet complete #277 — there are four more sites that need translations, as found at #277 (comment) , in addition to the sites that need new or updated TODO markers which I've just sent in #1332.

Comment on lines 1590 to 1591
text: "(unimplemented: DOM node type ${htmlNode.nodeType})",
text: zulipLocalizations.errorUnimplementedPlaceholder(htmlNode.nodeType),
Copy link
Member

Choose a reason for hiding this comment

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

I think this one is covered by the TODO comments above, so we can leave it out.

(I'm not sure I've ever seen this case happen, so there's no urgency to ask our translators to translate it before we have a solution for its neighbors. The is dom.Element case above is by far the most common.)

Comment on lines 26 to 32
// This does not need to be translated, as it is just a small fragment
// of text surrounded by a large quantity of English text that isn't
// translated anyway.
return '$licenseFileText\n\nAUTHORS file follows:\n\n$authorsFileText';
Copy link
Member

Choose a reason for hiding this comment

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

Great, this is helpful.

The other key piece of the reasoning is the fact that it wouldn't be easy to translate — if it were, we should translate it rather than try to think through whether this is a corner we can get away with cutting. So, drawing on that chat thread you linked earlier:

Suggested change
// This does not need to be translated, as it is just a small fragment
// of text surrounded by a large quantity of English text that isn't
// translated anyway.
return '$licenseFileText\n\nAUTHORS file follows:\n\n$authorsFileText';
// This does not need to be translated, as it is just a small fragment
// of text surrounded by a large quantity of English text that isn't
// translated anyway.
// (And it would be logistically tricky to translate, as this code is
// called from the `main` function before the [ZulipApp] widget is built,
// let alone has updated [GlobalLocalizations].)
return '$licenseFileText\n\nAUTHORS file follows:\n\n$authorsFileText';

@PIG208
Copy link
Member Author

PIG208 commented Feb 6, 2025

Updated the PR to include the missed strings.

Copy link
Member

@gnprice gnprice left a comment

Choose a reason for hiding this comment

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

Thanks for the revision! Comments below.

Comment on lines 812 to 810
"zulipAppTitle": "Zulip",
"@zulipAppTitle": {
"description": "Title of the application that appears above the task manager's app snapshot when the user presses the \"recent apps\" button on Android. This is not used for iOS."
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"zulipAppTitle": "Zulip",
"@zulipAppTitle": {
"description": "Title of the application that appears above the task manager's app snapshot when the user presses the \"recent apps\" button on Android. This is not used for iOS."
"zulipAppTitle": "Zulip",
"@zulipAppTitle": {
"description": "The name of Zulip. This should be either 'Zulip' or a transliteration."
The title right now is just "Zulip", which on its own should translate to
"Zulip" in other languages. But because this is user-visible, we make it
translatable, as we can always change it to a phrase or anything else.

Yeah, I don't anticipate the title changing. Even if it might, that doesn't make a reason to ask our translators to pick translations for it now. The reason to translate it is that translators for some languages will want to enter translations for it 🙂

In the zulip-mobile repo, try this search:

$ gr '^(?!.*Zulip.*Zulip).*Zulip' static/translations/

to find examples where the string "Zulip" appears just once on a line, in translation files that have the English version and translated version of each string on one line.

In some of those cases, it's because the translators reworded so that a mention of Zulip by name was removed (or added) relative to the English. But you'll see that translators for Arabic, Persian, Sinhala, Serbian, and in one case Gujarati have chosen to transliterate the name into their respective scripts.

Anyway: given that the point is to be just the name "Zulip", I think the context details become unneeded for translators and we can simplify what we ask of them by leaving those out.

Copy link
Member Author

Choose a reason for hiding this comment

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

"Зулип"

Ah, I see.

@@ -155,13 +155,21 @@
"filename": {"type": "String", "example": "file.txt"}
}
},
"filenameAndSizeInMiB": "{filename}: {size} MiB",
"@filenameAndSizeInMiB": {
"description": "Message to display the name of a file and its size on a single line.",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"description": "Message to display the name of a file and its size on a single line.",
"description": "The name of a file, and its size in mebibytes.",

The abbreviation "MiB" may not be recognizable to all translators, so spelling it out gives them a better shot at successfully looking it up.

(That unit's name is one of the things translators will edit — for example in French it'll be "Mio", for "mébioctets".)

Comment on lines 474 to 476
"loginServerUrlHint": "your-org.zulipchat.com",
"@loginServerUrlHint": {
"description": "Hint text in login page for Zulip server URL entry."
Copy link
Member

Choose a reason for hiding this comment

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

… Hmm. I started writing in my head a bit more for this description (the translation should keep the ".zulipchat.com" part), then realized: we actually can't have translators substituting their own translations of the phrase "your org" either.

That's because it's important that an example like this be a name we've reserved so that it won't be some actual Zulip organization.

So I'll drop this commit and instead write a comment saying why this isn't translated.

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting. TIL!

@PIG208
Copy link
Member Author

PIG208 commented Feb 8, 2025

Thanks for the review! I have updated the PR.

PIG208 and others added 5 commits February 7, 2025 19:32
The string is used at the end of the "errorFilesTooLarge" message,
which includes a list of files with its size that are too large.

Signed-off-by: Zixuan James Li <[email protected]>
The message, when used in lib/widgets/compose_box.dart, substitutes
`listMessage` with newline separated lines of filenames with size.
Update the example to match this usage.

Signed-off-by: Zixuan James Li <[email protected]>
to match the labels for the other fields (loginEmailLabel,
loginPasswordLabel, etc.)

This also updates existing translations in other languages to
match.

Signed-off-by: Zixuan James Li <[email protected]>
@gnprice
Copy link
Member

gnprice commented Feb 8, 2025

Thanks for the revision! Merging, after updating a couple of the commit messages to match:

3:  4760127f2 ! 3:  0ac905c2b app: Translate app title
    @@ Metadata
     Author: Zixuan James Li <[email protected]>
     
      ## Commit message ##
    -    app: Translate app title
    -
    -    The title right now is just "Zulip", which on its own should translate to
    -    "Zulip" in other languages. But because this is user-visible, we make it
    -    translatable, as we can always change it to a phrase or anything else.
    -
    -    This string is applicable for Android, not iOS. The description,
    -    adapted from the dartdoc of [WidgetsApp.title], for translator mentions
    -    this fact to describe context in which the string appears.
    -
    -    While this might also appear on other platforms like web, there is no
    -    need to bring this up as we do not currently support them.
    +    app: Translate (well, transliterate) app title
     
         Signed-off-by: Zixuan James Li <[email protected]>
     
4:  31446d55e ! 4:  a08dcda2f compose: Translate message for a filename with size
    @@ Commit message
         The string is used at the end of the "errorFilesTooLarge" message,
         which includes a list of files with its size that are too large.
     
    -    We didn't mention that it is a part of the error message because this
    -    part of the context is not as relevant as that it fits in a single
    -    line.
    -
         Signed-off-by: Zixuan James Li <[email protected]>

@gnprice gnprice merged commit 0417c87 into zulip:main Feb 8, 2025
@PIG208 PIG208 deleted the pr-l10n branch February 8, 2025 04:49
@gnprice
Copy link
Member

gnprice commented Feb 12, 2025

This fixed #277, but didn't cause that issue to be closed because the issue description didn't use the special GitHub syntax for it:
https://github.com/zulip/zulip-mobile/blob/main/docs/style.md#mention-fixed-issue

I've edited the description and will close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration review Added by maintainers when PR may be ready for integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sweep through UI strings to internationalize
3 participants