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

Add waitForSelector support #64

Merged
merged 3 commits into from
Jun 22, 2020

Conversation

andmcgregor
Copy link
Contributor

Hi, is there any interest in merging this change? It just adds support for waitForSelector which I needed on a recent project. Open to any feedback or changes needed.

Copy link
Contributor

@abrom abrom left a comment

Choose a reason for hiding this comment

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

Seems like a reasonable change. The waitForSelector API does also support some additional options (optional). Thoughts on including support for them too? I can see they might be useful in the same vein (if you wanted to have it wait until say a 'loading' element was hidden instead).

https://github.com/puppeteer/puppeteer/blob/v4.0.0/docs/api.md#pagewaitforselectorselector-options

Maybe something like:

    // If specified, wait for selector
    const waitForSelector = options.waitForSelector; delete options.waitForSelector;
    const waitForSelectorOptions = options.waitForSelectorOptions; delete options.waitForSelectorOptions;
    if (waitForSelector !== undefined) {
      await page.waitForSelector(waitForSelector, waitForSelectorOptions)
    }

The spec for it would likely just have some sort of "loading" element that gets hidden after a period, then expect that the result does not have the loading content.

Can you also please add an entry in the unreleased section of CHANGELOG.md

@abrom
Copy link
Contributor

abrom commented Jun 22, 2020

Although if the options were supported the README would likely also need to be a little more verbose to explain the options

@andmcgregor andmcgregor force-pushed the wait_for_selector_support branch from 4db6ba1 to 228681e Compare June 22, 2020 22:18

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@andmcgregor andmcgregor force-pushed the wait_for_selector_support branch from 228681e to ec94c76 Compare June 22, 2020 22:20
@andmcgregor
Copy link
Contributor Author

Thanks for the feedback, I added support for passing options in the most recent commit. If more in depth documentation would be preferred in the README let me know- I just linked to the puppeteer documentation for an explanation of the options.

Copy link
Contributor

@abrom abrom left a comment

Choose a reason for hiding this comment

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

Nice one, thanks @andmcgregor

@abrom abrom merged commit 54213f5 into Studiosity:main Jun 22, 2020
@abrom
Copy link
Contributor

abrom commented Jun 23, 2020

FYI I've released this in 0.12.2. Thanks for the PR 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants