Skip to content

Commit

Permalink
fix(toolbar): Fix tooltip & hovercard & menu react-portal rendering i…
Browse files Browse the repository at this point in the history
…nside the toolbar (#74797)

What you can barely see here is a css change in the before & after
shots.

In the before there's no css applied. After there is some css (but
z-index is too small, that's fixed in
37a40c6
in this PR)
What's happening?

We are importing a component that eventually calls, in the before case:
`createPortal(..., document.body)`. The trouble with that is where the
import happens. In the website codebase we import it and the whole react
tree, including emotion css, is inside of `document`.

But in this case we're importing it into toolbar code, so the react root
starts inside our `ShadowRoot` reference instead of `document.body`. All
our toolbar css is inside the shadowroot too, so when we use emotion and
generate a className like `sentry-devtools-b0afu4` nodes mounted at
`document` won't be able to access it.

**TL/DR:** the portal target inside the toolbar needs to match with this
snippet, using the same `container` value:
https://github.com/getsentry/sentry/blob/6bb4638c3f24b9d737b80730561748874f4a7558/static/app/components/devtoolbar/components/providers.tsx#L23-L34,
but only while we're developing inside sentry, and sharing components
fully like this.

**Before**

![SCR-20240723-nxfy](https://github.com/user-attachments/assets/f2690b8b-3315-4e19-a838-b4f178bfcf0f)

**After**

![SCR-20240723-nxek](https://github.com/user-attachments/assets/80f4e159-cadb-4c1d-8479-933cb978275d)
  • Loading branch information
ryan953 authored Jul 24, 2024
1 parent 2225851 commit 6be923a
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 20 deletions.
25 changes: 14 additions & 11 deletions static/app/components/devtoolbar/components/providers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import createCache from '@emotion/cache';
import {CacheProvider, ThemeProvider} from '@emotion/react';
import {QueryClient, QueryClientProvider} from '@tanstack/react-query';

import {ReactPortalTargetProvider} from 'sentry/components/react/useReactPortalTarget';
import {lightTheme} from 'sentry/utils/theme';

import {ConfigurationContextProvider} from '../hooks/useConfiguration';
Expand Down Expand Up @@ -33,16 +34,18 @@ export default function Providers({children, config, container}: Props) {
);

return (
<CacheProvider value={myCache}>
<ThemeProvider theme={lightTheme}>
<ConfigurationContextProvider config={config}>
<QueryClientProvider client={queryClient}>
<VisibilityContextProvider>
<ToolbarRouterContextProvider>{children}</ToolbarRouterContextProvider>
</VisibilityContextProvider>
</QueryClientProvider>
</ConfigurationContextProvider>
</ThemeProvider>
</CacheProvider>
<ReactPortalTargetProvider target={container}>
<CacheProvider value={myCache}>
<ThemeProvider theme={lightTheme}>
<ConfigurationContextProvider config={config}>
<QueryClientProvider client={queryClient}>
<VisibilityContextProvider>
<ToolbarRouterContextProvider>{children}</ToolbarRouterContextProvider>
</VisibilityContextProvider>
</QueryClientProvider>
</ConfigurationContextProvider>
</ThemeProvider>
</CacheProvider>
</ReactPortalTargetProvider>
);
}
2 changes: 1 addition & 1 deletion static/app/components/devtoolbar/styles/global.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export const globalCss = css`
--pink200: rgba(249, 26, 138, 0.5);
--pink100: rgba(249, 26, 138, 0.09);
--z-index: 100000;
--z-index: 10000;
color: var(--gray400);
font-family: system-ui, 'Helvetica Neue', Arial, sans-serif;
Expand Down
5 changes: 4 additions & 1 deletion static/app/components/dropdownMenu/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {Item, Section} from '@react-stately/collections';

import type {DropdownButtonProps} from 'sentry/components/dropdownButton';
import DropdownButton from 'sentry/components/dropdownButton';
import useReactPortalTarget from 'sentry/components/react/useReactPortalTarget';
import type {FormSize} from 'sentry/utils/theme';
import type {UseOverlayProps} from 'sentry/utils/useOverlay';
import useOverlay from 'sentry/utils/useOverlay';
Expand Down Expand Up @@ -153,6 +154,8 @@ function DropdownMenu({
}: DropdownMenuProps) {
const isDisabled = disabledProp ?? (!items || items.length === 0);

const portalTarget = useReactPortalTarget();

const {rootOverlayState} = useContext(DropdownMenuContext);
const {
isOpen,
Expand Down Expand Up @@ -248,7 +251,7 @@ function DropdownMenu({
</DropdownMenuList>
);

return usePortal ? createPortal(menu, document.body) : menu;
return usePortal ? createPortal(menu, portalTarget) : menu;
}

return (
Expand Down
5 changes: 4 additions & 1 deletion static/app/components/menuListItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import styled from '@emotion/styled';

import InteractionStateLayer from 'sentry/components/interactionStateLayer';
import {Overlay, PositionWrapper} from 'sentry/components/overlay';
import useReactPortalTarget from 'sentry/components/react/useReactPortalTarget';
import type {TooltipProps} from 'sentry/components/tooltip';
import {Tooltip} from 'sentry/components/tooltip';
import {space} from 'sentry/styles/space';
Expand Down Expand Up @@ -247,6 +248,8 @@ function DetailsOverlay({

const popper = usePopper(itemRef.current, overlayElement, POPPER_OPTIONS);

const portalTarget = useReactPortalTarget();

return createPortal(
<StyledPositionWrapper
{...popper.attributes.popper}
Expand All @@ -261,7 +264,7 @@ function DetailsOverlay({
// Safari will clip the overlay if it is inside a scrollable container, even though it is positioned fixed.
// See https://bugs.webkit.org/show_bug.cgi?id=160953
// To work around this, we append the overlay to the body
document.body
portalTarget
);
}

Expand Down
18 changes: 18 additions & 0 deletions static/app/components/react/useReactPortalTarget.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import {createContext, type ReactNode, useContext} from 'react';

const ReactPortalTarget = createContext<HTMLElement | ShadowRoot>(document.body);

interface Props {
children: ReactNode;
target: HTMLElement | ShadowRoot;
}

export function ReactPortalTargetProvider({children, target}: Props) {
return (
<ReactPortalTarget.Provider value={target}>{children}</ReactPortalTarget.Provider>
);
}

export default function useReactPortalTarget() {
return useContext(ReactPortalTarget);
}
12 changes: 6 additions & 6 deletions static/app/components/tooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ import styled from '@emotion/styled';
import {AnimatePresence} from 'framer-motion';

import {Overlay, PositionWrapper} from 'sentry/components/overlay';
import useReactPortalTarget from 'sentry/components/react/useReactPortalTarget';
import {space} from 'sentry/styles/space';
import type {UseHoverOverlayProps} from 'sentry/utils/useHoverOverlay';
import {useHoverOverlay} from 'sentry/utils/useHoverOverlay';

interface TooltipProps extends UseHoverOverlayProps {
export interface TooltipProps extends UseHoverOverlayProps {
/**
* The content to show in the tooltip popover
*/
Expand All @@ -26,7 +27,7 @@ interface TooltipProps extends UseHoverOverlayProps {
overlayStyle?: React.CSSProperties | SerializedStyles;
}

function Tooltip({
export function Tooltip({
children,
overlayStyle,
title,
Expand All @@ -44,6 +45,8 @@ function Tooltip({
}
}, [reset, disabled]);

const portalTarget = useReactPortalTarget();

if (disabled || !title) {
return <Fragment>{children}</Fragment>;
}
Expand All @@ -65,7 +68,7 @@ function Tooltip({
return (
<Fragment>
{wrapTrigger(children)}
{createPortal(<AnimatePresence>{tooltipContent}</AnimatePresence>, document.body)}
{createPortal(<AnimatePresence>{tooltipContent}</AnimatePresence>, portalTarget)}
</Fragment>
);
}
Expand All @@ -79,6 +82,3 @@ const TooltipContent = styled(Overlay)`
line-height: 1.2;
text-align: center;
`;

export type {TooltipProps};
export {Tooltip};

0 comments on commit 6be923a

Please sign in to comment.