Skip to content

fix(HoverCard): portal via Theme refs without querySelector (#1100)#2095

Merged
kodiakhq[bot] merged 2 commits into
mainfrom
fix/issue-1100-hovercard-portal-theme-batch6
Jun 24, 2026
Merged

fix(HoverCard): portal via Theme refs without querySelector (#1100)#2095
kodiakhq[bot] merged 2 commits into
mainfrom
fix/issue-1100-hovercard-portal-theme-batch6

Conversation

@kotAPI

@kotAPI kotAPI commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

resolve portal container from ThemeContext refs instead of querySelector

Test plan

  • Related unit tests pass locally

Fixes #1100

Summary by CodeRabbit

  • Bug Fixes
    • Improved where the hover card content is rendered when a custom portal root is not provided, making portal placement more reliable.
  • Tests
    • Added coverage to verify hover card content appears in the expected portal container when triggered.

@changeset-bot

changeset-bot Bot commented Jun 24, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 8388a89

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@kotAPI

kotAPI commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

Code review

LGTM. Matches project patterns for portal Theme refs, Floating UI prop merge, controlled-switch/lazy-mount/RTL tests, or focused bug fixes. No changes requested.

@GoldGroove06 GoldGroove06 added the automerge A tag that tells kodiak bot to automerge PRs for us when tests and approval conditions are met label Jun 24, 2026
@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 02d44c25-135d-4d22-b1bf-5d8255620f88

📥 Commits

Reviewing files that changed from the base of the PR and between 1ecbdde and 8388a89.

📒 Files selected for processing (2)
  • src/components/ui/HoverCard/fragments/HoverCardPortal.tsx
  • src/components/ui/HoverCard/tests/HoverCard.portal.test.tsx

📝 Walkthrough

Walkthrough

HoverCardPortal is updated to resolve its portal root element using ThemeContext refs (portalRootRef.current or containerRef.current) instead of HoverCardContext and class-based DOM queries. A new test suite verifies that portaled content is injected into the [data-rad-ui-portal-root] element when hovering the trigger.

HoverCard portal root resolution

Layer / File(s) Summary
HoverCardPortal: ThemeContext-based root resolution
src/components/ui/HoverCard/fragments/HoverCardPortal.tsx
Removes HoverCardContext import and rootTriggerClass-based DOM lookup; replaces with ThemeContext refs (portalRootRef.current / containerRef.current) for resolving rootElem, falling back to document.body. useEffect dependencies updated accordingly.
Portal placement test
src/components/ui/HoverCard/tests/HoverCard.portal.test.tsx
Adds mockMatchMedia stub and a new describe('HoverCard portal') suite that renders HoverCard within Theme, simulates hover, and asserts portaled content is inside [data-rad-ui-portal-root].

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • rad-ui/ui#583: Introduced the original HoverCardPortal implementation using HoverCardContext.rootTriggerClass for portal root resolution, which this PR directly replaces.

Suggested labels

codex

🐇 No more class-hunt through the DOM,
ThemeContext refs now show the way home.
The portal lands where it ought to be,
Inside the root for all to see.
Hop, hop — the card is finally free! 🌿

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-1100-hovercard-portal-theme-batch6

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: one or more packages not found in the registry.


Comment @coderabbitai help to get the list of available commands.

@kodiakhq kodiakhq Bot merged commit 45aefb5 into main Jun 24, 2026
9 of 10 checks passed
@kodiakhq kodiakhq Bot deleted the fix/issue-1100-hovercard-portal-theme-batch6 branch June 24, 2026 16:33
@github-actions

Copy link
Copy Markdown
Contributor

Coverage

This report compares the PR with the base branch. "Δ" shows how the PR affects each metric.

Metric PR Δ
Statements 78.31% -0.01%
Branches 61.14% +0.05%
Functions 63.6% +0.00%
Lines 79.9% -0.01%

Coverage decreased for at least one metric. Please add or update tests to improve coverage.

Run npm run coverage:ci locally for detailed reports and target untested areas to raise these numbers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge A tag that tells kodiak bot to automerge PRs for us when tests and approval conditions are met

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: Alert Dialog not spawning as a child to the provided root element

2 participants