-
Notifications
You must be signed in to change notification settings - Fork 0
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(ui): update Portal #465
Conversation
WiP: render portal root and individual containers for portals, leave interface intact
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This component is kinda blocking us for typescript migration cause staff that used there, also used in many other spots. If you do not mind I would easily transfer it to typescript as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have to move on and make this as a PR, would be nice to transfer it to TS as soon as possible if you need any support please let me know.
packages/ui-components/src/components/PortalProvider/PortalProvider.component.js
Show resolved
Hide resolved
packages/ui-components/src/components/PortalProvider/PortalProvider.component.js
Show resolved
Hide resolved
packages/ui-components/src/components/PortalProvider/PortalProvider.test.js
Outdated
Show resolved
Hide resolved
packages/ui-components/src/components/PortalProvider/PortalProvider.test.js
Outdated
Show resolved
Hide resolved
packages/ui-components/src/components/PortalProvider/PortalProvider.test.js
Outdated
Show resolved
Hide resolved
packages/ui-components/src/components/PortalProvider/PortalProvider.test.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
feat(ui): update Portal
Make our portals more robust and isolate them against each other. Avoid conflicts and make positioning portaled content more robust.
We need this as a basis to migrate more components to use the portal (
ContextMenu
,DateTimePicker
), so we can then address positioning issues we currently have on a common basis.This PR introduces individual parent divs per each individual portal in a parent root element that is rendered by
PortalProvider
:Users can use Portals in the same way as before, i.e. either as a
<PortalProvider.Portal />
component, or by calling theusePortalRef
hook and create a portal in their components manually that mounts at the node returned by the hook.AppShell makes sure PortalProvider is rendered in the correct place of the component tree so that all elements rendered into portals have access to the Juno styles.
PortalProvider
also exports aPortalContext
that contains a ref to the portal root node as its sole value.Changes Made
PortalProvider
to render a portal root element in our apps.PortalProvider.Portal
component andusePortalRef hook
to wrap portaled content into individual divsRelated Issues
Screenshots (if applicable)
Testing Instructions
npm i
npm run TASK
Checklist