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

docs: page.waitFor options object defination #5021

Merged
merged 5 commits into from Oct 24, 2019
Merged

docs: page.waitFor options object defination #5021

merged 5 commits into from Oct 24, 2019

Conversation

theeko
Copy link
Contributor

@theeko theeko commented Oct 9, 2019

Added options descriptiptions for page.waitFor.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@theeko
Copy link
Contributor Author

theeko commented Oct 21, 2019

signed

docs/api.md Outdated
@@ -1895,6 +1895,12 @@ This is a shortcut for [page.mainFrame().url()](#frameurl)
#### page.waitFor(selectorOrFunctionOrTimeout[, options[, ...args]])
- `selectorOrFunctionOrTimeout` <[string]|[number]|[function]> A [selector], predicate or timeout to wait for
- `options` <[Object]> Optional waiting parameters
- `visible` <[boolean]> wait for element to be present in DOM and to be visible, i.e. to not have `display: none` or `visibility: hidden` CSS properties. Defaults to `false`.
Copy link
Member

Choose a reason for hiding this comment

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

What about the hidden attribute?

Maybe it's better to not explicitly list the examples (like display: none or visibility: hidden) here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed as requested. Examples are not listed anymore. Just shows default value and description.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@mathiasbynens
Copy link
Member

LGTM but before we merge this, we should make the linter happy and make sure the CI checks pass.

@theeko theeko changed the title doc: page.waitFor options object defination docs: page.waitFor options object defination Oct 23, 2019
@mathiasbynens
Copy link
Member

LGTM but before we merge this, we should make the linter happy and make sure the CI checks pass.

To do this, https://github.com/GoogleChrome/puppeteer/blob/1248a19135fe54b8fe42e15435c11c3c8aa949d2/lib/Page.js#L1111 needs to be updated accordingly.

@theeko
Copy link
Contributor Author

theeko commented Oct 23, 2019

Checks are 9/10.
Dont know what last error is (Travis CI - Pull Request Action required after 10m — Build Errored)
Can you guide me.

@mathiasbynens
Copy link
Member

@theeko The remaining failure seems unrelated to your change. I've just restarted the build.

@mathiasbynens mathiasbynens merged commit 7f3e372 into puppeteer:master Oct 24, 2019
@mathiasbynens
Copy link
Member

Thanks!

@theeko
Copy link
Contributor Author

theeko commented Oct 24, 2019

Thanks for guidance Mathias.

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

Successfully merging this pull request may close these issues.

None yet

3 participants