Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Tooltip: improve tests #57345

Merged
merged 25 commits into from
Jan 4, 2024
Merged

Tooltip: improve tests #57345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
e826e66
Remove unnecessary RegExp
ciampo Dec 22, 2023
6ca3f5d
Use @ariakit/test instead of testing library
ciampo Dec 22, 2023
6b5334f
Use sleep utility instead of custom promise
ciampo Dec 22, 2023
c97a986
Remove unnecessary import
ciampo Dec 22, 2023
8292091
Add utility functions
ciampo Dec 22, 2023
32b5703
Make multiple children test actually test what it's supposed to test
ciampo Dec 22, 2023
8cd4033
Test for tooltip to hide when another element gets focus
ciampo Dec 22, 2023
3894dca
Test for tooltip to hide when hovering outside the anchor
ciampo Dec 22, 2023
a6ca61f
Improve tooltip hiding on click test
ciampo Dec 22, 2023
7674cb0
More utils refactor
ciampo Dec 22, 2023
0c47cab
Improve test with disabled anchor
ciampo Dec 22, 2023
18a228c
Wait for tooltip to be hidden in the hideOnClick={false} test
ciampo Dec 22, 2023
5e590e3
Refactor custom delay test
ciampo Dec 22, 2023
4fe91bc
Refactor "mouse leave early" test (skip as it's still not passing)
ciampo Dec 22, 2023
be278f8
Improve shortcut-related tests
ciampo Dec 22, 2023
090e22b
Improve the Modal Escape test
ciampo Dec 22, 2023
b5aaf5a
Refactor description text test
ciampo Dec 22, 2023
cf1ea1a
Add second-level describes
ciampo Dec 22, 2023
2d4692a
Improve TS expecte error message
ciampo Jan 3, 2024
536ddeb
Add missing async/await in test utils
ciampo Jan 3, 2024
636678f
Await for tooltip to appear in shortcut tests
ciampo Jan 3, 2024
0e8c5e6
Apply delay prop only when showing the tooltip
ciampo Jan 3, 2024
57b5335
Add skip timeout waiting time after each test
ciampo Jan 3, 2024
21d2c0e
CHANGELOG
ciampo Jan 3, 2024
89372ed
Apply review feedback
ciampo Jan 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
- `Modal`: Improve application of body class names ([#55430](https://github.com/WordPress/gutenberg/pull/55430)).
- `InputControl`, `NumberControl`, `UnitControl`, `SelectControl`, `TreeSelect`: Add `compact` size variant ([#57398](https://github.com/WordPress/gutenberg/pull/57398)).
- `ToggleGroupControl`: Update button size in large variant to be 32px ([#57338](https://github.com/WordPress/gutenberg/pull/57338)).
- `Tooltip`: improve unit tests ([#57345](https://github.com/WordPress/gutenberg/pull/57345)).

### Experimental

Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/modal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ function UnforwardedModal(

if (
shouldCloseOnEsc &&
event.code === 'Escape' &&
( event.code === 'Escape' || event.key === 'Escape' ) &&
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ariakit/test keyboard events use event.key, so I had to add this check in order for the Modal component to react to those simulated user events

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can implement code in @ariakit/test. I'm just not sure how to transform keys into code, and I'd prefer to sidestep the need to maintain a full map of values.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we can keep this change to the Modal component for now (cc @mirka )

! event.defaultPrevented
) {
event.preventDefault();
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/tooltip/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ function Tooltip( props: TooltipProps ) {

const tooltipStore = Ariakit.useTooltipStore( {
placement: computedPlacement,
timeout: delay,
showTimeout: delay,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the delay prop, it looks like it was always intended to only be applied to the "show" timeout, and not affect the "hide" timeout.

} );

return (
Expand Down
Loading
Loading