CustomSelectControl: align v1 and legacy v2 unit tests #62706
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
ciampo
added
[Package] Components
|
Size Change: 0 B Total Size: 1.76 MB ℹ️ View Unchanged
|
tyxla
approved these changes
Jun 20, 2024
| expect( mockOnChange ).toHaveBeenNthCalledWith( | ||
| 1, | ||
| expect.objectContaining( { | ||
| inputValue: '', | ||
| isOpen: false, | ||
| // TODO: key should be different — this is a known bug and will be fixed |
| } ) | ||
| ); | ||
|
|
||
| // DIFFERENCE WITH V2: NOT CALLED |
There was a problem hiding this comment.
If this is something that we need to update, let's add a TODO comment to remember to address it
There was a problem hiding this comment.
I'm not sure about how much we can do about this, since:
- we don't want to change the v1 behaviour (which doesn't call
onChangeon initial render) - the v2 behaviour comes directly from
Ariakit— I'm not sure how easy it would be to suppress only the first call without introducing unwanted side effects.
I will add an item to the tracking issue
There was a problem hiding this comment.
Good news! This will be fixed in #62733
| /> | ||
| ); | ||
|
|
||
| await user.click( |
There was a problem hiding this comment.
Just to confirm, using @testing-library/user-event for v1 vs using @ariakit/test for legacy v2 is an expected difference, right?
There was a problem hiding this comment.
It is — we had to use @ariakit/test for ariakit-based components as (if I recall) @testing-library/user-event doesn't work well with how Ariakit performs DOM updates
85 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
While working on #62255 and, more in general, on refining the new version of
CustomSelectControl, one of the main tasks is to make sure that the new version of the component offers an easy way to replace the old implementation with the new implementation pain-free — and that is what the legacy v2 version is for. It implements the same API surface as the v1 and it aims at behaving as closely as possible as the v1, despite its internal implementation is completely different.This PR aims at aligning further the unit tests between v1 and legacy v2 as much as possible, as a way to gain more confidence when making future fixes and changes to the legacy v2 version.
Why?
See above.
How?
Making sure that the unit tests for v1 and legacy v2 overlap as much as possible
Testing Instructions
No runtime changes