Skip to content

Commit

Permalink
Merge pull request ampproject#1094 from dvoytenko/layout23
Browse files Browse the repository at this point in the history 
Layout calculation refactored
  • Loading branch information
@dvoytenko
dvoytenko committed Dec 9, 2015
2 parents 3b5b353 + 37ad528 commit 051aaea
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 79 deletions.
2 changes: 1 addition & 1 deletion extensions/amp-audio/0.1/test/test-amp-audio.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('amp-audio', () => {

function attachAndRun(attributes, opt_childNodesAttrs) {
const ampAudio = getAmpAudio(attributes, opt_childNodesAttrs);
naturalDimensions_['AMP-AUDIO'] = {width: 300, height: 30};
naturalDimensions_['AMP-AUDIO'] = {width: '300px', height: '30px'};
return iframe.addElement(ampAudio);
}

Expand Down
145 changes: 76 additions & 69 deletions src/custom-element.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,92 +123,99 @@ export function stubElements(win) {
* @param {!AmpElement} element
*/
export function applyLayout_(element) {
let widthAttr = element.getAttribute('width');
let heightAttr = element.getAttribute('height');
const sizesAttr = element.getAttribute('sizes');
const layoutAttr = element.getAttribute('layout');
const widthAttr = element.getAttribute('width');
const heightAttr = element.getAttribute('height');
const sizesAttr = element.getAttribute('sizes');

// Handle elements that do not specify a width/height and are defined to have
// natural browser dimensions.
if ((!layoutAttr || layoutAttr == Layout.FIXED ||
layoutAttr == Layout.FIXED_HEIGHT) &&
(!widthAttr || !heightAttr) && hasNaturalDimensions(element.tagName)) {
// Input layout attributes.
const inputLayout = layoutAttr ? parseLayout(layoutAttr) : null;
assert(inputLayout !== undefined, 'Unknown layout: %s', layoutAttr);
const inputWidth = (widthAttr && widthAttr != 'auto') ?
parseLength(widthAttr) : widthAttr;
assert(inputWidth !== undefined, 'Invalid width value: %s', widthAttr);
const inputHeight = heightAttr ? parseLength(heightAttr) : null;
assert(inputHeight !== undefined, 'Invalid height value: %s', heightAttr);

// Effective layout attributes. These are effectively constants.
let width;
let height;
let layout;

// Calculate effective width and height.
if ((!inputLayout || inputLayout == Layout.FIXED ||
inputLayout == Layout.FIXED_HEIGHT) &&
(!inputWidth || !inputHeight) && hasNaturalDimensions(element.tagName)) {
// Default width and height: handle elements that do not specify a
// width/height and are defined to have natural browser dimensions.
const dimensions = getNaturalDimensions(element.tagName);
if (layoutAttr != Layout.FIXED_HEIGHT) {
widthAttr = widthAttr || dimensions.width;
}
heightAttr = heightAttr || dimensions.height;
width = (inputWidth || inputLayout == Layout.FIXED_HEIGHT) ? inputWidth :
dimensions.width;
height = inputHeight || dimensions.height;
} else {
width = inputWidth;
height = inputHeight;
}

let layout;
if (layoutAttr) {
// TODO(dvoytenko): show error state visually in the dev mode, e.g.
// red background + error message.
layout = parseLayout(layoutAttr.trim());
if (!layout) {
throw new Error('Unknown layout: ' + layoutAttr);
}
} else if (widthAttr || heightAttr) {
if (!widthAttr || widthAttr == 'auto') {
layout = Layout.FIXED_HEIGHT;
} else {
layout = sizesAttr ? Layout.RESPONSIVE : Layout.FIXED;
}
} else {
// Calculate effective layout.
if (inputLayout) {
layout = inputLayout;
} else if (!width && !height) {
layout = Layout.CONTAINER;
} else if (height && (!width || width == 'auto')) {
layout = Layout.FIXED_HEIGHT;
} else if (height && width && sizesAttr) {
layout = Layout.RESPONSIVE;
} else {
layout = Layout.FIXED;
}

// Verify layout attributes.
if (layout == Layout.FIXED || layout == Layout.FIXED_HEIGHT ||
layout == Layout.RESPONSIVE) {
assert(height, 'Expected height to be available: %s', heightAttr);
}
if (layout == Layout.FIXED_HEIGHT) {
assert(!width || width == 'auto',
'Expected width to be either absent or equal "auto" ' +
'for fixed-height layout: %s', widthAttr);
}
if (layout == Layout.FIXED || layout == Layout.RESPONSIVE) {
assert(width && width != 'auto',
'Expected width to be available and not equal to "auto": %s',
widthAttr);
}
if (layout == Layout.RESPONSIVE) {
assert(getLengthUnits(width) == getLengthUnits(height),
'Length units should be the same for width and height: %s, %s',
widthAttr, heightAttr);
}

// Apply UI.
element.classList.add(getLayoutClass(layout));
if (isLayoutSizeDefined(layout)) {
element.classList.add('-amp-layout-size-defined');
}

if (layout == Layout.FIXED || layout == Layout.FIXED_HEIGHT ||
layout == Layout.RESPONSIVE) {
let width = 0;
if (layout == Layout.FIXED_HEIGHT) {
if (widthAttr && widthAttr != 'auto') {
throw new Error('Expected width to be either absent or equal "auto" ' +
'for fixed-height layout: ' + widthAttr);
}
} else {
width = parseLength(widthAttr);
if (!width) {
throw new Error('Expected width to be available and be an ' +
'integer/length value: ' + widthAttr);
}
}
const height = parseLength(heightAttr);
if (!height) {
throw new Error('Expected height to be available and be an ' +
'integer/length value: ' + heightAttr);
}
if (layout == Layout.RESPONSIVE) {
if (getLengthUnits(width) != getLengthUnits(height)) {
throw new Error('Length units should be the same for width ' + width +
' and height ' + height);
}
const sizer = element.ownerDocument.createElement('i-amp-sizer');
sizer.style.display = 'block';
sizer.style.paddingTop =
((getLengthNumeral(height) / getLengthNumeral(width)) * 100) + '%';
element.insertBefore(sizer, element.firstChild);
element.sizerElement_ = sizer;
} else if (layout == Layout.FIXED_HEIGHT) {
element.style.height = height;
} else {
element.style.width = width;
element.style.height = height;
}
if (layout == Layout.NODISPLAY) {
element.style.display = 'none';
} else if (layout == Layout.FIXED) {
element.style.width = width;
element.style.height = height;
} else if (layout == Layout.FIXED_HEIGHT) {
element.style.height = height;
} else if (layout == Layout.RESPONSIVE) {
const sizer = element.ownerDocument.createElement('i-amp-sizer');
sizer.style.display = 'block';
sizer.style.paddingTop =
((getLengthNumeral(height) / getLengthNumeral(width)) * 100) + '%';
element.insertBefore(sizer, element.firstChild);
element.sizerElement_ = sizer;
} else if (layout == Layout.FILL) {
// Do nothing.
} else if (layout == Layout.CONTAINER) {
// Do nothing. Elements themselves will check whether the supplied
// layout value is acceptable. In particular container is only OK
// sometimes.
} else if (layout == Layout.NODISPLAY) {
element.style.display = 'none';
} else {
throw new Error('Unsupported layout value: ' + layout);
}
return layout;
}
Expand Down
15 changes: 8 additions & 7 deletions src/layout.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ let Length;

/**
* @typedef {{
* width: number,
* height: number
* width: string,
* height: string
* }}
*/
let Dimensions;
Expand All @@ -58,12 +58,13 @@ let Dimensions;
* `hasNaturalDimensions` checks for membership in this set.
* `getNaturalDimensions` determines the dimensions for an element in the
* set and caches it.
* @type {!Object<string, Dimensions>}
* @type {!Object<string, ?Dimensions>}
* @private Visible for testing only!
*/
export const naturalDimensions_ = {
'AMP-PIXEL': {width: 1, height: 1},
'AMP-ANALYTICS': {width: 1, height: 1},
'AMP-PIXEL': {width: '1px', height: '1px'},
'AMP-ANALYTICS': {width: '1px', height: '1px'},
// TODO(dvoytenko): audio should have width:auto.
'AMP-AUDIO': null
};

Expand Down Expand Up @@ -224,8 +225,8 @@ export function getNaturalDimensions(tagName) {
temp.style.visibility = 'hidden';
document.body.appendChild(temp);
naturalDimensions_[tagName] = {
width: temp./*OK*/offsetWidth || 1,
height: temp./*OK*/offsetHeight || 1
width: (temp./*OK*/offsetWidth || 1) + 'px',
height: (temp./*OK*/offsetHeight || 1) + 'px'
};
document.body.removeChild(temp);
}
Expand Down
34 changes: 32 additions & 2 deletions test/functional/test-layout.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ describe('Layout', () => {
it('layout=fixed - requires width/height', () => {
div.setAttribute('layout', 'fixed');
expect(() => applyLayout_(div)).to.throw(
/to be available and be an integer/);
/Expected height to be available/);
});


Expand Down Expand Up @@ -196,7 +196,7 @@ describe('Layout', () => {
it('layout=fixed-height - requires height', () => {
div.setAttribute('layout', 'fixed-height');
expect(() => applyLayout_(div)).to.throw(
/to be available and be an integer/);
/Expected height to be available/);
});


Expand Down Expand Up @@ -294,4 +294,34 @@ describe('Layout', () => {
expect(pixel.style.height).to.equal('1px');
expect(pixel.style.width).to.equal('');
});

it('should fail invalid width and height', () => {
const pixel = document.createElement('amp-pixel');

// Everything is good.
pixel.setAttribute('width', '1px');
pixel.setAttribute('height', '1px');
expect(() => {
applyLayout_(pixel);
}).to.not.throw();

// Width=auto is also correct.
pixel.setAttribute('width', 'auto');
expect(() => {
applyLayout_(pixel);
}).to.not.throw();

// Width=X is invalid.
pixel.setAttribute('width', 'X');
expect(() => {
applyLayout_(pixel);
}).to.throw(/Invalid width value/);

// Height=X is invalid.
pixel.setAttribute('height', 'X');
pixel.setAttribute('width', '1px');
expect(() => {
applyLayout_(pixel);
}).to.throw(/Invalid height value/);
});
});

0 comments on commit 051aaea

Please sign in to comment.