Add helper functions to ElementHandle #1151
Conversation
There was a problem hiding this comment.
Hey @sheerun, Thanks for the PR!
This is three PRs in one:
- ElementHandle.$/ElementHandle.$$
- ElementHandle.content()
- JSHandle.evaluate() (i'm not comfortable with this method name, given its current semantics)
I'd like this PR to be scoped to the ElementHandle.$/ElementHandle.$$ since it seems to be the most important. If you have time, I'd be happy to review other PR's.
lib/ElementHandle.js
Outdated
| * @param {!Array<*>} args | ||
| * @return {!Promise<*>} | ||
| */ | ||
| async evaluate(pageFunction, ...args) { |
There was a problem hiding this comment.
I don't like the evaluate and evaluateHandle methods as they are in this PR: they are named similarly to page.evaluate but have a different semantics, passing in the element as a first argument. They also should belong to JSHandle rather then ElementHandle.
lib/ElementHandle.js
Outdated
| * @return {!Promise<String>} | ||
| */ | ||
| async content() { | ||
| return this.evaluate(e => e.outerHTML, this); |
There was a problem hiding this comment.
This is nice, I'd love to see this in a separate PR with a test.
| * @param {string} selector | ||
| * @return {!Promise<?ElementHandle>} | ||
| */ | ||
| async $(selector) { |
There was a problem hiding this comment.
a test for this and for $$?
|
Please guide me where and how to write tests for this feature |
@sheerun sure:
|
|
In what file tests for this feature should be? |
|
@sheerun all tests live in a |
|
@aslushnikov I've done what you asked for |
test/test.js
Outdated
There was a problem hiding this comment.
Just put describe('ElementHandle.$', ..) and describe('ElementHandle.$$', ..) test suites somewhere next to other ElementHandle test suites. This will save you work for setting up a browser, and also will make tests faster since you'll avoid re-launching the browser.
a138d30 to
5aadb15
Compare
|
ok the tests are passing |
|
the errors on travis are irrelevant |
…eer#1151) This patch adds `ElementHandle.$` and `ElementHandle.$$` methods to query nested elements. Fixes puppeteer#508
Fixes #508