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

Add function for translation of UI elements #1057

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

haszi
Copy link
Contributor

@haszi haszi commented Sep 1, 2024

Add a function similar to the one in PhD that can be used to translate words or phrases, and use it to translate UI elements of the doc pages. If translation of a certain text is not defined for a language, the function will use the original English text.

Closes php/phd#145

As noted in this comment (by @derickr ?) the "User Contributed Notes" related parts of the UI may have not been translated on purpose.

Pinging @leonardolara and @cmb69 from the original issue.

Copy link

github-actions bot commented Sep 1, 2024

🚀 Preview for commit 5537e82 can be found at https://web-php-pr-1057.preview.thephp.foundation

@haszi
Copy link
Contributor Author

haszi commented Sep 1, 2024

🚀 Regression report for commit 3ced7a0 is at https://web-php-regression-report-pr-1057.preview.thephp.foundation

I don't quite know what to make of this test failure and visual diff in the above link as both pages seem the same to me. If I understand this correctly, they're off by 40 pixels somewhere. Any pointers on what to do with this would be welcome.

@saundefined
Copy link
Member

I don't quite know what to make of this test failure and visual diff in the above link as both pages seem the same to me. If I understand this correctly, they're off by 40 pixels somewhere. Any pointers on what to do with this would be welcome.

really weird, the .hero-versions block was a bit shorter than in the test sample.
I'll try this case locally, maybe we'll just make the tests less accurate by a few pixels

https://web-php-pr-1057.preview.thephp.foundation/

since the test environment only uses English, maybe make some other language as default to see how it works,
and then revert to English?

@saundefined
Copy link
Member

@haszi regression test fail fixed in 0361d00.
Thanks!

@@ -59,17 +62,19 @@ function manual_notes($notes):void {
$noteCountHtml = "<span class=\"count\">$num_notes note" . ($num_notes == 1 ? '' : 's') . "</span>";

Choose a reason for hiding this comment

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

Maybe
$noteCountHtml = "<span class=\"count\">$num_notes " . ($num_notes == 1 ? $note : $notes) . "</span>";
to allow translation of the number of notes beside the title?

$note = 'note';
$notes = 'notes';

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that would be much better! Appending an "s" works for few languages. However, pluralization is more difficult than distinguishing between singular and plural (@mvorisek can probably confirm for the Czech language), but it might be okay for the currently active languages (and we could still extend).

Copy link

Choose a reason for hiding this comment

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

I can confirm appending s to make words plular is NOT how Czech language works. We change the word ending based on many rules.

Choose a reason for hiding this comment

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

Also for Italian the plurals are not formed by appending s.

Copy link
Member

Choose a reason for hiding this comment

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

Or use the more universal format «notes: N»,

because, for example, in Russian (I guess it's not the only language),
different endings will be for 1, 3, and 5 notes.

Copy link
Member

Choose a reason for hiding this comment

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

because, for example, in Russian (I guess it's not the only language),
different endings will be for 1, 3, and 5 notes.

That's what I was referring to regarding the Czech language. :)

Copy link
Member

Choose a reason for hiding this comment

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

This is why gettext has ngettext(), but maybe using that would be overkill for this issue. If there was interest in doing more widespread internationalization of the PHP website, that could be a way to go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe $noteCountHtml = "<span class=\"count\">$num_notes " . ($num_notes == 1 ? $note : $notes) . "</span>"; to allow translation of the number of notes beside the title?

I think that doing this is a good idea but I would prefer not to do it as part of this PR. The changes in this PR address the simpler issue of translating the static elements of the documentation UI. Looking at the above discussion on the intricacies of translating the number of notes to various languages (and we haven't even heard from the Chinese or Japanese doc maintainers on the subject), I think all that needs a bit more thought put into it and should be split into its own PR.

So my suggestion is for this PR to only address the translation of the static elements of the doc pages and to open a new PR for the dynamic elements. Maybe even open an issue first and ask maintainers of every language repo to discuss how the dynamic elements (the total note count, the number of upvotes/downvotes and how long ago a note was submitted) can be translated to each of their respective languages and open a PR once it is clear how to adequately implement it for each of those languages.

@haszi
Copy link
Contributor Author

haszi commented Sep 14, 2024

If merging this PR with the translation of only the static UI elements is acceptable, should the actual translations be done as part of this PR (as was done with the Russian translation by @saundefined and @mmalferov) or as separate PRs?

@cmb69
Copy link
Member

cmb69 commented Sep 14, 2024

If translation of a certain text is not defined for a language, the function will use the original English text.

In my opinion, this is good enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

User Contributed Notes section title and footer do not follow selected language
7 participants