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

Strictly follow prerenderSize given by viewer (or default) during prerendering. #1384

Merged
merged 1 commit into from
Jan 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 10 additions & 6 deletions src/resources.js
Original file line number Diff line number Diff line change
Expand Up @@ -529,8 +529,9 @@ export class Resources {
mutateWork_() {
// Read all necessary data before mutates.
// The height changing depends largely on the target element's position
// in the active viewport. We consider the active viewport the part of the
// visible viewport below 10% from the top and above 25% from the bottom.
// in the active viewport. When not in prerendering, we also consider the
// active viewport the part of the visible viewport below 10% from the top
// and above 25% from the bottom.
// This is basically the portion of the viewport where the reader is most
// likely focused right now. The main goal is to avoid drastic UI changes
// in that part of the content. The elements below the active viewport are
Expand Down Expand Up @@ -736,14 +737,17 @@ export class Resources {
if (this.visible_) {
loadRect = expandLayoutRect(viewportRect, 0.25, 2);
} else if (this.prerenderSize_ > 0) {
loadRect = expandLayoutRect(viewportRect, 0.25,
this.prerenderSize_ - 1 + 0.25);
loadRect = expandLayoutRect(viewportRect, 0, this.prerenderSize_ - 1);
} else {
loadRect = null;
}

// Visible viewport = viewport + 25% up/down.
const visibleRect = expandLayoutRect(viewportRect, 0.25, 0.25);
const visibleRect = this.visible_
// When the doc is visible, consider the viewport to be 25% larger,
// to minimize effect from small scrolling and notify things that
// they are in viewport just before they are actually visible.
? expandLayoutRect(viewportRect, 0.25, 0.25)
: viewportRect;

// Phase 3: Schedule elements for layout within a reasonable distance from
// current viewport.
Expand Down
2 changes: 1 addition & 1 deletion test/functional/test-resources.js
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ describe('Resources discoverWork', () => {
resources.visible_ = false;
resources.prerenderSize_ = 1;
viewportMock.expects('getRect').returns(
layoutRectLtwh(0, 0, 300, 400)).once();
layoutRectLtwh(0, 0, 300, 1009)).once();

resources.discoverWork_();

Expand Down
4 changes: 4 additions & 0 deletions test/manual/amp-img.amp.html
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,9 @@ <h1>amp-image</h1>
<amp-img src="https://lh3.googleusercontent.com/5rcQ32ml8E5ONp9f9-Rf78IofLb9QjS5_0mqsY1zEFc=w400-h300-no-n" width=400 height=300 layout=responsive></amp-img>

<amp-img src="https://lh3.googleusercontent.com/Z4gtm5Bkxyv21Z2PtbTf95Clb9AE4VTR6olbBKYrenM=w400-h300-no-n" width=400 height=300 layout=responsive></amp-img>

<amp-img src="https://lh3.googleusercontent.com/Z4gtm5Bkxyv21Z2PtbTf95Clb9AE4VTR6olbBKYrenM=w400-h300-no-n" width=400 height=300 layout=responsive></amp-img>

<amp-img src="https://lh3.googleusercontent.com/Z4gtm5Bkxyv21Z2PtbTf95Clb9AE4VTR6olbBKYrenM=w400-h300-no-n" width=400 height=300 layout=responsive></amp-img>
</body>
</html>