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

Wait for lazy loaded elements when using the fullPage option #40

Merged
merged 15 commits into from Aug 10, 2020
Merged

Wait for lazy loaded elements when using the fullPage option #40

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
d81b43a
Wait for lazy loaded elements when using the ullPage option
Nov 13, 2019
d4000ad
Had to use eslint-disable. Had no clue how to fix these issues, altho…
Nov 13, 2019
d61dc4c
Added await proper function for images
Feb 24, 2020
5dec125
Timeout of 30 was too short, pushed to 60
Feb 24, 2020
8662efa
Update index.js
sgtrusty Mar 13, 2020
a940db1
Variable name fix
sgtrusty Mar 13, 2020
c165d3d
Added navigationPromise and using adequate variable names instead.
Mar 13, 2020
4b80210
Updates (#1)
sgtrusty Apr 12, 2020
16eec30
Added testing script
Apr 12, 2020
990ad56
Add provisional changes
sgtrusty Aug 2, 2020
e0ebd01
Update test.js
sgtrusty Aug 2, 2020
5f1aa86
Update test.js
sgtrusty Aug 2, 2020
ebb5c1c
Update test.js
sindresorhus Aug 10, 2020
7ae2147
Update test.js
sindresorhus Aug 10, 2020
46ee934
Merge branch 'master' into master
sindresorhus Aug 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 39 additions & 0 deletions index.js
View file
Open in desktop
Expand Up @@ -125,9 +125,15 @@ const parseCookie = (url, cookie) => {
ret.name = ret.key;
delete ret.key;

if (ret.expires) {
ret.expires = Math.floor(new Date(ret.expires) / 1000);
}

return ret;
};

const imagesHaveLoaded = () => [...document.images].map(element => element.complete);

const captureWebsite = async (input, options) => {
options = {
inputType: 'url',
Expand Down Expand Up @@ -321,6 +327,39 @@ const captureWebsite = async (input, options) => {
await options.beforeScreenshot(page, browser);
}

if (screenshotOptions.fullPage) {
// Get the height of the rendered page
const bodyHandle = await page.$('body');
const bodyBoundingHeight = await bodyHandle.boundingBox();
await bodyHandle.dispose();

// Scroll one viewport at a time, pausing to let content load
const viewportHeight = viewportOptions.height;
let viewportIncrement = 0;
while (viewportIncrement + viewportHeight < bodyBoundingHeight) {
const navigationPromise = page.waitForNavigation({waitUntil: 'networkidle0'});
/* eslint-disable no-await-in-loop */
await page.evaluate(_viewportHeight => {
/* eslint-disable no-undef */
window.scrollBy(0, _viewportHeight);
/* eslint-enable no-undef */
}, viewportHeight);
await navigationPromise;
/* eslint-enable no-await-in-loop */
viewportIncrement += viewportHeight;
}

// Scroll back to top
await page.evaluate(_ => {
/* eslint-disable no-undef */
window.scrollTo(0, 0);
/* eslint-enable no-undef */
});

// Some extra delay to let images load
await page.waitForFunction(imagesHaveLoaded, {timeout: 60});
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think the user would expect it to take up to one minute. If we keep this, it needs to be documented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might also be added as an argument with default value set to timeout 60. Might want to comment on this or implement some other better functionality? Maybe it's also worth looking into alternate functions, such as the waitForNavigation Vasile mentioned earlier.

This

await wait(100);

is not good.

You should wait actually for images and other snippets to load, not giving them 100ms...
The waitForNavigation method waiting for a network idle should be used instead.

In case it might be useful for documentation purposes; I retrieved part of the solution from https://stackoverflow.com/questions/51651830/how-to-wait-for-all-images-to-load-from-page-evaluate-function-in-puppeteer-when

Copy link
Owner

Choose a reason for hiding this comment

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

I would look into something better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

using page.waitForNavigation as per pointed by #40 (review)

}

const buffer = await page.screenshot(screenshotOptions);

await page.close();
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
@@ -1,6 +1,6 @@
{
"name": "capture-website",
"version": "0.8.0",
"version": "0.8.1",
Copy link
Owner

Choose a reason for hiding this comment

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

Don't bump this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dunno why that'd get bumped. Gotcha.

"description": "Capture screenshots of websites",
"license": "MIT",
"repository": "sindresorhus/capture-website",
Expand Down
42 changes: 42 additions & 0 deletions test.js
View file
Open in desktop
Expand Up @@ -156,6 +156,48 @@ test('`fullPage` option', async t => {
t.true(size.height > 100);
});

test('`fullPage` option - lazyloading', async t => {
const server = await createTestServer();

server.get('/', async (request, response) => {
response.end(`
<body>
<div style="display: grid; grid-template-columns: repeat( auto-fill, minmax(150px, 1fr) );" id="grid"></div>
<script>
const numItemsToGenerate = 250; //how many gallery items you want on the screen
const numImagesAvailable = 1100; //how many total images are in the collection you are pulling from
const imageWidth = 150; //your desired image width in pixels
const imageHeight = 150; //desired image height in pixels
const collectionID = 9717149; //the collection ID from the original url
function renderGalleryItem(randomNumber){
fetch('https://source.unsplash.com/collection/'+collectionID+'/'+imageWidth+'x'+imageHeight+'/?sig='+randomNumber)
.then((response)=> {
let galleryItem = document.createElement('div');
galleryItem.classList.add('gallery-item');
galleryItem.innerHTML = '<img class="gallery-image" src="'+response.url+'" alt="gallery image" loading="lazy" />';
document.getElementById('grid').appendChild(galleryItem);
})
}
for(let i=0;i<numItemsToGenerate;i++){
let randomImageIndex = Math.floor(Math.random() * numImagesAvailable);
renderGalleryItem(randomImageIndex);
}
Copy link
Owner

Choose a reason for hiding this comment

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

Can you clean up the code style here? It's a bit messy. Use async/await, template literals, etc.

Copy link
Owner

Choose a reason for hiding this comment

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

Also move the string to the top-level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha. Will learn a few things here and there and proceed as such. Thanks for the feedback

</script>
</body>
`);
});

const size = imageSize(await instance(server.url, {
width: 200,
height: 300,
scaleFactor: 1,
fullPage: true
}));

t.is(size.width, 200);
t.true(size.height > 150);
});

test('`timeout` option', async t => {
const server = await createTestServer();

Expand Down