From 55e618b1224705f19eb9f4219f45786eb63612e6 Mon Sep 17 00:00:00 2001
From: Caroline Liu <10456171+caroqliu@users.noreply.github.com>
Date: Wed, 24 Feb 2021 09:36:24 -0500
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Warn=20for=20amp-list[layout=3Dc?=
 =?UTF-8?q?ontainer]=20without=20placeholder=20(#32769)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Warn instead of assert

* Remove test

* Keep assertion for web

* (empty)
---
 extensions/amp-list/0.1/amp-list.js           | 26 ++++++++++++++-----
 extensions/amp-list/0.1/test/test-amp-list.js | 18 ++++++++++---
 2 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/extensions/amp-list/0.1/amp-list.js b/extensions/amp-list/0.1/amp-list.js
index 44eb0d6e441c..1c7e033c895b 100644
--- a/extensions/amp-list/0.1/amp-list.js
+++ b/extensions/amp-list/0.1/amp-list.js
@@ -194,17 +194,29 @@ export class AmpList extends AMP.BaseElement {
   isLayoutSupported(layout) {
     if (layout === Layout.CONTAINER) {
       const doc = this.element.ownerDocument;
+      const isEmail = doc && isAmp4Email(doc);
+      const hasPlaceholder =
+        this.getPlaceholder() ||
+        (this.element.hasAttribute('diffable') &&
+          this.queryDiffablePlaceholder_());
+      if (isEmail) {
+        if (!hasPlaceholder) {
+          user().warn(
+            TAG,
+            'amp-list[layout=container] should have a placeholder to establish an initial size. ' +
+              'See https://go.amp.dev/c/amp-list/#placeholder-and-fallback. %s',
+            this.element
+          );
+        }
+        return (this.enableManagedResizing_ = true);
+      }
       userAssert(
-        (doc && isAmp4Email(doc)) ||
-          isExperimentOn(this.win, 'amp-list-layout-container'),
+        isExperimentOn(this.win, 'amp-list-layout-container'),
         'Experiment "amp-list-layout-container" is not turned on.'
       );
-      const hasDiffablePlaceholder =
-        this.element.hasAttribute('diffable') &&
-        this.queryDiffablePlaceholder_();
       userAssert(
-        this.getPlaceholder() || hasDiffablePlaceholder,
-        'amp-list[layout=container] requires a placeholder to establish an initial size. ' +
+        hasPlaceholder,
+        'amp-list[layout=container] should have a placeholder to establish an initial size. ' +
           'See https://go.amp.dev/c/amp-list/#placeholder-and-fallback. %s',
         this.element
       );
diff --git a/extensions/amp-list/0.1/test/test-amp-list.js b/extensions/amp-list/0.1/test/test-amp-list.js
index 5cfb997fa78d..8835e1dec09d 100644
--- a/extensions/amp-list/0.1/test/test-amp-list.js
+++ b/extensions/amp-list/0.1/test/test-amp-list.js
@@ -315,11 +315,21 @@ describes.repeated(
 
                 it('should require placeholder', () => {
                   list.getPlaceholder = () => null;
-                  allowConsoleError(() => {
-                    expect(() => list.isLayoutSupported('container')).to.throw(
-                      /amp-list\[layout=container\] requires a placeholder/
+                  if (variant.type === 'experiment') {
+                    allowConsoleError(() => {
+                      expect(() =>
+                        list.isLayoutSupported('container')
+                      ).to.throw(
+                        /amp-list\[layout=container\] should have a placeholder/
+                      );
+                    });
+                  } else {
+                    expect(() =>
+                      list.isLayoutSupported('container')
+                    ).to.not.throw(
+                      /amp-list\[layout=container\] should have a placeholder/
                     );
-                  });
+                  }
                 });
 
                 it('should unlock height for layout=container with successful attemptChangeHeight', () => {