From 25bb462ae49eab74943da311f1c0c2df7797ab45 Mon Sep 17 00:00:00 2001
From: Micajuine Ho <micajuineho@google.com>
Date: Thu, 25 Feb 2021 12:24:07 -0800
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20[amp-carousel=200.1]=20Fix=20rac?=
 =?UTF-8?q?e=20for=20setting=20slideWidth=5F=20(#32866)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* init

* fix race

* Update extensions/amp-carousel/0.1/slidescroll.js

Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com>

Co-authored-by: Caroline Liu <10456171+caroqliu@users.noreply.github.com>
---
 extensions/amp-carousel/0.1/slidescroll.js          | 10 ++++++++++
 .../amp-carousel/0.1/test/test-slidescroll.js       | 13 +++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/extensions/amp-carousel/0.1/slidescroll.js b/extensions/amp-carousel/0.1/slidescroll.js
index 655e3a1f5888..4baaed721624 100644
--- a/extensions/amp-carousel/0.1/slidescroll.js
+++ b/extensions/amp-carousel/0.1/slidescroll.js
@@ -146,6 +146,9 @@ export class AmpSlideScroll extends BaseSlides {
       ? false
       : !isExperimentOn(this.win, 'amp-carousel-chrome-scroll-snap');
 
+    /** @private {boolean} */
+    this.hasFirstResizedOccured_ = false;
+
     this.onResized_ = this.onResized_.bind(this);
   }
 
@@ -331,6 +334,7 @@ export class AmpSlideScroll extends BaseSlides {
    */
   onResized_(size) {
     this.slideWidth_ = size.width;
+    this.hasFirstResizedOccured_ = true;
   }
 
   /** @override */
@@ -349,6 +353,12 @@ export class AmpSlideScroll extends BaseSlides {
       return Promise.resolve();
     }
 
+    // Account for race when onResized_ has not fired before layoutCallback,
+    // since we need slideWidth_ to proceed.
+    if (!this.hasFirstResizedOccured_) {
+      this.slideWidth_ = this.slidesContainer_./*OK*/ clientWidth;
+    }
+
     if (this.slideIndex_ === null) {
       this.showSlide_(this.initialSlideIndex_);
     } else {
diff --git a/extensions/amp-carousel/0.1/test/test-slidescroll.js b/extensions/amp-carousel/0.1/test/test-slidescroll.js
index 175bdd977254..6799c5298328 100644
--- a/extensions/amp-carousel/0.1/test/test-slidescroll.js
+++ b/extensions/amp-carousel/0.1/test/test-slidescroll.js
@@ -907,6 +907,19 @@ describes.realWin(
         expect(ampSlideScroll.hasAttribute('loop')).to.be.false;
       });
 
+      it('sets the correct scrollLeft for looping carousel', async () => {
+        const ampSlideScroll = await getAmpSlideScroll(true, 7, false, true);
+        doc.body.appendChild(ampSlideScroll);
+        await ampSlideScroll.buildInternal();
+        const impl = await ampSlideScroll.getImpl();
+        ampSlideScroll.layoutCallback();
+        expect(impl.slideWidth_).to.not.be.null;
+        expect(impl.slideWidth_).to.be.greaterThan(0);
+
+        // I.e. the scrollContainer is centered (not at 0)
+        expect(impl.slidesContainer_.scrollLeft).to.equal(impl.slideWidth_);
+      });
+
       // TODO(#17197): This test triggers sinonjs/sinon issues 1709 and 1321.
       it.skip('should hide unwanted slides when looping', async () => {
         const ampSlideScroll = await getAmpSlideScroll(true);