From 2a7ecd7e8bd043bb1ad87678b210c216bd123b48 Mon Sep 17 00:00:00 2001
From: Steven Ye <stevenye@google.com>
Date: Wed, 14 Aug 2019 13:57:27 -0700
Subject: [PATCH 1/2] Make tweet id a bindable property

---
 extensions/amp-bind/0.1/bind-validator.js     |  3 +++
 extensions/amp-twitter/0.1/amp-twitter.js     | 22 ++++++++++++++++++-
 .../amp-twitter/0.1/test/test-amp-twitter.js  | 12 ++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/extensions/amp-bind/0.1/bind-validator.js b/extensions/amp-bind/0.1/bind-validator.js
index 2202b1189a93..079667442db1 100644
--- a/extensions/amp-bind/0.1/bind-validator.js
+++ b/extensions/amp-bind/0.1/bind-validator.js
@@ -313,6 +313,9 @@ function createElementRules_() {
       'datetime': null,
       'title': null,
     },
+    'AMP-TWITTER': {
+      'data-tweetid': null,
+    },
     'AMP-VIDEO': {
       'alt': null,
       'attribution': null,
diff --git a/extensions/amp-twitter/0.1/amp-twitter.js b/extensions/amp-twitter/0.1/amp-twitter.js
index 2cd66bfc537a..1dde71064963 100644
--- a/extensions/amp-twitter/0.1/amp-twitter.js
+++ b/extensions/amp-twitter/0.1/amp-twitter.js
@@ -120,15 +120,17 @@ class AmpTwitter extends AMP.BaseElement {
    */
   updateForSuccessState_(height) {
     this.mutateElement(() => {
+      this.toggleLoading(false);
       if (this.userPlaceholder_) {
         this.togglePlaceholder(false);
       }
+
       this./*OK*/ changeHeight(height);
     });
   }
 
   /**
-   * Updates wheen the tweet that failed to load. This uses the fallback
+   * Updates when the tweet failed to load. This uses the fallback
    * provided if available. If not, it uses the user specified placeholder.
    * @private
    */
@@ -137,6 +139,7 @@ class AmpTwitter extends AMP.BaseElement {
     const content = fallback || this.userPlaceholder_;
 
     this.mutateElement(() => {
+      this.toggleLoading(false);
       if (fallback) {
         this.togglePlaceholder(false);
         this.toggleFallback(true);
@@ -148,6 +151,14 @@ class AmpTwitter extends AMP.BaseElement {
     });
   }
 
+  /**
+   * amp-twitter reuses the loading indicator when id changes via bind mutation
+   * @override
+   */
+  isLoadingReused() {
+    return true;
+  }
+
   /** @override */
   createLoaderLogoCallback() {
     const html = htmlFor(this.element);
@@ -182,6 +193,15 @@ class AmpTwitter extends AMP.BaseElement {
     }
     return true;
   }
+
+  /** @override */
+  mutatedAttributesCallback(mutations) {
+    if (this.iframe_ && mutations['data-tweetid'] != null) {
+      this.unlayoutCallback();
+      this.toggleLoading(true, /* opt_force */ true);
+      this.layoutCallback();
+    }
+  }
 }
 
 AMP.extension('amp-twitter', '0.1', AMP => {
diff --git a/extensions/amp-twitter/0.1/test/test-amp-twitter.js b/extensions/amp-twitter/0.1/test/test-amp-twitter.js
index 455bc8198053..ba5bdc0f1b46 100644
--- a/extensions/amp-twitter/0.1/test/test-amp-twitter.js
+++ b/extensions/amp-twitter/0.1/test/test-amp-twitter.js
@@ -128,6 +128,18 @@ describes.realWin(
       });
     });
 
+    it('should replace iframe after tweetid mutation', () => {
+      const newTweetId = '638793490521001985';
+      return getAmpTwitter(tweetId).then(ampTwitter => {
+        const spy = sandbox.spy(ampTwitter.implementation_, 'toggleLoading');
+        const iframe = ampTwitter.querySelector('iframe');
+        ampTwitter.setAttribute('data-tweetid', newTweetId);
+        ampTwitter.mutatedAttributesCallback({'data-tweetid': newTweetId});
+        expect(spy).to.be.calledWith(true);
+        expect(ampTwitter.querySelector('iframe')).to.not.equal(iframe);
+      });
+    });
+
     describe('cleanupTweetId_', () => {
       it('does not affect valid tweet ids', () => {
         const good = '585110598171631616';

From 9e88e6e176b2027bdc55b1db4ed6a916ecae0858 Mon Sep 17 00:00:00 2001
From: Steven Ye <stevenye@google.com>
Date: Wed, 14 Aug 2019 14:37:40 -0700
Subject: [PATCH 2/2] update amp-bind documentation

---
 extensions/amp-bind/amp-bind.md           | 5 +++++
 extensions/amp-twitter/0.1/amp-twitter.js | 1 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/extensions/amp-bind/amp-bind.md b/extensions/amp-bind/amp-bind.md
index 62aa015fbe7a..4293f4458478 100644
--- a/extensions/amp-bind/amp-bind.md
+++ b/extensions/amp-bind/amp-bind.md
@@ -488,6 +488,11 @@ Only binding to the following components and attributes are allowed:
     <td><code>[src]</code></td>
     <td>Fetches JSON from the new URL and merges it into the existing state. <em>Note the following update will ignore <code>&lt;amp-state&gt;</code> elements to prevent cycles.</em></td>
   </tr>
+  <tr>
+    <td><code>&lt;amp-twitter&gt;</code></td>
+    <td><code>[data-tweetid]</code></td>
+    <td>Changes the displayed Tweet.</td>
+  </tr>
   <tr>
     <td><code>&lt;amp-video&gt;</code></td>
     <td><code>[alt]</code><br><code>[attribution]</code><br><code>[controls]</code><br><code>[loop]</code><br><code>[poster]</code><br><code>[preload]</code><br><code>[src]</code></td>
diff --git a/extensions/amp-twitter/0.1/amp-twitter.js b/extensions/amp-twitter/0.1/amp-twitter.js
index 1dde71064963..4aad5a13d67c 100644
--- a/extensions/amp-twitter/0.1/amp-twitter.js
+++ b/extensions/amp-twitter/0.1/amp-twitter.js
@@ -124,7 +124,6 @@ class AmpTwitter extends AMP.BaseElement {
       if (this.userPlaceholder_) {
         this.togglePlaceholder(false);
       }
-
       this./*OK*/ changeHeight(height);
     });
   }