LG-4121: Fix Safari regressions and refactor Popover (#2549)

* Drop usePortal usage in modal story and spec * Update @floating-ui/react dependency * Address Safari regressions and refactor Popover
mongodb · Nov 26, 2024 · 24578ce · 24578ce
1 parent 732fae5
commit 24578ce
Show file tree

Hide file tree

Showing 10 changed files with 144 additions and 176 deletions.
diff --git a/packages/leafygreen-provider/src/PopoverPropsContext/PopoverPropsContext.tsx b/packages/leafygreen-provider/src/PopoverPropsContext/PopoverPropsContext.tsx
@@ -1,7 +1,10 @@
 import React, { createContext, PropsWithChildren, useContext } from 'react';
 import PropTypes from 'prop-types';
 
-import { PortalContextProvider } from '../PortalContext';
+import {
+  PortalContextProvider,
+  usePopoverPortalContainer,
+} from '../PortalContext';
 
 import { PopoverPropsProviderProps } from './PopoverPropsContext.types';
 
@@ -23,9 +26,12 @@ export const PopoverPropsProvider = ({
   children,
   ...props
 }: PropsWithChildren<PopoverPropsProviderProps>) => {
+  const popoverPortalContext = usePopoverPortalContainer();
   const popover = {
-    portalContainer: props.portalContainer,
-    scrollContainer: props.scrollContainer,
+    portalContainer:
+      props.portalContainer || popoverPortalContext.portalContainer,
+    scrollContainer:
+      props.scrollContainer || popoverPortalContext.scrollContainer,
   };
 
   return (

diff --git a/packages/modal/src/Modal.stories.tsx b/packages/modal/src/Modal.stories.tsx
@@ -156,7 +156,6 @@ export const DefaultSelect = (args: ModalProps) => {
               name="pets"
               value={value}
               onChange={setValue}
-              usePortal={true}
             >
               <OptionGroup label="Common">
                 <Option value="dog">Dog</Option>

diff --git a/packages/modal/src/Modal/Modal.spec.tsx b/packages/modal/src/Modal/Modal.spec.tsx
@@ -139,7 +139,6 @@ describe('packages/modal', () => {
             size="small"
             placeholder="animals"
             name="pets"
-            usePortal={true}
             data-testid="modal-select-test-id"
           >
             <OptionGroup label="Common">

diff --git a/packages/popover/package.json b/packages/popover/package.json
@@ -22,7 +22,7 @@
     "access": "public"
   },
   "dependencies": {
-    "@floating-ui/react": "^0.26.23",
+    "@floating-ui/react": "^0.26.28",
     "@leafygreen-ui/emotion": "^4.0.8",
     "@leafygreen-ui/hooks": "^8.1.3",
     "@leafygreen-ui/lib": "^13.5.0",

diff --git a/packages/popover/src/Popover.stories.tsx b/packages/popover/src/Popover.stories.tsx
@@ -25,7 +25,6 @@ const popoverStyle = css`
   border: 1px solid ${palette.gray.light1};
   text-align: center;
   padding: 12px;
-  max-width: 200px;
   max-height: 100%;
   overflow: hidden;
   // Reset these properties since they'll be inherited
@@ -61,32 +60,6 @@ const scrollableInnerStyles = css`
   justify-content: center;
 `;
 
-const buttonStyles = css`
-  position: relative;
-`;
-
-const referenceElPositions: { [key: string]: string } = {
-  centered: css`
-    position: relative;
-  `,
-  top: css`
-    top: 0;
-    position: absolute;
-  `,
-  right: css`
-    right: 0;
-    position: absolute;
-  `,
-  bottom: css`
-    bottom: 0;
-    position: absolute;
-  `,
-  left: css`
-    left: 0;
-    position: absolute;
-  `,
-};
-
 const defaultExcludedControls = [
   ...storybookExcludedControlParams,
   'active',
@@ -175,13 +148,6 @@ const meta: StoryMetaType<typeof Popover> = {
       options: Object.values(Justify),
       control: { type: 'radio' },
     },
-    refButtonPosition: {
-      options: ['centered', 'top', 'right', 'bottom', 'left'],
-      control: { type: 'select' },
-      description:
-        'Storybook only prop. Used to change position of the reference button',
-      defaultValue: 'centered',
-    },
     renderMode: {
       options: Object.values(RenderMode),
       control: { type: 'radio' },
@@ -192,10 +158,8 @@ export default meta;
 
 type PopoverStoryProps = PopoverProps & {
   buttonText: string;
-  refButtonPosition: string;
 };
 export const LiveExample: StoryFn<PopoverStoryProps> = ({
-  refButtonPosition,
   buttonText,
   ...props
 }: PopoverStoryProps) => {
@@ -212,8 +176,6 @@ export const LiveExample: StoryFn<PopoverStoryProps> = ({
   const buttonRef = useRef<HTMLButtonElement | null>(null);
   const [active, setActive] = useState<boolean>(false);
 
-  const position = referenceElPositions[refButtonPosition];
-
   const handleClick = () => {
     setActive(active => !active);
   };
@@ -241,11 +203,7 @@ export const LiveExample: StoryFn<PopoverStoryProps> = ({
 
   return (
     <div className={containerStyles}>
-      <Button
-        className={cx(buttonStyles, position)}
-        onClick={handleClick}
-        ref={buttonRef}
-      >
+      <Button onClick={handleClick} ref={buttonRef}>
         {buttonText}
       </Button>
       <Popover {...popoverProps}>
@@ -261,7 +219,6 @@ LiveExample.parameters = {
 };
 
 const PortalPopoverInScrollableContainer = ({
-  refButtonPosition,
   buttonText,
   ...props
 }: PopoverStoryProps) => {
@@ -270,15 +227,10 @@ const PortalPopoverInScrollableContainer = ({
   const portalRef = useRef<HTMLElement | null>(null);
   const scrollContainer = useRef<HTMLDivElement | null>(null);
 
-  const position = referenceElPositions[refButtonPosition];
-
   return (
     <div className={scrollableOuterStyles}>
       <div className={scrollableInnerStyles} ref={scrollContainer}>
-        <Button
-          onClick={() => setActive(active => !active)}
-          className={position}
-        >
+        <Button onClick={() => setActive(active => !active)}>
           {buttonText}
           <Popover
             {...rest}
@@ -314,11 +266,7 @@ export const RenderModePortalInScrollableContainer = {
   },
 };
 
-const InlinePopover = ({
-  refButtonPosition,
-  buttonText,
-  ...props
-}: PopoverStoryProps) => {
+const InlinePopover = ({ buttonText, ...props }: PopoverStoryProps) => {
   const {
     dismissMode,
     onToggle,
@@ -332,15 +280,9 @@ const InlinePopover = ({
   const buttonRef = useRef<HTMLButtonElement | null>(null);
   const [active, setActive] = useState<boolean>(false);
 
-  const position = referenceElPositions[refButtonPosition];
-
   return (
     <div className={containerStyles}>
-      <Button
-        className={cx(buttonStyles, position)}
-        onClick={() => setActive(active => !active)}
-        ref={buttonRef}
-      >
+      <Button onClick={() => setActive(active => !active)} ref={buttonRef}>
         {buttonText}
       </Button>
       <Popover

diff --git a/packages/popover/src/Popover/Popover.hooks.tsx b/packages/popover/src/Popover/Popover.hooks.tsx
@@ -58,7 +58,7 @@ export function usePopoverProps({
 
   const renderMode = forceUseTopLayer
     ? RenderMode.TopLayer
-    : renderModeProp || context.renderMode || RenderMode.TopLayer;
+    : renderModeProp || context.renderMode;
   const usePortal = renderMode === RenderMode.Portal;
   const useTopLayer = renderMode === RenderMode.TopLayer;
 
@@ -123,7 +123,8 @@ export function useReferenceElement(
   refEl?: PopoverProps['refEl'],
   scrollContainer?: PopoverProps['scrollContainer'],
 ): UseReferenceElementReturnObj {
-  const placeholderRef = useRef<HTMLSpanElement | null>(null);
+  const [placeholderElement, setPlaceholderElement] =
+    useState<HTMLSpanElement | null>(null);
   const [referenceElement, setReferenceElement] = useState<HTMLElement | null>(
     null,
   );
@@ -134,14 +135,14 @@ export function useReferenceElement(
       return;
     }
 
-    const placeholderEl = placeholderRef?.current;
-    const maybeParentEl = placeholderEl !== null && placeholderEl?.parentNode;
+    const maybeParentEl =
+      placeholderElement !== null && placeholderElement.parentNode;
 
     if (maybeParentEl && maybeParentEl instanceof HTMLElement) {
       setReferenceElement(maybeParentEl);
       return;
     }
-  }, [placeholderRef.current, refEl]);
+  }, [placeholderElement, refEl]);
 
   const referenceElDocumentPos = useObjectDependency(
     useMemo(
@@ -151,9 +152,9 @@ export function useReferenceElement(
   );
 
   return {
-    placeholderRef,
     referenceElement,
     referenceElDocumentPos,
+    setPlaceholderElement,
   };
 }
 

diff --git a/packages/popover/src/Popover/Popover.styles.ts b/packages/popover/src/Popover/Popover.styles.ts
@@ -15,16 +15,13 @@ export const hiddenPlaceholderStyle = css`
   display: none;
 `;
 
-const getBasePopoverStyles = (floatingStyles: React.CSSProperties) => css`
+const basePopoverStyles = css`
   margin: 0;
   border: none;
   padding: 0;
   overflow: visible;
   background-color: transparent;
-
-  position: ${floatingStyles.position};
-  top: ${floatingStyles.top}px;
-  left: ${floatingStyles.left}px;
+  width: max-content;
 
   transition-property: opacity, transform, overlay, display;
   transition-duration: ${TRANSITION_DURATION}ms;
@@ -49,6 +46,20 @@ const getBasePopoverStyles = (floatingStyles: React.CSSProperties) => css`
   }
 `;
 
+const getPositionStyles = ({
+  left,
+  position,
+  top,
+}: {
+  left: number;
+  position: 'absolute' | 'fixed';
+  top: number;
+}) => css`
+  left: ${left}px;
+  position: ${position};
+  top: ${top}px;
+`;
+
 const transformOriginStyles: Record<ExtendedPlacement, string> = {
   top: css`
     transform-origin: bottom;
@@ -107,28 +118,32 @@ const getClosedStyles = (spacing: number, transformAlign: TransformAlign) => {
       return cx(
         baseClosedStyles,
         css`
-          transform: translateY(${spacing}px) scale(${TRANSFORM_INITIAL_SCALE});
+          transform: translate3d(0, ${spacing}px, 0)
+            scale(${TRANSFORM_INITIAL_SCALE});
         `,
       );
     case TransformAlign.Bottom:
       return cx(
         baseClosedStyles,
         css`
-          transform: translateY(-${spacing}px) scale(${TRANSFORM_INITIAL_SCALE});
+          transform: translate3d(0, -${spacing}px, 0)
+            scale(${TRANSFORM_INITIAL_SCALE});
         `,
       );
     case TransformAlign.Left:
       return cx(
         baseClosedStyles,
         css`
-          transform: translateX(${spacing}px) scale(${TRANSFORM_INITIAL_SCALE});
+          transform: translate3d(${spacing}px, 0, 0)
+            scale(${TRANSFORM_INITIAL_SCALE});
         `,
       );
     case TransformAlign.Right:
       return cx(
         baseClosedStyles,
         css`
-          transform: translateX(-${spacing}px) scale(${TRANSFORM_INITIAL_SCALE});
+          transform: translate3d(-${spacing}px, 0, 0)
+            scale(${TRANSFORM_INITIAL_SCALE});
         `,
       );
     case TransformAlign.Center:
@@ -196,23 +211,28 @@ const getOpenStyles = (transformAlign: TransformAlign) => {
 
 export const getPopoverStyles = ({
   className,
-  floatingStyles,
+  left,
   placement,
   popoverZIndex,
+  position,
   spacing,
   state,
+  top,
   transformAlign,
 }: {
   className?: string;
-  floatingStyles: React.CSSProperties;
+  left: number;
   placement: ExtendedPlacement;
   popoverZIndex?: number;
+  position: 'absolute' | 'fixed';
   spacing: number;
   state: TransitionStatus;
+  top: number;
   transformAlign: TransformAlign;
 }) =>
   cx(
-    getBasePopoverStyles(floatingStyles),
+    basePopoverStyles,
+    getPositionStyles({ left, position, top }),
     transformOriginStyles[placement],
     {
       [getClosedStyles(spacing, transformAlign)]: state !== 'entered',