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

i18n: import #12515

Merged
merged 1 commit into from
May 19, 2021
Merged

i18n: import #12515

merged 1 commit into from
May 19, 2021

Conversation

connorjclark
Copy link
Collaborator

No description provided.

@connorjclark connorjclark requested a review from a team as a code owner May 19, 2021 19:29
@connorjclark connorjclark requested review from patrickhulce and removed request for a team May 19, 2021 19:29
@google-cla google-cla bot added the cla: yes label May 19, 2021
@@ -2060,6 +2069,9 @@
"lighthouse-core/report/html/renderer/util.js | runtimeUnknown": {
"message": "غير معروف"
},
"lighthouse-core/report/html/renderer/util.js | showRelevantAudits": {
"message": "Show audits relevant to:"
Copy link
Collaborator Author

@connorjclark connorjclark May 19, 2021

Choose a reason for hiding this comment

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

uhhhh? do we need to mark this as bad?

Copy link
Collaborator

Choose a reason for hiding this comment

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

doesn't look good but I think that setup with that string is already rough for rtl i18n (anything ending with a colon and saying "this stuff here"), maybe that's why it was skipped?

Copy link
Collaborator Author

@connorjclark connorjclark May 19, 2021

Choose a reason for hiding this comment

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

Don't think it was skipped for that reason, or at least others weren't (ca/Catalan is ltr but was skipped).

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah I found all the util.js german ones were just skipped.

@@ -2060,6 +2069,9 @@
"lighthouse-core/report/html/renderer/util.js | runtimeUnknown": {
"message": "Desconegut"
},
"lighthouse-core/report/html/renderer/util.js | showRelevantAudits": {
"message": "Show audits relevant to:"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

same...

prob tons more, should we have a test that the non-en strs don't === the en strs?

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

behavior is the same with the english ones, so doesn't seem to hurt anything at least.

@@ -2087,6 +2099,30 @@
"lighthouse-core/report/html/renderer/util.js | warningHeader": {
"message": "Warnungen: "
},
"lighthouse-treemap/app/src/util.js | allLabel": {
"message": "All"
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems like all of the utils just weren't translated

@brendankenny
Copy link
Member

all strings are filled out by the tc dump, but it uses en strings if a translation isn't available yet

@connorjclark connorjclark merged commit 99c47cd into master May 19, 2021
@connorjclark connorjclark deleted the strs branch May 19, 2021 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants