-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Block Editor: Refactor LinkControl tests to @testing-library
#43147
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
Conversation
|
Size Change: 0 B Total Size: 1.24 MB ℹ️ View Unchanged
|
mirka
left a comment
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.
Wow, what a thoroughly tested piece of code 😄 Thanks for working on the big tasks!
I'm not familiar with this component, but the tests do seem to reveal some small a11y things that could be looked at, as you've mentioned as well. Marco and I are not actively maintaining this component, so thanks for picking it up. It's great when we can get tangible user-facing improvements out of refactoring tests!
packages/block-editor/src/components/link-control/test/index.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/link-control/test/index.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/link-control/test/index.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/link-control/test/index.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/link-control/test/index.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/link-control/test/index.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/link-control/test/index.js
Outdated
Show resolved
Hide resolved
d15d14f to
b31e97f
Compare
Co-authored-by: Lena Morita <lena@jaguchi.com>
Co-authored-by: Lena Morita <lena@jaguchi.com>
In case anyone wonders: It's because See also #45777 |
|
Indeed @swissspidy, we did uncover that later and refactored a few of the components from using |
What?
This PR refactors the
<LinkControl />tests to use@testing-libraryinstead ofreact-domand its testing utils.Why?
We've been aiming to have all our component tests resemble user behavior as much as possible, so this is a step in that direction.
How?
This is a complex PR with many different types of changes, namely:
@testing-library/react'srender()instead ofreact-dom's render.@testing-library/user-eventinstead ofreact-dom/test-utilsSimulate.@testing-library/react'sact()instead of thereact-dom/test-utilsone.act()where it is redundant.screenqueries instead ofquerySelectorandquerySelectorAll()toBeTruthy()->toBeVisible()..innerHTML, and morejest-domassertions instead.There's a bug with the
keyDownevents - they simply don't work with@testing-library/user-event, as for some reason the events dispatched there have akeyCode: 0. That's why we're falling back tofireEvent, where we can actually provide thekeyCodeourselves and get around that problem.There are a few more TODOs (and unadressed items from the ones listed above) in the file we're touching, but some of them will likely need changes to the components themselves, and this PR has grown a bit more than I expected, so I'm happy to follow-up with addressing those in another PRs.
Testing Instructions
Verify tests pass:
npm run test:unit packages/block-editor/src/components/link-control/test/index.js