Update @ariakit/react to v0.3.12 and @ariakit/test to v0.3.7 #57547
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
ciampo
added
[Type] Enhancement
ciampo
changed the title
|
Size Change: -31 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
|
A few unit tests broke — it looks like the failures are related to keyboard focus. I'll look into it |
|
The issues seemed related to the general incompatibility between This PR is (once again) ready for review. |
tyxla
approved these changes
Jan 5, 2024
Comment on lines
152
to
153
| await hover( document.body ); | ||
| await hover( document.body, { clientX: 10, clientY: 10 } ); |
There was a problem hiding this comment.
That is something that I derived from looking at how ariakit tests are written, and it is used to simulate hovering outside an element. I refactored it to a separate utility function to better clarify. ec59f15 (#57547)
Comment on lines
156
to
161
| await waitFor( () => | ||
| expect( | ||
| screen.queryByRole( 'tooltip', { | ||
| name: 'Click for Delicious Gnocchi', | ||
| } ) | ||
| ).not.toBeInTheDocument() |
There was a problem hiding this comment.
Is waitForElementToBeRemoved() a better possible alternative here?
There was a problem hiding this comment.
It is not, because it turns out that there is no need to wait for it — the tooltip gets removed immediately, so I refactored the code and removed the waitFor altogether. I also took the chance to refine some tooltip-related tests in ToggleGroupControl to actually test that the tooltip doesn't show even after its default delay has passed 428809b
Does this mean the failures couldn't be replicated when manually tested in the browser? |
ciampo
force-pushed
the
update/ariakit
branch
from
06639c4 to
428809b
Compare
Exactly. I couldn't replicate failures in the browser. On top of that, those tests were flaky — sometimes they would pass, sometimes they would fail, also based on which specific tests are running. |
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?
Update
@ariakit/*dependencies to latest versionWhy?
We should keep our dependencies up to date to enjoy of the recent features and bug fixes. We should also update frequently to make sure that each update is quick and easier to test.
How?
As per the docs:
package.jsonfilesnpm run distclean && npm installnpm run distclean && npm installTesting Instructions
From the official CHANGELOG, there are no breaking changes highlighted since the previous version (
0.3.10).Most of the items seem to be related to the
Comboboxcomponent (which we don't use); here are the items that could be potentially relevant to our repository:Our ariakit-based components should be relatively well tested, so I assume that a good smoke test in Storybook and in the editor should be enough.