feat: add page convenience methods for textContent and getAttribute #2235
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch adds: - `page.innerText()` / `frame.innerText()` - `page.innerHTML()` / `frame.innerHTML()` - `page.textContent()` / `frame.textContent()` - `page.getAttribute()` / `frame.getAttribute()` Fixes microsoft#2143
dgozman
reviewed
May 14, 2020
docs/api.md
Outdated
| @@ -1275,6 +1279,15 @@ Returns frame matching the specified criteria. Either `name` or `url` must be sp | |||
| #### page.frames() | |||
| - returns: <[Array]<[Frame]>> An array of all frames attached to the page. | |||
|
|
|||
| #### page.getAttribute(selector, name[, options]) | |||
| - `selector` <[string]> A selector to search for an element. If there are multiple elements satisfying the selector, the first will be checked. See [working with selectors](#working-with-selectors) for more details. | |||
There was a problem hiding this comment.
"will be checked" is probably a copy-paste from somewhere 😄 Same in other methods.
docs/api.md
Outdated
| - `timeout` <[number]> Maximum time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the [browserContext.setDefaultTimeout(timeout)](#browsercontextsetdefaulttimeouttimeout) or [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) methods. | ||
| - returns: <[Promise]<null|[string]>> | ||
|
|
||
| Resolves to the `node.innerHTML`. |
There was a problem hiding this comment.
We always resolve to elements, make this element.innerHTML? Same in other methods.
src/page.ts
Outdated
| return this.mainFrame().textContent(selector, options); | ||
| } | ||
|
|
||
| async innerText(selector: string, options?: types.TimeoutOptions): Promise<string | null> { |
There was a problem hiding this comment.
How can innerText or innerHTML be null?
Also, for textContent, would it be much more convenient to convert null into empty string?
There was a problem hiding this comment.
these all are actually always strings; casted.
dgozman
approved these changes
May 14, 2020
src/frames.ts
Outdated
|
|
||
| async innerText(selector: string, options: types.TimeoutOptions = {}): Promise<string> { | ||
| return await this._retryWithSelectorIfNotConnected('innerText', selector, options, | ||
| (handle, deadline) => handle.innerText() as Promise<string>); |
There was a problem hiding this comment.
Why don't we update ElementHandle.innerText() and ElementHandle.innerHTML() to return string as well?
src/frames.ts
Outdated
| @@ -730,6 +730,26 @@ export class Frame { | |||
| (handle, deadline) => handle.focus()); | |||
| } | |||
|
|
|||
| async textContent(selector: string, options: types.TimeoutOptions = {}): Promise<string> { | |||
| return await this._retryWithSelectorIfNotConnected('textContent', selector, options, | |||
| (handle, deadline) => handle.textContent() as Promise<string>); | |||
There was a problem hiding this comment.
Hmm... We should have the same return type here and in ElementHandle.textContent.
dgozman
approved these changes
May 19, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds:
page.innerText()/frame.innerText()page.innerHTML()/frame.innerHTML()page.textContent()/frame.textContent()page.getAttribute()/frame.getAttribute()Fixes #2143