🐛 [amp story shopping] Do not render shopping CTA if no shopping tags…

… are on the page (#37503) * Rename. Do not render CTA. * Update comment * Update test. * Remove unused * revert * revert * install in shopping attachment * check in layoutCallback * Move selectors to buildCallback. * Refert install
ampproject · Feb 2, 2022 · ca1c7d4 · ca1c7d4
1 parent 23e5f09
commit ca1c7d4
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 17 deletions.
diff --git a/extensions/amp-story-shopping/0.1/amp-story-shopping-attachment.js b/extensions/amp-story-shopping/0.1/amp-story-shopping-attachment.js
@@ -33,13 +33,11 @@ export class AmpStoryShoppingAttachment extends AMP.BaseElement {
     /** @private {?Element} */
     this.attachmentEl_ = null;
 
-    /** @private {!Element} */
-    this.pageEl_ = this.element.closest('amp-story-page');
+    /** @private {?Element} */
+    this.pageEl_ = null;
 
-    /** @private {!Array<!Element>} */
-    this.shoppingTags_ = Array.from(
-      this.pageEl_.querySelectorAll('amp-story-shopping-tag')
-    );
+    /** @private {?Array<!Element>} */
+    this.shoppingTags_ = null;
 
     /** @private @const {!Element} */
     this.plpContainer_ = <div></div>;
@@ -53,6 +51,13 @@ export class AmpStoryShoppingAttachment extends AMP.BaseElement {
 
   /** @override */
   buildCallback() {
+    this.pageEl_ = this.element.closest('amp-story-page');
+    this.shoppingTags_ = Array.from(
+      this.pageEl_.querySelectorAll('amp-story-shopping-tag')
+    );
+    if (this.shoppingTags_.length === 0) {
+      return;
+    }
     loadFonts(this.win, FONTS_TO_LOAD);
 
     return Promise.all([
@@ -78,6 +83,9 @@ export class AmpStoryShoppingAttachment extends AMP.BaseElement {
 
   /** @override */
   layoutCallback() {
+    if (this.shoppingTags_.length === 0) {
+      return;
+    }
     this.storeService_.subscribe(
       StateProperty.PAGE_ATTACHMENT_STATE,
       (isOpen) => this.onPageAttachmentStateUpdate_(isOpen)

diff --git a/extensions/amp-story/1.0/amp-story-page.js b/extensions/amp-story/1.0/amp-story-page.js
@@ -520,7 +520,7 @@ export class AmpStoryPage extends AMP.BaseElement {
 
     this.muteAllMedia();
 
-    this.renderOpenAttachmentUI_();
+    this.installPageAttachmentExtension_();
 
     return Promise.all([
       this.waitForMediaLayout_().then(() => this.markPageAsLoaded_()),
@@ -1523,21 +1523,15 @@ export class AmpStoryPage extends AMP.BaseElement {
   }
 
   /**
-   * Renders the open attachment UI affordance.
+   * Installs the page attachment extension.
    * @private
    */
-  renderOpenAttachmentUI_() {
-    // AttachmentEl can be any component that extends draggable drawer.
-    const attachmentEl = this.element.querySelector(
+  installPageAttachmentExtension_() {
+    const elementsThatRequireExtension = this.element.querySelector(
       'amp-story-page-attachment, amp-story-page-outlink, amp-story-shopping-attachment'
     );
 
-    if (
-      !attachmentEl ||
-      (attachmentEl.tagName === 'AMP-STORY-SHOPPING-ATTACHMENT' &&
-        this.element.getElementsByTagName('amp-story-shopping-tag').length ===
-          0)
-    ) {
+    if (!elementsThatRequireExtension) {
       return;
     }