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.

Sign up for GitHub

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

Jump to bottom

Fix the load-more-button does not display bug #21304

Merged
merged 4 commits into from
Mar 7, 2019
Merged

Fix the load-more-button does not display bug #21304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0007e7e
Fix sample
cathyxz Mar 6, 2019
f69c83e
Fix urls after routes refactor
cathyxz Mar 6, 2019
918d5e8
Fix load-more-button doesn't display bug
cathyxz Mar 7, 2019
c7bd551
Fix type check
cathyxz Mar 7, 2019
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
19 changes: 11 additions & 8 deletions extensions/amp-list/0.1/amp-list.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ const TAG = 'amp-list';
resolver:!Function,
rejecter:!Function,
append:boolean,
payload: (?JsonObject|Array<JsonObject>),
}} */
export let RenderItems;

Expand Down Expand Up @@ -700,7 +701,6 @@ export class AmpList extends AMP.BaseElement {
const r = this.element.getResources().getResourceForElement(this.element);
r.resetPendingChangeSize();

// Attempt to resize to fit new rendered contents.
this.attemptToFit_(dev().assertElement(this.container_));
});
}
Expand Down Expand Up @@ -897,17 +897,20 @@ export class AmpList extends AMP.BaseElement {
});
return this.fetchList_(/* opt_append */ true)
.then(() => {
if (this.loadMoreSrc_) {
this.mutateElement(() =>
this.loadMoreService_.toggleLoadMoreLoading(false));
} else {
this.mutateElement(() =>
this.loadMoreService_.setLoadMoreEnded());
}
if (this.unlistenLoadMore_) {
this.unlistenLoadMore_();
this.unlistenLoadMore_ = null;
}
return this.mutateElement(() => {
if (this.loadMoreSrc_) {
this.loadMoreService_.toggleLoadMoreLoading(false);
} else {
this.loadMoreService_.setLoadMoreEnded();
}
});
}).then(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix bug.

// Necessary since load-more elements are toggled in the above block
this.attemptToFit_(dev().assertElement(this.container_));
}).catch(() => {
this.mutateElement(() => this.loadMoreService_.setLoadMoreFailed())
.then(() => {
Expand Down
6 changes: 3 additions & 3 deletions extensions/amp-list/0.1/test-e2e/test-load-more-manual.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

const pageWidth = 800;
const pageWidth = 400;
const pageHeight = 600;

describes.endtoend('AMP list load-more=manual', {
Expand Down Expand Up @@ -58,7 +58,7 @@ describes.endtoend('AMP list load-more=manual', {
await controller.takeScreenshot('screenshots/amp-list-load-more.png');
});

it.skip('should load more items on click', async() => {
it('should load more items on click', async() => {
let listItems = await controller.findElements('.item');
await expect(listItems).to.have.length(2);
const seeMore = await controller.findElement('[load-more-button]');
Expand All @@ -81,7 +81,7 @@ describes.endtoend('AMP list load-more=manual', {
});


it.skip('should show load-more-end when done', async() => {
it('should show load-more-end when done', async() => {
const seeMore = await controller.findElement('[load-more-button]');
await controller.click(seeMore);
await controller.findElement('div.item:nth-child(4)');
Expand Down
4 changes: 2 additions & 2 deletions test/manual/amp-list/infinite-scroll-1.amp.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
"items": 8,
"left": 5,
"latency": 0,
"url": "/infinite-scroll?items=8&left=2"
"url": "/list/infinite-scroll?items=8&left=2"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update routes changes due to #21214.

}
</script>
</amp-state>
Expand Down Expand Up @@ -115,7 +115,7 @@ <h1>AMP List</h1>

<amp-list width="auto" height="400"
layout="fixed-height"
src="/infinite-scroll?items=2&left=2"
src="/list/infinite-scroll?items=2&left=2"
[src]="data.url"
load-more="manual"
load-more-bookmark="next">
Expand Down
2 changes: 1 addition & 1 deletion test/manual/amp-list/load-more-auto.amp.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
<!-- Do not change this without editing the E2E test -->
<amp-list width="auto" height="400"
layout="fixed-height"
src="/infinite-scroll?items=2&left=1"
src="/list/infinite-scroll?items=2&left=1"
[src]="data.url"
load-more="auto"
load-more-bookmark="next">
Expand Down
2 changes: 1 addition & 1 deletion test/manual/amp-list/load-more-manual.amp.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
<!-- Do not change this without editing the E2E test -->
<amp-list width="auto" height="400"
layout="fixed-height"
src="/infinite-scroll?items=2&left=2"
src="/list/infinite-scroll?items=2&left=2"
[src]="data.url"
load-more="manual"
load-more-bookmark="next">
Expand Down