✨ [Story localization] Transform usages of localization service to async [part 1]#37947
✨ [Story localization] Transform usages of localization service to async [part 1]#37947mszylkowski merged 25 commits intoampproject:mainfrom
Conversation
| const renderActivateButtonTemplate = () => ( | ||
| <button class="i-amphtml-story-360-activate-button" role="button"> | ||
| <span class="i-amphtml-story-360-activate-text"></span> | ||
| <span |
There was a problem hiding this comment.
The class in this element was only used for localization, so we can remove it and use the attribute directly
| */ | ||
| export function localizeTemplate(template, context) { | ||
| const localizationService = getLocalizationService(context); | ||
| const localizationService = Services.localizationForDoc(context); |
There was a problem hiding this comment.
Need to use this function so it doesn't bundle the localization service with all the components
| vsync.mutatePromise(() => { | ||
| el.textContent = str; | ||
| }) | ||
| template.isConnected |
There was a problem hiding this comment.
We don't need to mutate if the template is not connected because the DOM is only updated when it's connected
| <div class="i-amphtml-story-overlay-text"> | ||
| {localize( | ||
| context, | ||
| <div |
There was a problem hiding this comment.
The class in this element is unnecessary as well
| <button | ||
| class="i-amphtml-story-button-move" | ||
| aria-label={initialState.label && localize(context, initialState.label)} | ||
| i-amphtml-i18n-aria-label={initialState.label} |
There was a problem hiding this comment.
initialState.label is always non-null, since all the buttons have labels. No need to check if it is non-null
|
Hey @gmajoulet! These files were changed: Hey @processprocess, @t0mg! These files were changed: Hey @newmuis! These files were changed: |
|
Warning: disparity between this PR Percy build and its The Percy build for this PR was approved (either manually by a member of the AMP team, or automatically if there were no visual diffs). However, during a continuous integration step we generated another Percy build using the commit on the This is possibly an indication of an issue with this pull request, but could also be the result of flakiness. Please inspect the two builds < This PR's Percy build /
|
Replaces all usages of
localize()helper function:amp-story,interactives,360,share-menu,consent,hint,info-dialog,unsupported-browser,pagination-buttonsMissing replacing usages of
localizationServices.getLocalizedString():story-ads,page-attachment(+draggable-drawer,open-page-attachment,forms),share-menu,shopping. Will work on these next.