test(DefinitionTooltip): fix unit tests and add coverage for DefinitionTooltip #17679
Conversation
* fix issue where initial click would open and immediately close tooltip * fix unit tests around this functionality
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17679 +/- ##
==========================================
+ Coverage 78.33% 79.00% +0.66%
==========================================
Files 408 408
Lines 14015 14008 -7
Branches 4344 4371 +27
==========================================
+ Hits 10979 11067 +88
+ Misses 2867 2774 -93
+ Partials 169 167 -2 ☔ View full report in Codecov by Sentry. |
* remove onMouseDown from Popover in favor of onMouseDown on button
Neues
changed the title
packages/react/src/components/Tooltip/__tests__/DefinitionTooltip-test.js
Outdated
Show resolved
Hide resolved
| await userEvent.click(screen.getByText('URL')); | ||
| expect(screen.getByText(definition)).toBeVisible(); | ||
| expect(screen.getByRole('button')).toHaveAttribute('aria-expanded', 'true'); |
There was a problem hiding this comment.
wondering if we can keep the check toBeVisible along with checking if aria-expanded is true
There was a problem hiding this comment.
I was debating leaving it but ultimately decided to remove it like I removed the others for the reason that it does not actually work as expected in this case; toBeVisible() returns true even when the tooltip is not expanded. I replaced the other instances where it was being used, and then elected to remove this one so that other devs don't get confused.
There was a problem hiding this comment.
Yeah I've seen this before too, the aria-expanded does seem like the best workaround 👍
* remove duplicate test case
alisonjoseph
added
status: ready to merge 🎉
and removed
status: one more review 👀
labels
85771f2
Changed
toBeVisiblewas not working as expected in this scenario, I am instead checking for thearia-expandedattribute).