[amp-list] finalize amp-list-load-more DIY API + documentation #19519
Conversation
|
This pull request introduces 1 alert when merging 9ec411e into 5e6c958 - view on LGTM.com new alerts:
Comment posted by LGTM.com |
cathyxz
force-pushed
the
feature/amp-list-api
branch
from
9ec411e
to
d0aa98a
Compare
| @@ -296,18 +296,19 @@ This attribute specifies an attribute in the returned data that will give the ur | |||
| ``` | |||
|
|
|||
| ### Additional children of `<amp-list>` | |||
There was a problem hiding this comment.
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
| getLoadMoreEndElement_() { | ||
| if (!this.loadMoreEndElement_) { | ||
| this.loadMoreEndElement_ = childElementByAttr( | ||
| this.element, 'load-more-end'); |
There was a problem hiding this comment.
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.
|
Thank you @cvializ and @aghassemi for the review! Assigning this back to myself for now because I've somehow managed to break everything. I'll ping back here when it's ready for you to TAL. |
cathyxz
force-pushed
the
feature/amp-list-api
branch
from
151f6a4
to
8c1518e
Compare
|
@cvializ @aghassemi PTAL? |
A few changes based on a sync with @aghassemi today:
Only one of these elements should ever be visible at any given time.
retryableattribute forload-more-failed, but @andrewwatterson brought up the point that UX-wise, we might not ever want this item to be inactionable (it should always try to reload the previously failed data on click). I'm struggling to find a situation where that would not be the case.Stacked PRs for templating / positioning coming shortly.