From 5470b61b448aeb29e859766ca0ff7f9f8457c690 Mon Sep 17 00:00:00 2001 From: Jonathan Fuchs Date: Tue, 31 Aug 2021 10:16:03 -0700 Subject: [PATCH] Make top and left position explicit props of Overlay handled separately 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 --- .changeset/good-zebras-swim.md | 5 +++++ docs/content/Overlay.mdx | 2 ++ src/AnchoredOverlay/AnchoredOverlay.tsx | 7 +++---- src/Overlay.tsx | 13 ++++++++++--- .../__snapshots__/AnchoredOverlay.tsx.snap | 6 +++--- 5 files changed, 23 insertions(+), 10 deletions(-) create mode 100644 .changeset/good-zebras-swim.md diff --git a/.changeset/good-zebras-swim.md b/.changeset/good-zebras-swim.md new file mode 100644 index 00000000000..6a74e1bab95 --- /dev/null +++ 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 index f64d38b8b15..c4322f8ae43 100644 --- 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 index 3ced5ed844f..974014dd627 100644 --- a/src/AnchoredOverlay/AnchoredOverlay.tsx +++ b/src/AnchoredOverlay/AnchoredOverlay.tsx @@ -127,9 +127,6 @@ export const AnchoredOverlay: React.FC = ({ }, [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 = ({ 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 index d9806db84e6..125bddb01af 100644 --- 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` +const StyledOverlay = styled.div` background-color: ${get('colors.bg.overlay')}; box-shadow: ${get('shadows.overlay.shadow')}; position: absolute; @@ -77,7 +77,6 @@ const StyledOverlay = styled.div void visibility?: 'visible' | 'hidden' [additionalKey: string]: unknown + top: number + left: number } & Omit, '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( ( @@ -115,6 +118,8 @@ const Overlay = React.forwardRef( onEscape, visibility = 'visible', height, + top, + left, anchorSide, ...rest }, @@ -166,6 +171,8 @@ const Overlay = React.forwardRef( 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 index 0d3fed86f1d..c98f4d4583d 100644 --- 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" >