Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support layout=container in amp-list #27159

Merged
merged 39 commits into from
Mar 20, 2020
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
ea2f79c
Prototype for container layout in amp-list.
Feb 20, 2020
16fba4b
Remove unused import
caroqliu Mar 4, 2020
402b7e0
Don't apply fillContent for layout=container
caroqliu Mar 9, 2020
98cf822
Restore old example page
caroqliu Mar 9, 2020
3e887d1
Move layout=container example to new document
caroqliu Mar 9, 2020
fc21ff5
Move isLayoutSizeDefined_ to buildCallback
caroqliu Mar 9, 2020
cea0978
Add line
caroqliu Mar 9, 2020
3ddfa58
Set isLayoutSizeDefined when changing to container
caroqliu Mar 9, 2020
565cdf2
Merge branch 'master' into amp-list-container
caroqliu Mar 9, 2020
8f75c9d
Check layout is defined
caroqliu Mar 9, 2020
52fc541
Replace isLayoutSizeDefined with isLayoutContainer
caroqliu Mar 10, 2020
2fb5c8b
Fix some tests, comment out 4
caroqliu Mar 10, 2020
236f509
Remove bottom padding for sample
caroqliu Mar 11, 2020
041b6da
Move resize logic to attemptToFit
caroqliu Mar 11, 2020
e86c9b8
Set styles on refresh
caroqliu Mar 11, 2020
898fe4e
Uncomment tests
caroqliu Mar 11, 2020
555adfc
Restore line
caroqliu Mar 11, 2020
a45aede
Will comments
caroqliu Mar 11, 2020
cd05420
Handle changeToLayoutContainer action
caroqliu Mar 11, 2020
78651db
Only set height for layout=container
caroqliu Mar 11, 2020
335d145
Validate layout=container
caroqliu Mar 12, 2020
90d78ee
Assert placeholder presence for layout=container
caroqliu Mar 12, 2020
00e900e
Fix indent
caroqliu Mar 13, 2020
ec19be7
Move height locking to helper methods
caroqliu Mar 13, 2020
49c6adf
Unit test for unlock
caroqliu Mar 13, 2020
5a92d89
Merge branch 'master' into amp-list-container
caroqliu Mar 13, 2020
aa98ede
Merge branch 'master' into amp-list-container
caroqliu Mar 16, 2020
b4a80d3
Unlock height inside render
caroqliu Mar 16, 2020
42b5a45
Merge branch 'master' into amp-list-container
caroqliu Mar 17, 2020
04168e5
Move measure to `lockHeightAndMutate`
caroqliu Mar 18, 2020
98d7b0f
Catch errors inside attemptToFit
caroqliu Mar 18, 2020
045bb57
Remove arrow
caroqliu Mar 18, 2020
68e53f1
Always measure height
caroqliu Mar 18, 2020
adc246f
Will comments
caroqliu Mar 18, 2020
8af262c
Add documentation
caroqliu Mar 18, 2020
48c2bc2
Update md language
caroqliu Mar 18, 2020
206e863
Update info link
caroqliu Mar 19, 2020
929f72f
Re-shrink boilerplate code
caroqliu Mar 19, 2020
451b7ae
Rename unlockHeight
caroqliu Mar 19, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 148 additions & 0 deletions examples/amp-list-container.amp.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
<!DOCTYPE html>
<html ⚡>
<head>
<meta charset="utf-8" />
<script async src="https://cdn.ampproject.org/v0.js"></script>
<script
async
custom-element="amp-list"
src="https://cdn.ampproject.org/v0/amp-list-0.1.js"
></script>
<script
async
custom-template="amp-mustache"
src="https://cdn.ampproject.org/v0/amp-mustache-latest.js"
></script>
<link
rel="canonical"
href="https://amp.dev/documentation/examples/components/amp-list/"
/>
<meta
name="viewport"
content="width=device-width,minimum-scale=1,initial-scale=1"
/>
<style amp-boilerplate>
body {
-webkit-animation: -amp-start 8s steps(1, end) 0s 1 normal both;
-moz-animation: -amp-start 8s steps(1, end) 0s 1 normal both;
-ms-animation: -amp-start 8s steps(1, end) 0s 1 normal both;
animation: -amp-start 8s steps(1, end) 0s 1 normal both;
}
@-webkit-keyframes -amp-start {
from {
visibility: hidden;
}
to {
visibility: visible;
}
}
@-moz-keyframes -amp-start {
from {
visibility: hidden;
}
to {
visibility: visible;
}
}
@-ms-keyframes -amp-start {
from {
visibility: hidden;
}
to {
visibility: visible;
}
}
@-o-keyframes -amp-start {
from {
visibility: hidden;
}
to {
visibility: visible;
}
}
@keyframes -amp-start {
from {
visibility: hidden;
}
to {
visibility: visible;
}
}
</style>
<noscript
><style amp-boilerplate>
body {
-webkit-animation: none;
-moz-animation: none;
-ms-animation: none;
animation: none;
}
</style></noscript
>
caroqliu marked this conversation as resolved.
Show resolved Hide resolved
<style amp-custom>
body {
padding: 2em 2em 0em;
}
</style>
</head>
<body>
<button on="tap:list1.refresh">
Refresh List
</button>
<button on="tap:text.toggleVisibility">
Toggle text
</button>
<h3>amp-list with layout="container"</h3>
<amp-list
id="list1"
layout="container"
src="list.example.json"
class="m1"
reset-on-refresh
>
<div placeholder>
<div role="listitem">
<a target="_top" href="http://localhost:8000/components/amp-img/"
>amp-img</a
>
</div>
<div role="listitem">
<a
target="_top"
href="http://localhost:8000/components/amp-accordion/"
>amp-accordion</a
>
</div>
</div>
<template type="amp-mustache" id="amp-template-id">
<div><a href="{{url}}">{{title}}</a></div>
</template>
</amp-list>

<p id="text">
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus vel quam
eu turpis eleifend pretium. Aenean sagittis, leo ac congue mattis, est
nisl accumsan eros, sit amet suscipit massa lectus non nibh. In justo
libero, ultricies sed tristique vel, tincidunt non nulla. Pellentesque vel
pharetra risus. Donec tempus tortor at est vulputate ultrices. Quisque
sagittis pretium facilisis. Aliquam dignissim dapibus nibh sed bibendum.
Phasellus et nulla pulvinar, placerat sapien ac, vehicula mauris. Mauris
efficitur quis orci sed convallis. Maecenas ut nibh placerat, gravida
risus eu, tincidunt diam. Vivamus vitae mauris interdum, iaculis nisi
eget, vulputate lectus. Maecenas condimentum risus eu sapien molestie
sollicitudin. Fusce lobortis purus at fringilla imperdiet. Duis non orci
ultricies, vulputate urna sit amet, euismod velit. Vivamus tempor, nisi
vitae suscipit dignissim, risus lacus imperdiet risus, et mattis leo massa
nec erat. Nulla cursus a leo a dapibus. Nullam dapibus est ut condimentum
iaculis. Sed mollis lorem at sagittis dignissim. Donec euismod urna nec
ligula rhoncus dapibus ullamcorper at erat. Curabitur nec quam vitae nisi
efficitur tincidunt. Aliquam molestie turpis eget risus molestie, non
cursus metus cursus. Nullam diam nisl, tincidunt viverra luctus id, mattis
sed dui. Integer egestas, nunc eu hendrerit tristique, est magna cursus
ex, nec gravida elit magna sit amet neque. Pellentesque luctus leo sed
purus blandit malesuada et eu magna. Praesent molestie dolor diam, at
finibus purus efficitur ullamcorper. Mauris sit amet neque sapien. Nunc
tincidunt velit id nisl eleifend, a imperdiet ligula sagittis.
</p>
</body>
</html>
149 changes: 91 additions & 58 deletions extensions/amp-list/0.1/amp-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,7 @@ import {
markElementForDiffing,
} from '../../../src/purifier/sanitation';
import {Deferred} from '../../../src/utils/promise';
import {
Layout,
getLayoutClass,
isLayoutSizeDefined,
parseLayout,
} from '../../../src/layout';
import {Layout, getLayoutClass, parseLayout} from '../../../src/layout';
import {LoadMoreService} from './service/load-more-service';
import {Pass} from '../../../src/pass';
import {Services} from '../../../src/services';
Expand Down Expand Up @@ -61,7 +56,7 @@ import {
} from '../../../src/utils/xhr-utils';
import {isArray, toArray} from '../../../src/types';
import {isExperimentOn} from '../../../src/experiments';
import {px, setStyles, toggle} from '../../../src/style';
import {px, setImportantStyles, setStyles, toggle} from '../../../src/style';
import {setDOM} from '../../../third_party/set-dom/set-dom';
import {startsWith} from '../../../src/string';

Expand Down Expand Up @@ -130,6 +125,9 @@ export class AmpList extends AMP.BaseElement {
*/
this.layoutCompleted_ = false;

/** @private {boolean} */
this.isLayoutContainer_ = false;

/**
* The `src` attribute's initial value.
* @private {?string}
Expand Down Expand Up @@ -177,7 +175,8 @@ export class AmpList extends AMP.BaseElement {

/** @override */
isLayoutSupported(layout) {
return isLayoutSizeDefined(layout);
this.isLayoutContainer_ = layout === Layout.CONTAINER;
return true;
}

/** @override */
Expand Down Expand Up @@ -426,6 +425,7 @@ export class AmpList extends AMP.BaseElement {

const isLayoutContainer = mutations['is-layout-container'];
if (isLayoutContainer) {
this.isLayoutContainer_ = true;
this.changeToLayoutContainer_();
}

Expand Down Expand Up @@ -455,7 +455,7 @@ export class AmpList extends AMP.BaseElement {
// In the load-more case, we allow the container to be height auto
// in order to reasonably make space for the load-more button and
// load-more related UI elements underneath.
if (!this.loadMoreEnabled_) {
if (!this.loadMoreEnabled_ && !this.isLayoutContainer_) {
caroqliu marked this conversation as resolved.
Show resolved Hide resolved
this.applyFillContent(container, true);
}
return container;
Expand Down Expand Up @@ -514,24 +514,33 @@ export class AmpList extends AMP.BaseElement {
(isFetch && this.element.hasAttribute('reset-on-refresh')) ||
this.element.getAttribute('reset-on-refresh') === 'always'
) {
// Placeholder and loading don't need a mutate context.
this.togglePlaceholder(true);
this.toggleLoading(true, /* opt_force */ true);
this.mutateElement(() => {
this.toggleFallback_(false);
// Clean up bindings in children before removing them from DOM.
if (this.bind_) {
const removed = toArray(this.container_.children);
this.bind_.rescan(/* added */ [], removed, {
'fast': true,
'update': false,
let currentHeight;
this.measureMutateElement(
() => {
currentHeight = this.element./*OK*/ offsetHeight;
caroqliu marked this conversation as resolved.
Show resolved Hide resolved
},
() => {
caroqliu marked this conversation as resolved.
Show resolved Hide resolved
setImportantStyles(this.element, {
'height': `${currentHeight}px`,
'overflow': 'hidden',
});
caroqliu marked this conversation as resolved.
Show resolved Hide resolved
this.togglePlaceholder(true);
this.toggleLoading(true, /* opt_force */ true);
this.toggleFallback_(false);
// Clean up bindings in children before removing them from DOM.
if (this.bind_) {
const removed = toArray(this.container_.children);
this.bind_.rescan(/* added */ [], removed, {
'fast': true,
'update': false,
});
}
removeChildren(dev().assertElement(this.container_));
if (this.loadMoreEnabled_) {
this.getLoadMoreService_().hideAllLoadMoreElements();
}
}
removeChildren(dev().assertElement(this.container_));
if (this.loadMoreEnabled_) {
this.getLoadMoreService_().hideAllLoadMoreElements();
}
});
);
}
}

Expand Down Expand Up @@ -956,35 +965,51 @@ export class AmpList extends AMP.BaseElement {
dev().info(TAG, 'render:', this.element, elements);
const container = dev().assertElement(this.container_);

return this.mutateElement(() => {
this.hideFallbackAndPlaceholder_();

if (this.element.hasAttribute('diffable') && container.hasChildNodes()) {
this.diff_(container, elements);
} else {
if (!opt_append) {
removeChildren(container);
}
this.addElementsToContainer_(elements, container);
}
let currentHeight;
return this.measureMutateElement(
() => {
currentHeight = this.element./*OK*/ offsetHeight;
},
() => {
setImportantStyles(this.element, {
'height': `${currentHeight}px`,
'overflow': 'hidden',
});

const event = createCustomEvent(
this.win,
AmpEvents.DOM_UPDATE,
/* detail */ null,
{bubbles: true}
);
this.container_.dispatchEvent(event);
this.hideFallbackAndPlaceholder_();

// Now that new contents have been rendered, clear pending size requests
// from previous calls to attemptToFit_(). Rejected size requests are
// saved as "pending" and are fulfilled later on 'focus' event.
// See resources-impl.checkPendingChangeSize_().
const r = this.element.getResources().getResourceForElement(this.element);
r.resetPendingChangeSize();
if (
this.element.hasAttribute('diffable') &&
container.hasChildNodes()
) {
this.diff_(container, elements);
} else {
if (!opt_append) {
removeChildren(container);
}
this.addElementsToContainer_(elements, container);
}

this.maybeResizeListToFitItems_();
});
const event = createCustomEvent(
this.win,
AmpEvents.DOM_UPDATE,
/* detail */ null,
{bubbles: true}
);
this.container_.dispatchEvent(event);

// Now that new contents have been rendered, clear pending size requests
// from previous calls to attemptToFit_(). Rejected size requests are
// saved as "pending" and are fulfilled later on 'focus' event.
// See resources-impl.checkPendingChangeSize_().
const r = this.element
.getResources()
.getResourceForElement(this.element);
r.resetPendingChangeSize();

this.maybeResizeListToFitItems_();
}
);
}

/**
Expand Down Expand Up @@ -1092,14 +1117,22 @@ export class AmpList extends AMP.BaseElement {
* @private
*/
attemptToFit_(target) {
if (this.element.getAttribute('layout') == Layout.CONTAINER) {
return;
}
this.measureElement(() => {
const targetHeight = target./*OK*/ scrollHeight;
const height = this.element./*OK*/ offsetHeight;
if (targetHeight > height) {
this.attemptChangeHeight(targetHeight).catch(() => {});
this.attemptChangeHeight(targetHeight).then(
() => {
if (!this.isLayoutContainer_) {
caroqliu marked this conversation as resolved.
Show resolved Hide resolved
return;
}
setImportantStyles(this.element, {
'height': '',
'overflow': '',
});
},
() => {}
caroqliu marked this conversation as resolved.
Show resolved Hide resolved
);
caroqliu marked this conversation as resolved.
Show resolved Hide resolved
}
});
}
Expand All @@ -1110,6 +1143,9 @@ export class AmpList extends AMP.BaseElement {
* @private
*/
attemptToFitLoadMore_(target) {
if (this.isLayoutContainer_) {
return;
}
const element = !!this.loadMoreSrc_
? this.getLoadMoreService_().getLoadMoreButton()
: this.getLoadMoreService_().getLoadMoreEndElement();
Expand All @@ -1122,9 +1158,6 @@ export class AmpList extends AMP.BaseElement {
* @private
*/
attemptToFitLoadMoreElement_(element, target) {
if (this.element.getAttribute('layout') == Layout.CONTAINER) {
return;
}
this.measureElement(() => {
const targetHeight = target./*OK*/ scrollHeight;
const height = this.element./*OK*/ offsetHeight;
Expand Down
Loading