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

[amp-list] finalize amp-list-load-more DIY API + documentation #19519

Merged
merged 10 commits into from
Nov 30, 2018
7 changes: 6 additions & 1 deletion bundles.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,12 @@ exports.extensionBundles = [
options: {hasCss: true},
type: TYPES.MISC,
},
{name: 'amp-list', version: '0.1', type: TYPES.MISC},
{
name: 'amp-list',
version: '0.1',
options: {hasCss: true},
type: TYPES.MISC,
},
{
name: 'amp-live-list',
version: '0.1',
Expand Down
46 changes: 21 additions & 25 deletions css/amp.css
Original file line number Diff line number Diff line change
Expand Up @@ -520,15 +520,6 @@ i-amphtml-scroll-container.amp-active, i-amp-scroll-container.amp-active {
visibility: hidden;
}

/**
* The default amp-hidden changes visibility, which takes up space. In the case
* where amp-list becomes a container via the resizable-children attribute, the
* loader should be fully hidden and not take up space.
*/
amp-list[resizable-children] > .i-amphtml-loading-container.amp-hidden {
display: none !important;
}

.i-amphtml-loader-line {
position: absolute;
top:0;
Expand Down Expand Up @@ -630,22 +621,6 @@ amp-list[resizable-children] > .i-amphtml-loading-container.amp-hidden {
visibility: visible;
}

amp-list[load-more] > [load-more-button] > .i-amphtml-loader {
visibility: hidden;
cathyxz marked this conversation as resolved.
Show resolved Hide resolved
}

amp-list[load-more] > [load-more-loading],
amp-list[load-more] > [load-more-failed],
amp-list[load-more] > [load-more-button]{
visibility: hidden;
}

amp-list[load-more] > [load-more-loading].amp-visible,
amp-list[load-more] > [load-more-failed].amp-visible,
amp-list[load-more] > [load-more-button].amp-visible {
visibility: visible;
}

/* Polyfill for IE and any other browser that don't understand templates. */
template {
display: none !important;
Expand Down Expand Up @@ -814,6 +789,27 @@ amp-accordion > section[expanded] > :last-child {
display: block !important;
}

/* amp-list CSS to avoid FOUC */

/**
* The default amp-hidden changes visibility, which takes up space. In the case
* where amp-list becomes a container via the resizable-children attribute, the
* loader should be fully hidden and not take up space.
*/
amp-list[resizable-children] > .i-amphtml-loading-container.amp-hidden {
cathyxz marked this conversation as resolved.
Show resolved Hide resolved
display: none !important;
}

/* load-more elements should be hidden by default */
amp-list[load-more] > [load-more-loading],
amp-list[load-more] > [load-more-failed],
amp-list[load-more] > [load-more-button],
amp-list[load-more] > [load-more-end],
amp-list[load-more] > [load-more-button] > .i-amphtml-loader
{
visibility: hidden;
}

/**
* amp-story
*/
Expand Down
22 changes: 22 additions & 0 deletions extensions/amp-list/0.1/amp-list.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* Copyright 2018 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

amp-list[load-more] > [load-more-loading].amp-visible,
amp-list[load-more] > [load-more-failed].amp-visible,
amp-list[load-more] > [load-more-button].amp-visible,
amp-list[load-more] > [load-more-end].amp-visible {
visibility: visible;
}
86 changes: 40 additions & 46 deletions extensions/amp-list/0.1/amp-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import * as setDOM from 'set-dom/src/index';
import {ActionTrust} from '../../../src/action-constants';
import {AmpEvents} from '../../../src/amp-events';
import {CSS} from '../../../build/amp-list-0.1.css';
import {Deferred} from '../../../src/utils/promise';
import {Layout, isLayoutSizeDefined} from '../../../src/layout';
import {Pass} from '../../../src/pass';
Expand Down Expand Up @@ -119,8 +120,10 @@ export class AmpList extends AMP.BaseElement {
this.loadMoreLoadingElement_ = null;
/** @private {?Element} */
this.loadMoreFailedElement_ = null;
/** @private {?Element} */
this.loadMoreEndElement_ = null;
/**@private {?UnlistenDef} */
this.unlistenLoadMoreButton_ = null;
this.unlistenLoadMore_ = null;

/** @private {?../../../src/service/position-observer/position-observer-impl.PositionObserver} */
this.positionObserver_ = null;
Expand Down Expand Up @@ -185,6 +188,7 @@ export class AmpList extends AMP.BaseElement {
this.getLoadMoreLoadingOverlay_();
}
this.getLoadMoreFailedElement_();
this.getLoadMoreEndElement_();
}
}

Expand Down Expand Up @@ -376,27 +380,14 @@ export class AmpList extends AMP.BaseElement {
}

return fetch.catch(error => {
if (opt_append) {
this.handleLoadMoreFailed_();
if (opt_append && this.loadMoreFailedElement_) {
this.setLoadMoreFailed_();
} else {
this.showFallback_(error);
}
});
}

/**
* When the fetch fails, we should show the load-more-failed element if
* one exists, otherwise show the load-more-button element that triggers
* a new fetch on click.
* @private
*/
handleLoadMoreFailed_() {
if (this.loadMoreFailedElement_) {
this.setLoadMoreFailed_();
} else {
this.setLoadMoreReload_();
}
}
/**
* Proxies the template rendering to the viewer.
* @param {boolean} refresh
Expand Down Expand Up @@ -692,6 +683,7 @@ export class AmpList extends AMP.BaseElement {
// Done loading, nothing more to load.
if (!this.loadMoreSrc_) {
this.loadMoreButton_.classList.toggle('amp-visible', false);
this.loadMoreEndElement_.classList.toggle('amp-visible', true);
return;
}
const triggerOnScroll = this.element.getAttribute('load-more') === 'auto';
Expand All @@ -701,7 +693,7 @@ export class AmpList extends AMP.BaseElement {
if (this.loadMoreButton_) {
this.mutateElement(() => {
this.loadMoreButton_.classList.toggle('amp-visible', true);
this.unlistenLoadMoreButton_ = listen(this.loadMoreButton_, 'click',
this.unlistenLoadMore_ = listen(this.loadMoreButton_, 'click',
() => this.loadMoreCallback_());
});
}
Expand All @@ -712,21 +704,6 @@ export class AmpList extends AMP.BaseElement {
}
}

/**
* Called when a fetch fails under load-more. Shows the load-more-button
* element and triggers a reloading of the failed src on click.
* @private
*/
setLoadMoreReload_() {
if (this.loadMoreButton_) {
this.mutateElement(() => {
this.loadMoreButton_.classList.toggle('amp-visible', true);
this.unlistenLoadMoreButton_ = listen(this.loadMoreButton_, 'click',
() => this.loadMoreCallback_());
});
}
}

/**
* Called when 3 viewports above bottom of automatic load-more list, or
* manually on clicking the load-more-button element. Sets the amp-list
Expand All @@ -742,14 +719,15 @@ export class AmpList extends AMP.BaseElement {
return this.fetchList_(/* opt_append */ true)
.then(() => {
this.toggleLoadMoreLoading_(false);
if (this.unlistenLoadMoreButton_) {
this.unlistenLoadMoreButton_();
this.unlistenLoadMoreButton_ = null;
if (this.unlistenLoadMore_) {
this.unlistenLoadMore_();
this.unlistenLoadMore_ = null;
}
});
}

/**
* @return {!Element|null}
cathyxz marked this conversation as resolved.
Show resolved Hide resolved
* @private
*/
getLoadMoreLoadingElement_() {
Expand All @@ -761,6 +739,7 @@ export class AmpList extends AMP.BaseElement {
}

/**
* @return {!Element}
* @private
*/
getLoadMoreLoadingOverlay_() {
Expand All @@ -780,19 +759,19 @@ export class AmpList extends AMP.BaseElement {
* @private
*/
toggleLoadMoreLoading_(state) {
if (this.loadMoreLoadingElement_) {
this.mutateElement(() => {
if (state) {
this.loadMoreButton_.classList.toggle('amp-visible', false);
}
this.mutateElement(() => {
// If it's loading, then it's no longer failed
if (this.loadMoreFailedElement_) {
this.loadMoreFailedElement_.classList.toggle('amp-visible', false);
}
if (this.loadMoreLoadingElement_) {
this.loadMoreButton_.classList.toggle('amp-visible', !state);
this.loadMoreLoadingElement_.classList.toggle('amp-visible', state);
});
} else if (this.loadMoreButton_) {
this.mutateElement(() => {
} else if (this.loadMoreButton_) {
cathyxz marked this conversation as resolved.
Show resolved Hide resolved
this.loadMoreButton_.classList.toggle('amp-load-more-loading', state);
this.loadMoreLoadingOverlay_.classList.toggle('amp-active', !state);
});
}
}
});
}

/**
Expand All @@ -807,6 +786,8 @@ export class AmpList extends AMP.BaseElement {
this.mutateElement(() => {
if (this.loadMoreFailedElement_) {
this.loadMoreFailedElement_.classList.toggle('amp-visible', true);
this.unlistenLoadMore_ = listen(this.loadMoreFailedElement_, 'click',
() => this.loadMoreCallback_());
}
if (this.loadMoreButton_) {
this.loadMoreButton_.classList.toggle('amp-visible', false);
Expand All @@ -815,6 +796,7 @@ export class AmpList extends AMP.BaseElement {
}

/**
* @return {!Element|null}
* @private
*/
getLoadMoreFailedElement_() {
Expand All @@ -825,6 +807,18 @@ export class AmpList extends AMP.BaseElement {
return this.loadMoreFailedElement_;
}

/**
* @return {!Element|null}
* @private
*/
getLoadMoreEndElement_() {
if (!this.loadMoreEndElement_) {
this.loadMoreEndElement_ = childElementByAttr(
this.element, 'load-more-end');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rules in amp.css needs to be updated for amp-list[load-more-end] as well.

I like to also make amp.css smaller, we should move the amp-list[load-more] > [load-more-*].amp-visible rules into an amp-list.css files as they can not be trigged until the extension has loaded anyway.

}
return this.loadMoreEndElement_;
}


/**
* @param {boolean} opt_refresh
Expand Down Expand Up @@ -902,5 +896,5 @@ export class AmpList extends AMP.BaseElement {
}

AMP.extension(TAG, '0.1', AMP => {
AMP.registerElement(TAG, AmpList);
AMP.registerElement(TAG, AmpList, CSS);
});
15 changes: 8 additions & 7 deletions extensions/amp-list/amp-list.md
Original file line number Diff line number Diff line change
Expand Up @@ -296,18 +296,19 @@ This attribute specifies an attribute in the returned data that will give the ur
```

### Additional children of `<amp-list>`
Copy link
Contributor Author

@cathyxz cathyxz Nov 29, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's intended that we provide UX defaults for these, so when that happens these elements will no longer be mandatory. However, they are mandatory for now.
/cc @CrystalFaith

`<amp-list>` with the `load-more` attribute expects the following additional child elements:
`<amp-list>` with the `load-more` attribute expects the following additional child elements. All of these elements are templated. (Note that UX defaults for these are on the roadmap, thus it will not always be necessary to include these elements.)

#### load-more-button (mandatory)
An element containing the `load-more-button` attribute. Clicking on this button will trigger a fetch to load more elements from the url contained in the field of the data returned corresponding to the `load-more-bookmark` attribute.
An element containing the `load-more-button` attribute, usually a button. Clicking on this element will trigger a fetch to load more elements from the url contained in the field of the data returned corresponding to the `load-more-bookmark` attribute.

In the case of `load-more="auto"`, or infinite scroll, this button will show up if the user has reached the end of the list but the contents are still loading.
#### load-more-loading (mandatory)
An element containing the `load-more-loading` attribute. This element will be displayed if the user reaches the end of the list and the contents are still loading, or as a result of clicking on the `load-more-button` element (while the new children of the amp-list are still loading).

#### load-more-failed (optional)
An element containing the `load-more-failed` attribute. This element will be displayed at the bottom of the `<amp-list>` if loading failed. If this element is not provided, the `load-more-button` element will be displayed and clicking on it will result in an attempt to re-fetch data from the last (failed) url.
#### load-more-failed (mandatory)
An element containing the `load-more-failed` attribute. This element will be displayed at the bottom of the `<amp-list>` if loading failed. Clicking on this element will trigger a reload of the url that failed.

#### .amp-load-more-loading (css class)
This class is applied to the element with the `load-more-button` attribute while the data is loading. This can be used to tweak the visual appearance of the load-more-button (e.g. show a loader) when the `<amp-list>` is in the middle of loading data.
#### load-more-end (optional)
An element containing the `load-more-end` attribute. This element will be displayed at the bottom of the `<amp-list>` if there are no more items. This element is optional.


##### common attributes
Expand Down
12 changes: 10 additions & 2 deletions test/manual/amp-list/infinite-scroll-1.amp.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
amp-list {
border: 1px solid green;
}
[load-more-button], [load-more-failed], [load-more-loading] {
[load-more-button], [load-more-failed], [load-more-loading], [load-more-end] {
background: gray;
color: #fff;
cursor: pointer;
Expand All @@ -29,9 +29,14 @@
padding: 16px;
}

.amp-load-more-loading {
.amp-load-more-loading, [load-more-loading] {
background: blue;
}

[load-more-end] {
background: red;
}

.story-entry {
padding: 40px;
border: 1px solid gray;
Expand Down Expand Up @@ -82,6 +87,9 @@ <h1>AMP List</h1>
<div load-more-loading>
LOADING
</div>
<div load-more-end>
LOAD END
</div>
</amp-list>

</article>
Expand Down