-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Migrate format API test for playwright #42035
Migrate format API test for playwright #42035
Conversation
Hello @kevin940726, Could you review this PR. ?. |
Hi @kevin940726, Hope you are doing well. |
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.
Hi! Sorry I missed the pings! Thank you as always 💯
const button = await page.locator( | ||
`//button[contains(text(), '${ 'Custom Link' }')]` | ||
); | ||
await button.click(); |
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.
Have you read the best practice guide? I think we can refactor this into role selector instead:
const button = await page.locator( | |
`//button[contains(text(), '${ 'Custom Link' }')]` | |
); | |
await button.click(); | |
await page.click( 'role=button[name="Custom Link"i]' ); |
Hi @kevin940726, Yes, I tried that, but because the button doesn't have the name attribute available, it is failing. |
@pavanpatil1 It's not a DOM attribute but a computed accessible attribute interpreted by the browsers and screen readers. You can view what accessible attributes browsers see via devtools, take Chrome as an example: You can see that the element has a role of |
Hi @kevin940726, Thank you so much 💯 I addressed the above feedback. Could you check again ?. |
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.
Looks good! Remember to also delete the snapshot of the original test though (format-api.test.js.snap
).
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.
LGTM 👍 Thanks! 💯
What?
Part of #38851.
Migrate format-api.test.js to its Playwright version.
Why?
Part of #38851.
How?
See MIGRATION.md for migration steps.
Testing Instructions
Run
npm run test-e2e:playwright test/e2e/specs/editor/plugins/format-api.spec.js