Add new usePopoverShouldClose hook #1629
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1629 +/- ##
=======================================
Coverage 99.90% 99.90%
=======================================
Files 61 62 +1
Lines 1066 1071 +5
Branches 408 409 +1
=======================================
+ Hits 1065 1070 +5
Misses 1 1 ☔ View full report in Codecov by Sentry. |
acelaya
force-pushed
the
use-popover-should-close
branch
2 times, most recently
from
37398db to
1f232eb
Compare
acelaya
force-pushed
the
use-popover-should-close
branch
from
1f232eb to
4c92369
Compare
| assert.calledWith( | ||
| fakeUseClickAway, | ||
| sinon.match.any, | ||
| closeHandler, | ||
| options ?? {}, | ||
| ); | ||
| assert.calledWith( | ||
| fakeUseFocusAway, | ||
| sinon.match.any, | ||
| closeHandler, | ||
| options ?? {}, | ||
| ); | ||
| assert.calledWith( | ||
| fakeUseKeyPress, | ||
| ['Escape'], | ||
| closeHandler, | ||
| options ?? {}, | ||
| ); |
There was a problem hiding this comment.
Just mocking the other hooks and verifying they are called, as those hooks have their own tests.
|
The missing line of coverage is being reported against 
|
Oh, good catch! I was about to test these changes in client, I guess I would have realized it was not exported then 😅 |
Yeah, that is now always hardcoded as true or false when called through |
Sounds good. Let's move forward and do that soon. |
|
I thought I was merging something else 🤦🏼 Now I need to create another PR to export the symbol 😅 |
This PR adds a convenient
usePopoverShouldClosehook which is similar to the deprecateduseElementShouldClose, but internally combinesuseClickAway,useFocusAwayanduseKeyPressto make sure there's no implementation differences.It also provides a more contextual name that makes it clear in which cases is supposed to be used, as opposed to the cases in which some of the other three individual hooks should be used instead.