Skip to content
This repository has been archived by the owner on Mar 14, 2024. It is now read-only.

[i18n] Localize string in Instruction.js #6984

Merged
merged 2 commits into from
Dec 14, 2021
Merged

[i18n] Localize string in Instruction.js #6984

merged 2 commits into from
Dec 14, 2021

Conversation

jecfish
Copy link
Contributor

@jecfish jecfish commented Dec 7, 2021

Fixes #6982

Add an optional locale param in the Instruction shortcode. Can use that like below in any article, especially translated one.

{% Instruction 'devtools-network', 'li', locale %}

Preferably able to access to @this {EleventyPage} object like the components/Aside.js, then we can eliminate this additional locale param. However, I couldn't get that work.

@netlify
Copy link

netlify bot commented Dec 7, 2021

✔️ Deploy Preview for web-dev-staging ready!

🔨 Explore the source changes: bd50355

🔍 Inspect the deploy log: https://app.netlify.com/sites/web-dev-staging/deploys/61af91581a82e500077a3642

😎 Browse the preview: https://deploy-preview-6984--web-dev-staging.netlify.app

@@ -25,7 +27,7 @@ const capitalize = require('../../_filters/capitalize');
* @param {string} listStyle The list style to use. Defaults to 'ul'.
* @return {string} A list of instructions.
*/
module.exports = (type, listStyle = 'ul') => {
module.exports = (type, listStyle = 'ul', locale = defaultLocale) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's avoid changing the way this function is called (eg by adding new parameters). You can use getLocaleFromPath to get locale instead, as shown here: https://github.com/GoogleChrome/web.dev/blob/main/src/site/_includes/components/Aside.js#L35

Please note that for this to work you need to change the notation of the component definition from arrow function for "function() {...}"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh!!! That's the trick. Thanks. Will update it now.

@devnook
Copy link
Contributor

devnook commented Dec 7, 2021

Fantastic, thanks a lot @chybie ! I left one comment everything else looks great.

@jecfish jecfish requested a review from devnook December 7, 2021 16:53
@jecfish
Copy link
Contributor Author

jecfish commented Dec 7, 2021

Updated. PTAL.

@jecfish
Copy link
Contributor Author

jecfish commented Dec 8, 2021

@devnook please help to approve the Percy web changes as well

@devnook devnook added the $-presubmit Add label to run presubmit tests. label Dec 13, 2021
@github-actions github-actions bot removed the $-presubmit Add label to run presubmit tests. label Dec 13, 2021
@devnook devnook merged commit b1de8ed into main Dec 14, 2021
@devnook devnook deleted the issue-6982 branch December 14, 2021 18:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Localize Instruction.js
2 participants