♻️ ✅ [amp story shopping] Move i18n CTA label logic into shopping-attachment component by processprocess · Pull Request #37515 · ampproject/amphtml (original) (raw)
This PR encapsulates shopping attachment logic by setting the cta-text attribute on the inner page-attachment element and includes a unit test.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
nit: It may be worth playing around and refactoring some of the calls at the beginning of some of the in the unit tests into a helper function to enforce DRY.
Hey @gmajoulet! These files were changed:
extensions/amp-story-page-attachment/0.1/amp-story-open-page-attachment.js
extensions/amp-story-shopping/0.1/amp-story-shopping-attachment.js
extensions/amp-story-shopping/0.1/test/test-amp-story-shopping-attachment.js
Hey @newmuis, @mszylkowski! These files were changed:
extensions/amp-story-page-attachment/0.1/amp-story-open-page-attachment.js
samouri pushed a commit to samouri/amphtml that referenced this pull request
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})