-
Notifications
You must be signed in to change notification settings - Fork 543
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
Portal Hardcoded Z-Index Buries Components #4622
Comments
Uh oh! @ntrappe-msft, the image you shared is missing helpful alt text. Check your issue body. Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image. Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.
|
Hi! Thanks for taking the time to report this issue! For now, I would recommend I say that because long term, we are hoping to remove this batch of problems by using the popover API. We have a tracking issue open, but it's not prioritised yet: github/primer#2864 (private github repo) |
As suggested by @siddharthkp there can be a workaround using a custom portal root. Below is the snippet demonstrating the same. import { useEffect } from 'react';
import { registerPortalRoot } from '@primer/react';
const HeaderWrapper = () => {
useEffect(() => {
const portalRoot = document.getElementById("__primerPortalRoot__");
if (portalRoot) {
registerPortalRoot(portalRoot, "__primerPortalRoot__");
}
}, []);
return (
<>
<div id="__primerPortalRoot__"></div>
<div id="header-wrapper" style={{ position: 'fixed', zIndex: 50 }}>
<ActionMenu id="action-menu">
<ActionMenu.Button>Color Mode</ActionMenu.Button>
<ActionMenu.Overlay portalContainerName="__primerPortalRoot__">
<ActionList id="action-menu-list">
<ActionList.Item>Light Mode</ActionList.Item>
<ActionList.Item>Dark Mode</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>
</div>
</>
);
};
export default HeaderWrapper; Explanation:Add Portal Root Element: The |
Description
Hi, I wanted to bring up a few problems that arise from mixing Primer and non-Primer components. The problems emerge because the Primer Portal sets a div whose children will inherit a z-index of 1. This works fine if you're only working with Primer components that abide by this fake DOM hierarchy. But, if you start mixing in non-Primer components with their own z-indices, those Primer components will become buried.
Example
Let's say we created a header component from
<div>
and added anActionMenu
.On the screen below, you'll see the header as a blue bar with the button Color Mode. When we click on the button, an overlay pops up with the options for Light Mode and Dark Mode. The top half of the overlay is covered because the z-index of the header is higher than it.
If we query the div responsible for setting the z-index to 1 and set it instead to 100, you'll notice that the overlay is no longer covered but fully visible.
Problem
If you take a look at the DOM, you'll see the header component and a Portal component, with the ActionMenu attached to the Portal component. Because the header isn't a Primer component, it doesn't get nested in the Portal root so it doesn't agree to the same z-index hierarchy.
The only workaround I've found it to query the root for my React DOM, descend down the DOM to the div responsible, then overwrite it. It's pretty hacky and not great if you've got a number of components that you need to adjust.
Fixes
I'm open to hearing your thoughts and I'm curious about what you'd recommend as best practices when mixing Primer and non-Primer components or modifying z-indices. I've been debating on whether I should:
(1) Opt out of using non-Primer components to avoid this (e.g., just use
Box
).(2) Put non-Primer components in the same React Portal.
(3) Keep using the hacky way of overwriting the z-index.
Steps to reproduce
(More examples of how to reproduce included in the description above).
z-index
to more than 1.Overlay
orActionMenu
as a child.Version
v36.19.0
Browser
Safari
The text was updated successfully, but these errors were encountered: