Skip to content

✨ [Story localization] Use inlined JSONs for localization bundles#37836

Merged
mszylkowski merged 25 commits intoampproject:mainfrom
mszylkowski:stringjson_inlined
Mar 10, 2022
Merged

✨ [Story localization] Use inlined JSONs for localization bundles#37836
mszylkowski merged 25 commits intoampproject:mainfrom
mszylkowski:stringjson_inlined

Conversation

@mszylkowski
Copy link
Copy Markdown
Contributor

@mszylkowski mszylkowski commented Mar 8, 2022

If the localization strings are inlined in the document and have the correct version, then we will want to use them.

This PR allows stories on 60+ languages to be translated correctly from the AMP cache once the translations are inlined.

@amp-owners-bot
Copy link
Copy Markdown

amp-owners-bot bot commented Mar 8, 2022

Hey @gmajoulet, @newmuis! These files were changed:

extensions/amp-story/1.0/amp-story.js
extensions/amp-story/1.0/test/test-amp-story.js

@mszylkowski mszylkowski self-assigned this Mar 8, 2022

// If there are inline strings, register as current document language.
const inlineStrings = this.win.document.querySelector(
'script[amp-strings="amp-story"]'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it still time to use a more specific word than just string? Would be cool to add localization or i18n or anything in the mix

Copy link
Copy Markdown
Contributor Author

@mszylkowski mszylkowski Mar 9, 2022

Choose a reason for hiding this comment

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

We can change it since the transformer wasn't merged AFAIK. Sth like what's below looks good to you?

<script amp-localization="amp-story" i-amphtml-version="123456"></script>

to follow closely

<style amp-extension="amp-story" i-amphtml-version="123456"></style>

cc @erwinmombay for the transformer change.

@mszylkowski mszylkowski requested a review from gmajoulet March 9, 2022 19:21
mszylkowski and others added 2 commits March 10, 2022 13:08
@mszylkowski mszylkowski requested a review from gmajoulet March 10, 2022 18:19
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.

3 participants