Make top and left position explicit props of Overlay handled separate…

…ly from other styles (#1385) * Make top and left position explicit props of Overlay handled separately from other styles * Update overlay documentation and make top/left non-requred * changeset Co-authored-by: Cole Bemis <colebemis@github.com>
primer · Aug 31, 2021 · 5470b61 · 5470b61
1 parent 511bf71
commit 5470b61
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 10 deletions.
diff --git a/.changeset/good-zebras-swim.md b/.changeset/good-zebras-swim.md
@@ -0,0 +1,5 @@
+---
+'@primer/components': minor
+---
+
+Make top and left position explicit props of Overlay handled separately from other styles
diff --git a/docs/content/Overlay.mdx b/docs/content/Overlay.mdx
@@ -85,3 +85,5 @@ System props are deprecated in all components except [Box](/Box). Please use the
 | height          | `'xsmall', 'small', 'medium', 'large', 'xlarge', 'auto'`       |   `auto`    | Sets the height of the `Overlay`, pick from our set list of heights, or pass `auto` to automatically set the height based on the content of the `Overlay`. `xsmall` corresponds to `192px`, `small` corresponds to `256px`, `medium` corresponds to `320px`, `large` corresponds to `432px`, `xlarge` corresponds to `600px`. |
 | visibility      | `'visible', 'hidden'`                                          |  `visible`  | Sets the visibility of the `Overlay`.                                                                                                                                                                                                                                                                                         |
 | anchorSide      | `AnchorSide`                                                   |  undefined  | Optional. If provided, the Overlay will slide into position from the side of the anchor with a brief animation                                                                                                                                                                                                                |
+| top             | `number`                                                       |      0      | Vertical position of the overlay, relative to its closest positioned ancestor (often its `Portal`).                                                                                                                                                                                                                           |
+| left            | `number`                                                       |      0      | Horizontal position of the overlay, relative to its closest positioned ancestor (often its `Portal`).                                                                                                                                                                                                                         |
diff --git a/src/AnchoredOverlay/AnchoredOverlay.tsx b/src/AnchoredOverlay/AnchoredOverlay.tsx
@@ -127,9 +127,6 @@ export const AnchoredOverlay: React.FC<AnchoredOverlayProps> = ({
     },
     [overlayRef.current]
   )
-  const overlayPosition = useMemo(() => {
-    return position && {style: {top: `${position.top}px`, left: `${position.left}px`}, anchorSide: position.anchorSide}
-  }, [position])
 
   useEffect(() => {
     // ensure overlay ref gets cleared when closed, so position can reset between closing/re-opening
@@ -168,7 +165,9 @@ export const AnchoredOverlay: React.FC<AnchoredOverlayProps> = ({
           visibility={position ? 'visible' : 'hidden'}
           height={height}
           width={width}
-          {...overlayPosition}
+          top={position?.top || 0}
+          left={position?.left || 0}
+          anchorSide={position?.anchorSide}
           {...overlayProps}
         >
           {children}

diff --git a/src/Overlay.tsx b/src/Overlay.tsx
@@ -1,6 +1,6 @@
 import styled from 'styled-components'
 import React, {ReactElement, useEffect, useLayoutEffect, useRef} from 'react'
-import {get, COMMON, POSITION, SystemPositionProps, SystemCommonProps} from './constants'
+import {get, COMMON, SystemPositionProps, SystemCommonProps} from './constants'
 import {ComponentProps} from './utils/types'
 import {useOverlay, TouchOrMouseEvent} from './hooks'
 import Portal from './Portal'
@@ -51,7 +51,7 @@ function getSlideAnimationStartingVector(anchorSide?: AnchorSide): {x: number; y
   return {x: 0, y: 0}
 }
 
-const StyledOverlay = styled.div<StyledOverlayProps & SystemCommonProps & SystemPositionProps & SxProp>`
+const StyledOverlay = styled.div<StyledOverlayProps & SystemCommonProps & SxProp>`
   background-color: ${get('colors.bg.overlay')};
   box-shadow: ${get('shadows.overlay.shadow')};
   position: absolute;
@@ -77,7 +77,6 @@ const StyledOverlay = styled.div<StyledOverlayProps & SystemCommonProps & System
     outline: none;
   }
   ${COMMON};
-  ${POSITION};
   ${sx};
 `
 export type OverlayProps = {
@@ -88,6 +87,8 @@ export type OverlayProps = {
   onEscape: (e: KeyboardEvent) => void
   visibility?: 'visible' | 'hidden'
   [additionalKey: string]: unknown
+  top: number
+  left: number
 } & Omit<ComponentProps<typeof StyledOverlay>, 'visibility' | keyof SystemPositionProps>
 
 /**
@@ -103,6 +104,8 @@ export type OverlayProps = {
  * @param height Sets the height of the `Overlay`, pick from our set list of heights, or pass `auto` to automatically set the height based on the content of the `Overlay`, or pass `initial` to set the height based on the initial content of the `Overlay` (i.e. ignoring content changes). `xsmall` corresponds to `192px`, `small` corresponds to `256px`, `medium` corresponds to `320px`, `large` corresponds to `432px`, `xlarge` corresponds to `600px`.
  * @param maxHeight Sets the maximum height of the `Overlay`, pick from our set list of heights. `xsmall` corresponds to `192px`, `small` corresponds to `256px`, `medium` corresponds to `320px`, `large` corresponds to `432px`, `xlarge` corresponds to `600px`.
  * @param anchorSide If provided, the Overlay will slide into position from the side of the anchor with a brief animation
+ * @param top Optional. Vertical position of the overlay, relative to its closest positioned ancestor (often its `Portal`).
+ * @param left Optional. Horizontal position of the overlay, relative to its closest positioned ancestor (often its `Portal`).
  */
 const Overlay = React.forwardRef<HTMLDivElement, OverlayProps>(
   (
@@ -115,6 +118,8 @@ const Overlay = React.forwardRef<HTMLDivElement, OverlayProps>(
       onEscape,
       visibility = 'visible',
       height,
+      top,
+      left,
       anchorSide,
       ...rest
     },
@@ -166,6 +171,8 @@ const Overlay = React.forwardRef<HTMLDivElement, OverlayProps>(
           ref={combinedRef}
           style={
             {
+              top: `${top || 0}px`,
+              left: `${left || 0}px`,
               ...rest.style,
               '--styled-overlay-visibility': visibility
             } as React.CSSProperties

diff --git a/src/__tests__/__snapshots__/AnchoredOverlay.tsx.snap b/src/__tests__/__snapshots__/AnchoredOverlay.tsx.snap
@@ -215,7 +215,7 @@ Object {
               data-focus-trap="active"
               height="auto"
               role="none"
-              style="--styled-overlay-visibility: visible; top: 4px; left: 0px;"
+              style="top: 4px; left: 0px; --styled-overlay-visibility: visible;"
               width="auto"
             >
               <button
@@ -255,11 +255,11 @@ Object {
           style="position: relative; z-index: 1;"
         >
           <div
-            class="Overlay__StyledOverlay-jhwkzw-0 kVoHwu"
+            class="Overlay__StyledOverlay-jhwkzw-0 cWgXnb"
             data-focus-trap="active"
             height="auto"
             role="none"
-            style="--styled-overlay-visibility: visible; top: 4px; left: 0px;"
+            style="top: 4px; left: 0px; --styled-overlay-visibility: visible;"
             width="auto"
           >
             <button