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.

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

feat(overlay): allow passing HTMLElement #4463

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

imdhemy
Copy link

@imdhemy imdhemy commented Mar 3, 2025

Description

Context:

We use a custom HTML element with a shadow root to render our application. This encapsulates our element from the document's DOM, preventing it from accessing main document elements.

Currently, the OverlayContainerProvider expects an element ID and retrieves it using document.getElementById(portalContainer). However, this approach fails in our use case, as the query always returns null due to shadow DOM encapsulation. Additionally, there is no body in our case, which further limits our ability to append overlays to the standard document structure.

Screenshot 2025-03-03 at 11 06 22 PM

This PR:

  • Adds support for passing an HTMLElement directly to the OverlayContainerProvider.

  • Preserves the existing behavior of accepting an element ID.

  • Enhances and fixes unit tests for OverlayContainerProvider.

  • This change requires a UI-Kit update - inform @tirado-rx @benjirsvx

What should be tested?

  • Render portal container by element ID.
  • Fallback the portal container to the body.
  • Render portal container by HTMLElement.

Are they some special informations required to test something?

Reviewers:

Which persons should review this PR? Add person with @mentions

Examples:
@marigold-ui/developer
@marigold-ui/designer

wip: pass html element

wip: add test for ssr case

wip: add test for the fallback

wip: fix unit test

wip: code refactoring
Copy link

changeset-bot bot commented Mar 3, 2025

⚠️ No Changeset found

Latest commit: 4998c7e

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

Copy link

vercel bot commented Mar 3, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
marigold-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 3, 2025 10:18pm

Copy link

vercel bot commented Mar 3, 2025

@imdhemy is attempting to deploy a commit to the Marigold Team on Vercel.

A member of the Team first needs to authorize it.

: portalContainer
? document.getElementById(portalContainer)
: document.body;
if (useIsSSR()) return null;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept this logic, but why isn't this available for SSR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant