From 9f71c96740646c6c0754beb8cb79a7a5d54ed1e9 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 1 Jul 2021 13:30:16 -0700 Subject: [PATCH] api: remove timeout option from isVisible and isHidden methods (#7414) It is unused and confusing. --- docs/src/api/class-frame.md | 4 - docs/src/api/class-page.md | 6 +- src/client/frame.ts | 8 +- src/client/page.ts | 8 +- src/dispatchers/frameDispatcher.ts | 4 +- src/protocol/channels.ts | 7 +- src/protocol/protocol.yml | 4 +- src/protocol/validator.ts | 2 - src/server/frames.ts | 8 +- types/types.d.ts | 122 +++++++++++++--------------- utils/generate_types/overrides.d.ts | 38 +++++++++ utils/generate_types/test/test.ts | 4 + 12 files changed, 120 insertions(+), 95 deletions(-) diff --git a/docs/src/api/class-frame.md b/docs/src/api/class-frame.md index d3456c693d..83a33c7feb 100644 --- a/docs/src/api/class-frame.md +++ b/docs/src/api/class-frame.md @@ -910,8 +910,6 @@ Returns whether the element is hidden, the opposite of [visible](./actionability ### param: Frame.isHidden.selector = %%-input-selector-%% -### option: Frame.isHidden.timeout = %%-input-timeout-%% - ## async method: Frame.isVisible - returns: <[boolean]> @@ -919,8 +917,6 @@ Returns whether the element is [visible](./actionability.md#visible). [`option: ### param: Frame.isVisible.selector = %%-input-selector-%% -### option: Frame.isVisible.timeout = %%-input-timeout-%% - ## method: Frame.name - returns: <[string]> diff --git a/docs/src/api/class-page.md b/docs/src/api/class-page.md index 854abe0cd0..aff6b9d585 100644 --- a/docs/src/api/class-page.md +++ b/docs/src/api/class-page.md @@ -2016,8 +2016,6 @@ Returns whether the element is hidden, the opposite of [visible](./actionability ### param: Page.isHidden.selector = %%-input-selector-%% -### option: Page.isHidden.timeout = %%-input-timeout-%% - ## async method: Page.isVisible - returns: <[boolean]> @@ -2025,8 +2023,6 @@ Returns whether the element is [visible](./actionability.md#visible). [`option: ### param: Page.isVisible.selector = %%-input-selector-%% -### option: Page.isVisible.timeout = %%-input-timeout-%% - ## property: Page.keyboard - type: <[Keyboard]> @@ -2624,7 +2620,7 @@ page.select_option("select#colors", value=["red", "green", "blue"]) await page.SelectOptionAsync("select#colors", new[] { "blue" }); // single selection matching both the value and the label await page.SelectOptionAsync("select#colors", new[] { new SelectOptionValue() { Label = "blue" } }); -// multiple +// multiple await page.SelectOptionAsync("select#colors", new[] { "red", "green", "blue" }); ``` diff --git a/src/client/frame.ts b/src/client/frame.ts index 1a9ac172b5..4af1fddcbc 100644 --- a/src/client/frame.ts +++ b/src/client/frame.ts @@ -372,15 +372,15 @@ export class Frame extends ChannelOwner { + async isHidden(selector: string): Promise { return this._wrapApiCall(async (channel: channels.FrameChannel) => { - return (await channel.isHidden({ selector, ...options })).value; + return (await channel.isHidden({ selector })).value; }); } - async isVisible(selector: string, options: channels.FrameIsVisibleOptions = {}): Promise { + async isVisible(selector: string): Promise { return this._wrapApiCall(async (channel: channels.FrameChannel) => { - return (await channel.isVisible({ selector, ...options })).value; + return (await channel.isVisible({ selector })).value; }); } diff --git a/src/client/page.ts b/src/client/page.ts index 93c0c369e9..929f5a6fc3 100644 --- a/src/client/page.ts +++ b/src/client/page.ts @@ -557,12 +557,12 @@ export class Page extends ChannelOwner { - return this._mainFrame.isHidden(selector, options); + async isHidden(selector: string): Promise { + return this._mainFrame.isHidden(selector); } - async isVisible(selector: string, options?: channels.FrameIsVisibleOptions): Promise { - return this._mainFrame.isVisible(selector, options); + async isVisible(selector: string): Promise { + return this._mainFrame.isVisible(selector); } async hover(selector: string, options?: channels.FrameHoverOptions) { diff --git a/src/dispatchers/frameDispatcher.ts b/src/dispatchers/frameDispatcher.ts index f998cf8381..843e0da1ab 100644 --- a/src/dispatchers/frameDispatcher.ts +++ b/src/dispatchers/frameDispatcher.ts @@ -175,11 +175,11 @@ export class FrameDispatcher extends Dispatcher { - return { value: await this._frame.isHidden(metadata, params.selector, params) }; + return { value: await this._frame.isHidden(metadata, params.selector) }; } async isVisible(params: channels.FrameIsVisibleParams, metadata: CallMetadata): Promise { - return { value: await this._frame.isVisible(metadata, params.selector, params) }; + return { value: await this._frame.isVisible(metadata, params.selector) }; } async hover(params: channels.FrameHoverParams, metadata: CallMetadata): Promise { diff --git a/src/protocol/channels.ts b/src/protocol/channels.ts index f8e1372588..cd53d5dfd8 100644 --- a/src/protocol/channels.ts +++ b/src/protocol/channels.ts @@ -1627,20 +1627,18 @@ export type FrameIsEnabledResult = { }; export type FrameIsHiddenParams = { selector: string, - timeout?: number, }; export type FrameIsHiddenOptions = { - timeout?: number, + }; export type FrameIsHiddenResult = { value: boolean, }; export type FrameIsVisibleParams = { selector: string, - timeout?: number, }; export type FrameIsVisibleOptions = { - timeout?: number, + }; export type FrameIsVisibleResult = { value: boolean, @@ -3290,6 +3288,7 @@ export const commandsWithTracingSnapshots = new Set([ 'Frame.isDisabled', 'Frame.isEnabled', 'Frame.isHidden', + 'Frame.isVisible', 'Frame.isEditable', 'Frame.press', 'Frame.selectOption', diff --git a/src/protocol/protocol.yml b/src/protocol/protocol.yml index e4999a8770..535257c220 100644 --- a/src/protocol/protocol.yml +++ b/src/protocol/protocol.yml @@ -1355,7 +1355,6 @@ Frame: isHidden: parameters: selector: string - timeout: number? returns: value: boolean tracing: @@ -1364,9 +1363,10 @@ Frame: isVisible: parameters: selector: string - timeout: number? returns: value: boolean + tracing: + snapshot: true isEditable: parameters: diff --git a/src/protocol/validator.ts b/src/protocol/validator.ts index 9491f90e26..603dd56568 100644 --- a/src/protocol/validator.ts +++ b/src/protocol/validator.ts @@ -669,11 +669,9 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme { }); scheme.FrameIsHiddenParams = tObject({ selector: tString, - timeout: tOptional(tNumber), }); scheme.FrameIsVisibleParams = tObject({ selector: tString, - timeout: tOptional(tNumber), }); scheme.FrameIsEditableParams = tObject({ selector: tString, diff --git a/src/server/frames.ts b/src/server/frames.ts index e53ae48fc8..356e744f93 100644 --- a/src/server/frames.ts +++ b/src/server/frames.ts @@ -1062,17 +1062,17 @@ export class Frame extends SdkObject { return dom.throwFatalDOMError(dom.throwRetargetableDOMError(result)); } - async isVisible(metadata: CallMetadata, selector: string, options: types.TimeoutOptions = {}): Promise { + async isVisible(metadata: CallMetadata, selector: string): Promise { const controller = new ProgressController(metadata, this); return controller.run(async progress => { progress.log(` checking visibility of "${selector}"`); const element = await this.$(selector); return element ? await element.isVisible() : false; - }, this._page._timeoutSettings.timeout(options)); + }, this._page._timeoutSettings.timeout({})); } - async isHidden(metadata: CallMetadata, selector: string, options: types.TimeoutOptions = {}): Promise { - return !(await this.isVisible(metadata, selector, options)); + async isHidden(metadata: CallMetadata, selector: string): Promise { + return !(await this.isVisible(metadata, selector)); } async isDisabled(metadata: CallMetadata, selector: string, options: types.TimeoutOptions = {}): Promise { diff --git a/types/types.d.ts b/types/types.d.ts index 90d47f1f50..23939cf045 100644 --- a/types/types.d.ts +++ b/types/types.d.ts @@ -367,6 +367,35 @@ export interface Page { */ exposeBinding(name: string, playwrightBinding: (source: BindingSource, arg: JSHandle) => any, options: { handle: true }): Promise; exposeBinding(name: string, playwrightBinding: (source: BindingSource, ...args: any[]) => any, options?: { handle?: boolean }): Promise; + + /** + * Returns whether the element is hidden, the opposite of [visible](https://playwright.dev/docs/actionability#visible). `selector` that does not + * match any elements is considered hidden. + * @param selector A selector to search for element. If there are multiple elements satisfying the selector, the first will be used. See [working with selectors](https://playwright.dev/docs/selectors) for more details. + */ + isHidden(selector: string): Promise; + isHidden(selector: string, options?: { + /** + * Option `timeout` has no effect. + * @deprecated + */ + timeout?: number + }): Promise; + + /** + * Returns whether the element is [visible](https://playwright.dev/docs/actionability#visible). `selector` that does not match any elements is + * considered not visible. + * @param selector A selector to search for element. If there are multiple elements satisfying the selector, the first will be used. See [working with selectors](https://playwright.dev/docs/selectors) for more details. + */ + isVisible(selector: string): Promise; + isVisible(selector: string, options?: { + /** + * Option `timeout` has no effect. + * @deprecated + */ + timeout?: number + }): Promise; + /** * Emitted when the page closes. */ @@ -2109,38 +2138,6 @@ export interface Page { timeout?: number; }): Promise; - /** - * Returns whether the element is hidden, the opposite of [visible](https://playwright.dev/docs/actionability#visible). `selector` that does not - * match any elements is considered hidden. - * @param selector A selector to search for element. If there are multiple elements satisfying the selector, the first will be used. See [working with selectors](https://playwright.dev/docs/selectors) for more details. - * @param options - */ - isHidden(selector: string, options?: { - /** - * 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)](https://playwright.dev/docs/api/class-browsercontext#browser-context-set-default-timeout) - * or [page.setDefaultTimeout(timeout)](https://playwright.dev/docs/api/class-page#page-set-default-timeout) methods. - */ - timeout?: number; - }): Promise; - - /** - * Returns whether the element is [visible](https://playwright.dev/docs/actionability#visible). `selector` that does not match any elements is - * considered not visible. - * @param selector A selector to search for element. If there are multiple elements satisfying the selector, the first will be used. See [working with selectors](https://playwright.dev/docs/selectors) for more details. - * @param options - */ - isVisible(selector: string, options?: { - /** - * 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)](https://playwright.dev/docs/api/class-browsercontext#browser-context-set-default-timeout) - * or [page.setDefaultTimeout(timeout)](https://playwright.dev/docs/api/class-page#page-set-default-timeout) methods. - */ - timeout?: number; - }): Promise; - keyboard: Keyboard; /** @@ -3556,6 +3553,35 @@ export interface Frame { waitForSelector(selector: string, options?: PageWaitForSelectorOptionsNotHidden): Promise>; waitForSelector(selector: K, options: PageWaitForSelectorOptions): Promise | null>; waitForSelector(selector: string, options: PageWaitForSelectorOptions): Promise>; + + /** + * Returns whether the element is hidden, the opposite of [visible](https://playwright.dev/docs/actionability#visible). `selector` that does not + * match any elements is considered hidden. + * @param selector A selector to search for element. If there are multiple elements satisfying the selector, the first will be used. See [working with selectors](https://playwright.dev/docs/selectors) for more details. + */ + isHidden(selector: string): Promise; + isHidden(selector: string, options?: { + /** + * Option `timeout` has no effect. + * @deprecated + */ + timeout?: number + }): Promise; + + /** + * Returns whether the element is [visible](https://playwright.dev/docs/actionability#visible). `selector` that does not match any elements is + * considered not visible. + * @param selector A selector to search for element. If there are multiple elements satisfying the selector, the first will be used. See [working with selectors](https://playwright.dev/docs/selectors) for more details. + */ + isVisible(selector: string): Promise; + isVisible(selector: string, options?: { + /** + * Option `timeout` has no effect. + * @deprecated + */ + timeout?: number + }): Promise; + /** * Returns the added tag when the script's onload fires or when the script content was injected into frame. * @@ -4161,38 +4187,6 @@ export interface Frame { timeout?: number; }): Promise; - /** - * Returns whether the element is hidden, the opposite of [visible](https://playwright.dev/docs/actionability#visible). `selector` that does not - * match any elements is considered hidden. - * @param selector A selector to search for element. If there are multiple elements satisfying the selector, the first will be used. See [working with selectors](https://playwright.dev/docs/selectors) for more details. - * @param options - */ - isHidden(selector: string, options?: { - /** - * 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)](https://playwright.dev/docs/api/class-browsercontext#browser-context-set-default-timeout) - * or [page.setDefaultTimeout(timeout)](https://playwright.dev/docs/api/class-page#page-set-default-timeout) methods. - */ - timeout?: number; - }): Promise; - - /** - * Returns whether the element is [visible](https://playwright.dev/docs/actionability#visible). `selector` that does not match any elements is - * considered not visible. - * @param selector A selector to search for element. If there are multiple elements satisfying the selector, the first will be used. See [working with selectors](https://playwright.dev/docs/selectors) for more details. - * @param options - */ - isVisible(selector: string, options?: { - /** - * 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)](https://playwright.dev/docs/api/class-browsercontext#browser-context-set-default-timeout) - * or [page.setDefaultTimeout(timeout)](https://playwright.dev/docs/api/class-page#page-set-default-timeout) methods. - */ - timeout?: number; - }): Promise; - /** * Returns frame's name attribute as specified in the tag. * diff --git a/utils/generate_types/overrides.d.ts b/utils/generate_types/overrides.d.ts index 75a76c27e5..4278954795 100644 --- a/utils/generate_types/overrides.d.ts +++ b/utils/generate_types/overrides.d.ts @@ -59,6 +59,25 @@ export interface Page { exposeBinding(name: string, playwrightBinding: (source: BindingSource, arg: JSHandle) => any, options: { handle: true }): Promise; exposeBinding(name: string, playwrightBinding: (source: BindingSource, ...args: any[]) => any, options?: { handle?: boolean }): Promise; + + isHidden(selector: string): Promise; + isHidden(selector: string, options?: { + /** + * Option `timeout` has no effect. + * @deprecated + */ + timeout?: number + }): Promise; + + isVisible(selector: string): Promise; + isVisible(selector: string, options?: { + /** + * Option `timeout` has no effect. + * @deprecated + */ + timeout?: number + }): Promise; + } export interface Frame { @@ -91,6 +110,25 @@ export interface Frame { waitForSelector(selector: string, options?: PageWaitForSelectorOptionsNotHidden): Promise>; waitForSelector(selector: K, options: PageWaitForSelectorOptions): Promise | null>; waitForSelector(selector: string, options: PageWaitForSelectorOptions): Promise>; + + isHidden(selector: string): Promise; + isHidden(selector: string, options?: { + /** + * Option `timeout` has no effect. + * @deprecated + */ + timeout?: number + }): Promise; + + isVisible(selector: string): Promise; + isVisible(selector: string, options?: { + /** + * Option `timeout` has no effect. + * @deprecated + */ + timeout?: number + }): Promise; + } export interface BrowserContext { diff --git a/utils/generate_types/test/test.ts b/utils/generate_types/test/test.ts index fb08ac3782..e7663122bb 100644 --- a/utils/generate_types/test/test.ts +++ b/utils/generate_types/test/test.ts @@ -130,6 +130,10 @@ playwright.chromium.launch().then(async browser => { await page.emulateMedia({media: 'screen'}); await page.pdf({ path: 'page.pdf' }); + // Deprecated option still compiles. + await page.isVisible('div', { timeout: 123 }); + await page.mainFrame().isHidden('div', { timeout: 123 }); + await page.route('**/*', (route, interceptedRequest) => { if ( interceptedRequest.url().endsWith('.png') ||