Skip to content

Commit

Permalink
🐛 Don't render CTA button when there are no shopping tags (#37114)
Browse files Browse the repository at this point in the history
* added no rendering of shopping tag items when there are no items in the array

* added quick fix for attachmentEl

* added attachment element

* remove extra line

* added additional log

* Added additional check for if the attachment element is an amp story shopping attachment

* do not merge, removed line to fix end to end tests, will revert with next push

* reverting last push

* added UPPERCASE tag name check
  • Loading branch information
jshamble committed Dec 14, 2021
1 parent 92d03d4 commit d2a7450
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 1 deletion.
39 changes: 39 additions & 0 deletions examples/amp-story/amp-story-shopping.html
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,45 @@
</amp-story-grid-layer>
<amp-story-shopping-attachment theme="dark"></amp-story-shopping-attachment>
</amp-story-page>
<amp-story-page id="inline-no-product">
<!--
Example of:
CTA Button not rendering due to no product tags.
-->
<amp-story-shopping-config layout="nodisplay">
<script type="application/json">
{
"items" : []
}
</script>
</amp-story-shopping-config>
<amp-story-grid-layer template="fill">
<amp-img layout="fill" src="/examples/visual-tests/amp-story/img/shopping/bg.png"></amp-img>
</amp-story-grid-layer>
<amp-story-grid-layer template="vertical">
</amp-story-grid-layer>
<amp-story-shopping-attachment></amp-story-shopping-attachment>
</amp-story-page>
<amp-story-page id="remote-with-product">
<!--
Example of:
CTA Button rendering due to remote product tag existing, but no inline.
-->
<amp-story-shopping-config layout="nodisplay" src="/examples/amp-story/shopping/remote.json" >
<script type="application/json">
{
"items" : []
}
</script>
</amp-story-shopping-config>
<amp-story-grid-layer template="fill">
<amp-img layout="fill" src="/examples/visual-tests/amp-story/img/shopping/bg.png"></amp-img>
</amp-story-grid-layer>
<amp-story-grid-layer template="vertical">
<amp-story-shopping-tag data-tag-id="city-pop" ></amp-story-shopping-tag>
</amp-story-grid-layer>
<amp-story-shopping-attachment theme="dark"></amp-story-shopping-attachment>
</amp-story-page>
</amp-story>
</body>
</html>
39 changes: 39 additions & 0 deletions examples/visual-tests/amp-story/amp-story-shopping.html
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,45 @@
</amp-story-grid-layer>
<amp-story-shopping-attachment theme="dark"></amp-story-shopping-attachment>
</amp-story-page>
<amp-story-page id="inline-no-product">
<!--
Example of:
CTA Button not rendering due to no product tags.
-->
<amp-story-shopping-config layout="nodisplay">
<script type="application/json">
{
"items" : []
}
</script>
</amp-story-shopping-config>
<amp-story-grid-layer template="fill">
<amp-img layout="fill" src="/examples/visual-tests/amp-story/img/shopping/bg.png"></amp-img>
</amp-story-grid-layer>
<amp-story-grid-layer template="vertical">
</amp-story-grid-layer>
<amp-story-shopping-attachment></amp-story-shopping-attachment>
</amp-story-page>
<amp-story-page id="remote-with-product">
<!--
Example of:
CTA Button rendering due to remote product tag existing, but no inline.
-->
<amp-story-shopping-config layout="nodisplay" src="/examples/amp-story/shopping/remote.json" >
<script type="application/json">
{
"items" : []
}
</script>
</amp-story-shopping-config>
<amp-story-grid-layer template="fill">
<amp-img layout="fill" src="/examples/visual-tests/amp-story/img/shopping/bg.png"></amp-img>
</amp-story-grid-layer>
<amp-story-grid-layer template="vertical">
<amp-story-shopping-tag data-tag-id="city-pop" ></amp-story-shopping-tag>
</amp-story-grid-layer>
<amp-story-shopping-attachment theme="dark"></amp-story-shopping-attachment>
</amp-story-page>
</amp-story>
</body>
</html>
8 changes: 7 additions & 1 deletion extensions/amp-story/1.0/amp-story-page.js
Original file line number Diff line number Diff line change
Expand Up @@ -1577,7 +1577,13 @@ export class AmpStoryPage extends AMP.BaseElement {
const attachmentEl = this.element.querySelector(
'amp-story-page-attachment, amp-story-page-outlink, amp-story-shopping-attachment'
);
if (!attachmentEl) {

if (
!attachmentEl ||
(attachmentEl.tagName === 'AMP-STORY-SHOPPING-ATTACHMENT' &&
this.element.getElementsByTagName('amp-story-shopping-tag').length ===
0)
) {
return;
}

Expand Down

0 comments on commit d2a7450

Please sign in to comment.