-
Notifications
You must be signed in to change notification settings - Fork 4.9k
feat(toHaveCss) Overload toHaveCSS matcher to accept React.CSSProperties #38617
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,7 @@ | |
| import type { TestStepInfoImpl } from '../worker/testInfo'; | ||
| import type { APIResponse, Locator, Frame, Page } from 'playwright-core'; | ||
| import type { FrameExpectParams } from 'playwright-core/lib/client/types'; | ||
| import type { CSSProperties } from 'react'; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't afford a dependency on 'react'. Can you make it work in a best effort fashion similar to what we do for Electron types today? Basically, if the user has 'react' in their project, strong typecheck would apply. Otherwise accepted type should be |
||
|
|
||
| export type ExpectMatcherStateInternal = ExpectMatcherState & { _stepInfo?: TestStepInfoImpl }; | ||
|
|
||
|
|
@@ -308,17 +309,39 @@ | |
| }, expected, options); | ||
| } | ||
|
|
||
| export function toHaveCSS(this: ExpectMatcherState, locator: LocatorEx, name: string, expected: string | RegExp, options?: { timeout?: number }): Promise<MatcherResult<any, any>>; | ||
| export function toHaveCSS(this: ExpectMatcherState, locator: LocatorEx, styles: CSSProperties, options?: { timeout?: number }): Promise<MatcherResult<any, any>>; | ||
| export function toHaveCSS( | ||
| this: ExpectMatcherState, | ||
| locator: LocatorEx, | ||
| name: string, | ||
| expected: string | RegExp, | ||
| nameOrStyles: string | CSSProperties, | ||
| expectedOrOptions?: (string | RegExp) | { timeout?: number }, | ||
| options?: { timeout?: number }, | ||
| ) { | ||
| return toMatchText.call(this, 'toHaveCSS', locator, 'Locator', async (isNot, timeout) => { | ||
| const expectedText = serializeExpectedTextValues([expected]); | ||
| return await locator._expect('to.have.css', { expressionArg: name, expectedText, isNot, timeout }); | ||
| }, expected, options); | ||
| if (typeof nameOrStyles === 'string') { | ||
| if (expectedOrOptions === undefined) | ||
| throw new Error(`toHaveCSS expected value must be provided`); | ||
| const expected = expectedOrOptions as string | RegExp; | ||
| return toMatchText.call(this, 'toHaveCSS', locator, 'Locator', async (isNot, timeout) => { | ||
| const expectedText = serializeExpectedTextValues([expected]); | ||
| return await locator._expect('to.have.css', { expressionArg: nameOrStyles, expectedText, isNot, timeout }); | ||
| }, expected, options); | ||
| } else { | ||
| const styles = nameOrStyles as CSSProperties; | ||
| const options = expectedOrOptions as { timeout?: number }; | ||
| return toEqual.call(this, 'toHaveCSS', locator, 'Locator', async (isNot, timeout) => { | ||
| const results: any[] = []; | ||
| for (const [property, value] of Object.entries(styles)) { | ||
| const cssProperty = reactCSSPropertyToCSSName(property); | ||
| const expectedText = serializeExpectedTextValues([value as string]); | ||
| const result = await locator._expect('to.have.css', { expressionArg: cssProperty, expectedText, isNot, timeout }); | ||
| results.push(result); | ||
| if (!result.matches) | ||
| return result; | ||
| } | ||
| return { matches: true }; | ||
| }, styles, options); | ||
| } | ||
| } | ||
|
|
||
| export function toHaveId( | ||
|
|
@@ -506,3 +529,18 @@ | |
| } | ||
| return {}; | ||
| } | ||
|
|
||
| function reactCSSPropertyToCSSName(name: keyof CSSProperties | string): string { | ||
| const isCustomProperty = name.startsWith('--'); | ||
| if (isCustomProperty) | ||
| return name; | ||
|
|
||
| const vendorMatch = name.match(/^(Webkit|Moz|Ms|O)([A-Z].*)/); | ||
| if (vendorMatch) { | ||
| const prefix = vendorMatch[1].toLowerCase(); | ||
| const property = vendorMatch[2]; | ||
| return `-${prefix}-${toKebabCase(property)}`; | ||
| } | ||
|
|
||
| return toKebabCase(name); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ | |
| */ | ||
|
|
||
| import type { APIRequestContext, Browser, BrowserContext, BrowserContextOptions, Page, LaunchOptions, ViewportSize, Geolocation, HTTPCredentials, Locator, APIResponse, PageScreenshotOptions } from 'playwright-core'; | ||
| import type { CSSProperties } from 'react'; | ||
| export * from 'playwright-core'; | ||
|
|
||
| export type BlobReporterOptions = { outputDir?: string, fileName?: string }; | ||
|
|
@@ -492,6 +493,12 @@ export type Expect<ExtendedMatchers = {}> = { | |
| declare global { | ||
| export namespace PlaywrightTest { | ||
| export interface Matchers<R, T = unknown> { | ||
| /** | ||
| * Ensures the Locator resolves to an element with given CSS values. | ||
| * | ||
| * @param styles CSS property names and values as an object. | ||
| */ | ||
| toHaveCSS(styles: CSSProperties, options?: { timeout?: number }): Promise<void>; | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wasn't 100% on how the build process works. Is this the correct place to add the extension?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is correct place for type overrides. But ideally you just update the parameter type definition in |
||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to update
valueparameter's type to acceptReact.CSSPropertiesrather than add a new parameter.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What makes this a little awkward is that this feature requires less parameters than before. ie. one parameter for the pojo CSS Properties, as opposed to the two parameters for the name and the value.
The current PR handles multiple possible inputs for the first argument (either the name string, or the React.CSSProperties), albeit with some questionable naming.
Should I just delete the overload signatures, and use the main function definition that has union parameters?