Skip to content
This repository has been archived by the owner on Jun 21, 2022. It is now read-only.

Localization of WebExtensions Sidebar #755

Merged
merged 3 commits into from Aug 12, 2018
Merged

Localization of WebExtensions Sidebar #755

merged 3 commits into from Aug 12, 2018

Conversation

etiennewan
Copy link
Contributor

No description provided.

@etiennewan
Copy link
Contributor Author

etiennewan commented Jul 31, 2018

👋
I didn't finish to translate some entries, sorry for the inconvenience, but I needed to push this ASAP.

@wbamberg wbamberg self-requested a review August 5, 2018 07:12
@SphinxKnight
Copy link
Member

@etiennewan ping me whenever you are done :) Thanks for the PR

@SphinxKnight SphinxKnight changed the title Localization of WebExtensions Sidebar [WIP] Localization of WebExtensions Sidebar Aug 7, 2018
@etiennewan
Copy link
Contributor Author

@SphinxKnight I'm done translating, I did a quick review to remove or rename some duplicate keys. If some are still there, it's time to find them ! 🙂

Copy link

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

This looks great, thank you!

I had a few comments. I'm trusting you and @SphinxKnight that the translations are good :).

@@ -12,145 +12,320 @@ function currentPageIsUnder(root) {
return "closed";
}

var text = mdn.localStringMap({
'en-US': {

Choose a reason for hiding this comment

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

We are horribly inconsistent throughout this code in single versus double quotes, but we should at least try to be consistent within each macro.

I think single quotes is better, so would you mind fixing up lines 1-13 of this file to use single quotes?

I see that we also use double quotes for HTML attributes. Perhaps we should leave those ones, that seems to be what we use on MDN.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, and I agree, it is better to have consistency within a file.

'Forums': 'Add-on forums',
'#Contact_us': 'Contact us'
},
'fr-FR': {

Choose a reason for hiding this comment

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

I know almost nothing about the way Kuma handles locales, but I think this needs to be just "fr".

If I try your macro, on, say, "http://llocalhost:8000/fr-FR/docs/Web/CSS", then I get redirected to "http://localhost:8000/fr/docs/Web/CSS" and see the en-US strings (I guess this is used as the fallback.

If I change line 102 to just "fr" then I see the French strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed 👍

'Forums': 'Forum extensions',
'#Contact_us': 'Nous contacter'
}
})

Choose a reason for hiding this comment

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

End with a semicolon please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's blame the linter for that 🙂 fixed 👍

Copy link

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

Thanks @etiennewan , this works great.

I'll ask @SphinxKnight for a review, if he's happy with the strings then I'm happy for this to be merged.

@etiennewan
Copy link
Contributor Author

etiennewan commented Aug 11, 2018

Fixed some typos
The job failed during the make build-kumascript, we are currently investigating it on mozilla irc channel #ci
EDIT: retriggered the job and it went fine

Copy link
Member

@SphinxKnight SphinxKnight left a comment

Choose a reason for hiding this comment

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

OK with me regarding the localization. Great job @etiennewan :)

@SphinxKnight SphinxKnight changed the title [WIP] Localization of WebExtensions Sidebar Localization of WebExtensions Sidebar Aug 12, 2018
@SphinxKnight SphinxKnight merged commit 9b7a543 into mdn:master Aug 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants