-
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 editing-widgets
to Playwright
#57483
Conversation
Size Change: -747 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
d843c12
to
2465c47
Compare
2465c47
to
5cf39f6
Compare
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.
Thank you, @kevin940726!
I just had two minor notes, but migration looks great. I like that we could also enable a few skipped tests 🎉
I discovered some bugs in the widgets screen during the migration.
Saw a few inline notes. Should we create separate issues for those?
await page | ||
.getByRole( 'button', { name: 'Toggle block inserter' } ) | ||
.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.
Nit (not a blocker): Should we make the locator more specific?
await page
.getByRole( 'toolbar', { name: 'Document tools' } )
.getByRole( 'button', { name: 'Toggle block inserter' } )
.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.
Sure!
// eslint-disable-next-line playwright/no-force-option | ||
force: true, |
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.
Nit (not a blocker): It could be helpful in future to have reason for using force: true
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.
I thought I did 😅 .
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.
Turns out it's not doing what it seems, I refactored it 😅 .
What?
Part of #38851. Migrate
editing-widgets
to Playwright.This is also the last missing piece to completely remove
puppeteer-testing-library
from the project.Why?
See #38851.
How?
I discovered some bugs in the widgets screen during the migration. Also updated
insertBlock
andgetBlocks
to allow retrieving from a specified parent block.Testing Instructions
CI Should pass.
Screenshots or screencast
N/A