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

Exporting RadixUI Portal to Avoid Hidden Hover Content #1984

Closed
jonioni opened this issue Nov 20, 2023 · 3 comments · May be fixed by #5409
Closed

Exporting RadixUI Portal to Avoid Hidden Hover Content #1984

jonioni opened this issue Nov 20, 2023 · 3 comments · May be fixed by #5409
Labels
Stale

Comments

@jonioni
Copy link

jonioni commented Nov 20, 2023 •
edited
Loading

Related: #1940

In current Tooltip, HoverCard components, the Portal component from radix UI isn't exported:

ui/apps/www/registry/default/ui/tooltip.tsx

Line 30 in 6f30502

export { Tooltip, TooltipTrigger, TooltipContent, TooltipProvider }

ui/apps/www/registry/default/ui/hover-card.tsx

Line 29 in 6f30502

export { HoverCard, HoverCardTrigger, HoverCardContent }

And it's also not used in the official example.

ui/apps/www/content/docs/components/hover-card.mdx

Lines 61 to 66 in 6f30502

<HoverCard>
<HoverCardTrigger>Hover</HoverCardTrigger>
<HoverCardContent>
The React Framework – created and maintained by @vercel.
</HoverCardContent>
</HoverCard>

However, the portal is of crucial importance to the hover related components, without which the hover content will be constrained within the wrapping component if the wrapping component itself has limited size, with everything exceeding the wrapping component boundary hidden.

People new to the library (and Radix UI) will likely be stepping into this issue. It might be good to have the export and official example corrected.

Please let me know if a PR is preferred. I can help create one.
@shadcn
Copy link
Collaborator

shadcn commented Nov 21, 2023

Agreed. We should wrap the <TooltipContent /> in a portal by default.

ui/apps/www/registry/new-york/ui/tooltip.tsx

Line 18 in 6f30502

<TooltipPrimitive.Content
like we did for <AlertDialog />:

ui/apps/www/registry/new-york/ui/alert-dialog.tsx

Line 34 in 6f30502

<AlertDialogPortal>

Happy to review and merge a PR. Thank you.

@shadcn shadcn added the Stale label Feb 4, 2024
@shadcn
Copy link
Collaborator

shadcn commented Feb 12, 2024

This issue has been automatically closed because it received no activity for a while. If you think it was closed by accident, please leave a comment. Thank you.

@shadcn shadcn closed this as completed Feb 12, 2024
@jonioni jonioni mentioned this issue Oct 16, 2024
Open
@jonioni
Copy link
Author

jonioni commented Oct 16, 2024

Hi @shadcn, a new PR (#5409) has been made for this issue. Please review, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(hover-card,tooltip): add missing portal for hover card and tooltip jonioni/shadcn-ui
2 participants
@shadcn @jonioni