Address code review comments

ampproject · Feb 27, 2019 · 148cb51 · 148cb51
1 parent 7918faa
commit 148cb51
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 18 deletions.
diff --git a/builtins/amp-img.js b/builtins/amp-img.js
@@ -178,18 +178,19 @@ export class AmpImg extends BaseElement {
     const viewportWidth = this.getViewport().getWidth();
 
     const entry = `(max-width: ${viewportWidth}px) ${width}px, `;
-    const ratio = Math.round(width * 100 / viewportWidth);
-
     let defaultSize = width + 'px';
 
     if (this.getLayout() !== Layout.FIXED) {
-      defaultSize = Math.max(ratio, 100) + `vw`;
-        ratio + 'vw' : '100vw';
+      const ratio = Math.round(width * 100 / viewportWidth);
+      defaultSize = Math.max(ratio, 100) + 'vw';
     }
+
     const generatedSizes = entry + defaultSize;
 
     if (this.shouldSetSizes_(width)) {
-      this.img_.setAttribute('sizes', generatedSizes);
+      this.mutateElement(() => {
+        this.img_.setAttribute('sizes', generatedSizes);
+      });
       this.sizesWidth_ = width;
     }
   }

diff --git a/src/custom-element.js b/src/custom-element.js
@@ -659,11 +659,7 @@ function createBaseCustomElementClass(win) {
         const sizesAttr = this.getAttribute('sizes');
         this.sizeList_ = sizesAttr ? parseSizeList(sizesAttr) : null;
       }
-      let shouldSetWidth = !!this.sizeList_;
-      if (isExperimentOn(this.ampdoc_.win, 'amp-img-auto-sizes')) {
-        shouldSetWidth = !!this.sizeList_ && this.tagName !== 'AMP-IMG';
-      }
-      if (shouldSetWidth) {
+      if (this.sizeList_) {
         setStyle(this, 'width', this.sizeList_.select(
             toWin(this.ownerDocument.defaultView)));
       }

diff --git a/test/unit/test-amp-img.js b/test/unit/test-amp-img.js
@@ -477,13 +477,7 @@ describe('amp-img', () => {
       impl.getAmpDoc = function() {
         return iframe;
       };
-      impl.getVsync = function() {
-        return {
-          mutate(fn) {
-            fn();
-          },
-        };
-      };
+      impl.mutateElement = fn => fn();
       impl.getViewport = function() {
         return {
           getWidth: () => windowWidth,

diff --git a/test/unit/test-custom-element.js b/test/unit/test-custom-element.js
@@ -1041,7 +1041,6 @@ describes.realWin('CustomElement', {amp: true}, env => {
         matchMedia.withArgs('(min-width: 1px)').returns({matches: true});
         matchMedia.withArgs('(min-width: 1111111px)').returns(
             {matches: false});
-        element1.ampdoc_ = env.ampdoc;
         element2 = new ElementClass();
         element2.ampdoc_ = env.ampdoc;
       });