fix: support anchor elements with dataset capability instead of only HTMLElement#1280
Conversation
📝 WalkthroughWalkthroughThis PR widens tooltip anchor types from ChangesSVG/non-HTMLElement Anchor Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/components/Tooltip/Tooltip.tsxOops! Something went wrong! :( ESLint: 10.2.0 TypeError [ERR_IMPORT_ATTRIBUTE_MISSING]: Module "file:///.eslintrc.json?mtime=1780584077822" needs an import attribute of "type: json" src/components/Tooltip/TooltipTypes.d.tsOops! Something went wrong! :( ESLint: 10.2.0 TypeError [ERR_IMPORT_ATTRIBUTE_MISSING]: Module "file:///.eslintrc.json?mtime=1780584077822" needs an import attribute of "type: json" src/components/Tooltip/anchor-registry.tsOops! Something went wrong! :( ESLint: 10.2.0 TypeError [ERR_IMPORT_ATTRIBUTE_MISSING]: Module "file:///.eslintrc.json?mtime=1780584077822" needs an import attribute of "type: json"
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Beta version released with the last commit 🚀 or |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/test/tooltip-anchor-selection.spec.js`:
- Around line 217-231: The test should exercise delegated SVG target resolution
by firing the mouseOver from the inner <circle> element rather than the <svg>;
locate the SVG via screen.getByLabelText('SVG anchor'), grab the nested circle
(e.g. via querySelector('circle')), then call hoverAnchor or directly dispatch
mouseOver on that circle (preserving the 100ms timer advancement) and finally
await waitForTooltip('svg-anchor-test') so the delegated hover handler path is
covered; update the test named "opens for delegated hover on an svg anchor" and
keep TooltipController and the existing delay logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1dd53806-ac07-4b49-88a8-36af1ccf8075
📒 Files selected for processing (10)
src/components/Tooltip/Tooltip.tsxsrc/components/Tooltip/TooltipTypes.d.tssrc/components/Tooltip/anchor-registry.tssrc/components/Tooltip/use-tooltip-anchors.tsxsrc/components/Tooltip/use-tooltip-events.tsxsrc/components/TooltipController/TooltipController.tsxsrc/components/TooltipController/TooltipControllerTypes.d.tssrc/components/TooltipController/shared-attribute-observer.tssrc/test/tooltip-anchor-selection.spec.jssrc/utils/resolve-data-tooltip-anchor.ts
| test('opens for delegated hover on an svg anchor', async () => { | ||
| render( | ||
| <> | ||
| <svg data-tooltip-id="svg-anchor-test" aria-label="SVG anchor" width="16" height="16"> | ||
| <circle cx="8" cy="8" r="8" /> | ||
| </svg> | ||
| <TooltipController id="svg-anchor-test" content="SVG Anchor Test" /> | ||
| </>, | ||
| ) | ||
|
|
||
| const anchor = screen.getByLabelText('SVG anchor') | ||
|
|
||
| hoverAnchor(anchor, 100) | ||
| await waitForTooltip('svg-anchor-test') | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify delegated-hover helpers and event types used by this spec.
rg -n -C2 '\bhoverAnchor\b|fireEvent\.mouseEnter|fireEvent\.mouseOver' src/test/tooltip-anchor-selection.spec.js src/test/test-utils.jsRepository: ReactTooltip/react-tooltip
Length of output: 3830
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show context around delegated-hover-test and the SVG-anchor test
sed -n '180,260p' src/test/tooltip-anchor-selection.spec.js
# Show hover helpers for context
sed -n '1,120p' src/test/test-utils.jsRepository: ReactTooltip/react-tooltip
Length of output: 4670
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' src/test/tooltip-anchor-selection.spec.js
sed -n '120,220p' src/test/tooltip-anchor-selection.spec.jsRepository: ReactTooltip/react-tooltip
Length of output: 6394
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "delayShow|delayHide|showDelay|openDelay" src/components/TooltipController* src/components -S || true
rg -n "setTimeout|jest.useFakeTimers|advanceTimersByTime" src/components/TooltipController* src/components -S | head -n 80Repository: ReactTooltip/react-tooltip
Length of output: 3644
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "delayShow|delayHide|showDelay|openDelay|closeDelay" src/components -S || true
rg -n "setTimeout\(|clearTimeout\(|schedule|timer" src/components -S | head -n 120
ls -R src/components | head -n 80Repository: ReactTooltip/react-tooltip
Length of output: 4308
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba src/components/Tooltip/use-tooltip-events.tsx | sed -n '1,220p'
nl -ba src/components/Tooltip/use-tooltip-events.tsx | sed -n '220,360p'Repository: ReactTooltip/react-tooltip
Length of output: 113
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Re-run without `nl`
cat -n src/components/Tooltip/use-tooltip-events.tsx | sed -n '1,240p'
cat -n src/components/Tooltip/use-tooltip-events.tsx | sed -n '240,380p'Repository: ReactTooltip/react-tooltip
Length of output: 14958
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "function debounce|export function debounce|debounce\\(" src/utils -S
ls src/utilsRepository: ReactTooltip/react-tooltip
Length of output: 507
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n src/utils/debounce.tsRepository: ReactTooltip/react-tooltip
Length of output: 1875
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n src/components/Tooltip/use-tooltip-events.tsx | sed -n '380,620p'Repository: ReactTooltip/react-tooltip
Length of output: 7208
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "'mouseenter'|mouseenter" src/components/Tooltip/use-tooltip-events.tsxRepository: ReactTooltip/react-tooltip
Length of output: 308
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "mouseenter|mouseleave|mouseover|mouseout" src/components/Tooltip -SRepository: ReactTooltip/react-tooltip
Length of output: 2304
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "addEventListener\\(['\"]mouseenter['\"]|onMouseEnter|mouseenter\\b" src/components/Tooltip -S
rg -n "mouseEnter\\b" src -S
rg -n "addEventListener\\(['\"]mouseleave['\"]|onMouseLeave|mouseleave\\b" src/components/Tooltip -S
rg -n "event-delegation|addDelegatedEventListener" src/components/Tooltip -SRepository: ReactTooltip/react-tooltip
Length of output: 3415
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n src/components/Tooltip/event-delegation.tsRepository: ReactTooltip/react-tooltip
Length of output: 2774
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "mouseenter|mouseleave" src/components/Tooltip -S
rg -n "addEventListener\\([^\\)]*['\"]mouseenter['\"]|onMouseEnter" src/components/Tooltip -S || true
sed -n '1,240p' src/components/Tooltip/use-tooltip-anchors.tsx 2>/dev/null || true
sed -n '1,260p' src/components/Tooltip/Tooltip.tsx 2>/dev/null || trueRepository: ReactTooltip/react-tooltip
Length of output: 11408
🌐 Web query:
@testing-library/dom fireEvent.mouseEnter dispatches which native events mouseover mouseenter
💡 Result:
When you call fireEvent.mouseEnter in @testing-library/dom, it dispatches both a mouseenter event and a mouseover event [1]. Historically, this behavior was implemented to ensure that handlers added via both React (which often relies on mouseover for its synthetic mouseEnter events) and native addEventListener methods are triggered correctly [1]. Specifically, the library dispatches both event types to ensure compatibility across different event-handling strategies in the DOM [1]. It is important to note that fireEvent is intended for simple, direct DOM event dispatching and does not simulate full, complex user interactions—such as the exact sequence of events a browser would trigger during a mouse movement [2]. For more realistic simulations of user behavior, the library authors recommend using the @testing-library/user-event package [2].
Citations:
- 1: fix: make fireEvent mouseEnter/mouseLeave work with addEventListener testing-library/react-testing-library#588
- 2: Explore alternative event dispatch/simulation testing-library/react-testing-library#35
Exercise delegated SVG target resolution by hovering the inner <circle>.
hoverAnchor uses fireEvent.mouseEnter, and RTL dispatches mouseover too, so the delegated hover handler may already run. The missing coverage is when the event target is a nested SVG element (the current test targets the <svg> itself). Dispatch mouseOver from the <circle> and keep timer advancement so the tooltip show delay runs.
Suggested change
test('opens for delegated hover on an svg anchor', async () => {
render(
<>
<svg data-tooltip-id="svg-anchor-test" aria-label="SVG anchor" width="16" height="16">
<circle cx="8" cy="8" r="8" />
</svg>
<TooltipController id="svg-anchor-test" content="SVG Anchor Test" />
</>,
)
const anchor = screen.getByLabelText('SVG anchor')
+ const circle = anchor.querySelector('circle')
- hoverAnchor(anchor, 100)
+ fireEvent.mouseOver(circle)
+ advanceTimers(100)
await waitForTooltip('svg-anchor-test')
})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/tooltip-anchor-selection.spec.js` around lines 217 - 231, The test
should exercise delegated SVG target resolution by firing the mouseOver from the
inner <circle> element rather than the <svg>; locate the SVG via
screen.getByLabelText('SVG anchor'), grab the nested circle (e.g. via
querySelector('circle')), then call hoverAnchor or directly dispatch mouseOver
on that circle (preserving the 100ms timer advancement) and finally await
waitForTooltip('svg-anchor-test') so the delegated hover handler path is
covered; update the test named "opens for delegated hover on an svg anchor" and
keep TooltipController and the existing delay logic unchanged.
1 participant
Fix #1279
Summary by CodeRabbit
New Features
Tests