Skip to content

Commit

Permalink
Support disable-inline-width attribute when applying layout sizes (a…
Browse files Browse the repository at this point in the history
…mpproject#27083)

* Do not apply sizes for `disable-inline-width`

* Add documentation for `disable-inline-width`

* Add amp-img test

* Update documentation

* Move attribute check to ternary

* Add attribute to validator
  • Loading branch information
caroqliu authored and twintwox committed Mar 24, 2020
1 parent e13978d commit dcc707f
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 1 deletion.
20 changes: 20 additions & 0 deletions spec/amp-html-layout.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,26 @@ In the following example, if the viewport is wider than `320px`, the image will
</amp-img>
```

### `disable-inline-width`

The `sizes` attribute on its own will set an inline `width` style on the element. When pairing `disable-inline-width` with `sizes`, the AMP element will propagate the value of `sizes` to the element's underlying tag, as with the `img` nested inside an `amp-img`, **without** setting the inline `width` as `sizes` typically does on its own in AMP.

**Example**: Using the `disable-inline-width` attribute

In the following example, the width of the `<amp-img>` element is unaffected, and `sizes` is only used to select one of the sources from the `srcset`.

```html
<amp-img
src="https://acme.org/image1.png"
width="400"
height="300"
layout="responsive"
sizes="(min-width: 320px) 320px, 100vw"
disable-inline-width
>
</amp-img>
```

### `heights`

All AMP elements that support the `responsive` layout, also support the `heights` attribute.
Expand Down
4 changes: 3 additions & 1 deletion src/custom-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,9 @@ function createBaseCustomElementClass(win) {
// Sizes.
if (this.sizeList_ === undefined) {
const sizesAttr = this.getAttribute('sizes');
this.sizeList_ = sizesAttr ? parseSizeList(sizesAttr) : null;
const isDisabled = this.hasAttribute('disable-inline-width');
this.sizeList_ =
!isDisabled && sizesAttr ? parseSizeList(sizesAttr) : null;
}
if (this.sizeList_) {
setStyle(
Expand Down
16 changes: 16 additions & 0 deletions test/unit/test-amp-img.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,22 @@ describes.sandboxed('amp-img', {}, env => {
});
});

it('should propagate srcset and sizes with disable-inline-width', async () => {
const ampImg = await getImg({
src: '/examples/img/sample.jpg',
srcset: SRCSET_STRING,
sizes: '(max-width: 320px) 640px, 100vw',
width: 320,
height: 240,
'disable-inline-width': null,
});
const img = ampImg.querySelector('img');
expect(img.getAttribute('srcset')).to.equal(SRCSET_STRING);
expect(img.getAttribute('sizes')).to.equal(
'(max-width: 320px) 640px, 100vw'
);
});

describe('#fallback on initial load', () => {
let el;
let impl;
Expand Down
7 changes: 7 additions & 0 deletions test/unit/test-custom-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -1164,6 +1164,13 @@ describes.realWin('CustomElement', {amp: true}, env => {
element2.ampdoc_ = env.ampdoc;
});

it('should not apply sizes when "disable-inline-width" is present', () => {
element1.setAttribute('disable-inline-width', null);
element1.setAttribute('sizes', '(min-width: 1px) 200px, 50vw');
element1.applySizesAndMediaQuery();
expect(element1.style.width).not.to.equal('200px');
});

it('should apply media condition', () => {
element1.setAttribute('media', '(min-width: 1px)');
element1.applySizesAndMediaQuery();
Expand Down
1 change: 1 addition & 0 deletions validator/validator-main.protoascii
Original file line number Diff line number Diff line change
Expand Up @@ -5439,6 +5439,7 @@ tags: {
# their values.
attr_lists: {
name: "$AMP_LAYOUT_ATTRS"
attrs: { name: "disable-inline-width" }
attrs: { name: "height" }
attrs: { name: "heights" }
attrs: { name: "layout" }
Expand Down

0 comments on commit dcc707f

Please sign in to comment.