From eb298ad5d7cbacfb9b26d77cdf01d54ebf420f53 Mon Sep 17 00:00:00 2001 From: Clay Miller Date: Thu, 10 Jun 2021 14:22:05 -0400 Subject: [PATCH 1/6] feat: Add 'height: initial' to 'Overlay' --- src/Overlay.tsx | 33 ++++++++++++--- src/stories/SelectPanel.stories.tsx | 63 +++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 6 deletions(-) diff --git a/src/Overlay.tsx b/src/Overlay.tsx index 47f675bcdd9..692de68ca00 100644 --- a/src/Overlay.tsx +++ b/src/Overlay.tsx @@ -1,5 +1,5 @@ import styled from 'styled-components' -import React, {ReactElement, useRef} from 'react' +import React, {ReactElement, useEffect, useRef} from 'react' import {get, COMMON, POSITION, SystemPositionProps, SystemCommonProps} from './constants' import {ComponentProps} from './utils/types' import {useOverlay, TouchOrMouseEvent} from './hooks' @@ -9,7 +9,7 @@ import {useCombinedRefs} from './hooks/useCombinedRefs' type StyledOverlayProps = { width?: keyof typeof widthMap - height?: keyof typeof heightMap + height?: string visibility?: 'visible' | 'hidden' } @@ -38,7 +38,7 @@ const StyledOverlay = styled.div heightMap[props.height || 'auto']}; + height: ${props => props.height}; width: ${props => widthMap[props.width || 'auto']}; border-radius: 12px; overflow: hidden; @@ -68,8 +68,9 @@ export type OverlayProps = { onClickOutside: (e: TouchOrMouseEvent) => void onEscape: (e: KeyboardEvent) => void visibility?: 'visible' | 'hidden' + height?: keyof typeof heightMap | 'initial' [additionalKey: string]: unknown -} & Omit, 'visibility' | keyof SystemPositionProps> +} & Omit, 'height' | 'visibility' | keyof SystemPositionProps> /** * An `Overlay` is a flexible floating surface, used to display transient content such as menus, @@ -81,12 +82,22 @@ export type OverlayProps = { * @param onClickOutside Required. Function to call when clicking outside of the `Overlay`. Typically this function sets the `Overlay` visibility state to `false`. * @param onEscape Required. Function to call when user presses `Escape`. Typically this function sets the `Overlay` visibility state to `false`. * @param width Sets the width of the `Overlay`, pick from our set list of widths, or pass `auto` to automatically set the width based on the content of the `Overlay`. `small` corresponds to `256px`, `medium` corresponds to `320px`, `large` corresponds to `480px`, `xlarge` corresponds to `640px`, `xxlarge` corresponds to `960px`. - * @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`. `xsmall` corresponds to `192px`, `small` corresponds to `256px`, `medium` corresponds to `320px`, `large` corresponds to `432px`, `xlarge` corresponds to `600px`. + * @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 visibility Sets the visibility of the `Overlay` */ const Overlay = React.forwardRef( ( - {onClickOutside, role = 'dialog', initialFocusRef, returnFocusRef, ignoreClickRefs, onEscape, visibility, ...rest}, + { + onClickOutside, + role = 'dialog', + initialFocusRef, + returnFocusRef, + ignoreClickRefs, + onEscape, + visibility, + height: heightKey, + ...rest + }, forwardedRef ): ReactElement => { const overlayRef = useRef(null) @@ -100,12 +111,22 @@ const Overlay = React.forwardRef( onClickOutside, initialFocusRef }) + + const initialHeight = useRef('auto') + useEffect(() => { + if (overlayRef.current?.clientHeight) { + initialHeight.current = `${overlayRef.current.clientHeight}px` + } + }, [overlayRef]) + const height = heightKey === 'initial' ? initialHeight.current : heightMap[heightKey || 'auto'] + return ( (items[0]) + const [filter, setFilter] = React.useState('') + const filteredItems = items.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase())) + const [open, setOpen] = useState(false) + + return ( + <> +

Single Select Panel

+
Please select a label that describe your issue:
+ ( + + {children ?? 'Select Labels'} + + )} + placeholderText="Filter Labels" + open={open} + onOpenChange={setOpen} + items={filteredItems} + selected={selected} + onSelectedChange={setSelected} + onFilterChange={setFilter} + showItemDividers={true} + overlayProps={{width: 'small', height: 'initial', sx: {maxHeight: '192px'}}} + /> + + ) +} +SelectPanelHeightInitialWithOverflowingItemsStory.storyName = 'SelectPanel, Height: Initial, Overflowing Items' + +export function SelectPanelHeightInitialWithUnderflowingItemsStory(): JSX.Element { + const underflowingItems = [items[0], items[1]] + const [selected, setSelected] = React.useState(underflowingItems[0]) + const [filter, setFilter] = React.useState('') + const filteredItems = underflowingItems.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase())) + const [open, setOpen] = useState(false) + + return ( + <> +

Single Select Panel

+
Please select a label that describe your issue:
+ ( + + {children ?? 'Select Labels'} + + )} + placeholderText="Filter Labels" + open={open} + onOpenChange={setOpen} + items={filteredItems} + selected={selected} + onSelectedChange={setSelected} + onFilterChange={setFilter} + showItemDividers={true} + overlayProps={{width: 'small', height: 'initial', sx: {maxHeight: '192px'}}} + /> + + ) +} +SelectPanelHeightInitialWithUnderflowingItemsStory.storyName = 'SelectPanel, Height: Initial, Underflowing Items' From c54c7e5b8e025983dd8190de9d50d356ab55e924 Mon Sep 17 00:00:00 2001 From: Clay Miller Date: Thu, 10 Jun 2021 17:15:19 -0400 Subject: [PATCH 2/6] fix: Use 'combinedRef'. Omit overriden 'overlayRef'. Imperatively set height. --- src/Overlay.tsx | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Overlay.tsx b/src/Overlay.tsx index 692de68ca00..eee527a96a4 100644 --- a/src/Overlay.tsx +++ b/src/Overlay.tsx @@ -19,7 +19,8 @@ const heightMap = { medium: '320px', large: '432px', xlarge: '600px', - auto: 'auto' + auto: 'auto', + initial: 'auto' // Passing 'initial' initially applies 'auto' } const widthMap = { @@ -68,7 +69,7 @@ export type OverlayProps = { onClickOutside: (e: TouchOrMouseEvent) => void onEscape: (e: KeyboardEvent) => void visibility?: 'visible' | 'hidden' - height?: keyof typeof heightMap | 'initial' + height?: keyof typeof heightMap [additionalKey: string]: unknown } & Omit, 'height' | 'visibility' | keyof SystemPositionProps> @@ -103,7 +104,7 @@ const Overlay = React.forwardRef( const overlayRef = useRef(null) const combinedRef = useCombinedRefs(overlayRef, forwardedRef) - const overlayProps = useOverlay({ + useOverlay({ overlayRef, returnFocusRef, onEscape, @@ -112,18 +113,16 @@ const Overlay = React.forwardRef( initialFocusRef }) - const initialHeight = useRef('auto') useEffect(() => { - if (overlayRef.current?.clientHeight) { - initialHeight.current = `${overlayRef.current.clientHeight}px` + if (heightKey === 'initial' && combinedRef.current?.clientHeight) { + combinedRef.current.style.height = `${combinedRef.current.clientHeight}px` } - }, [overlayRef]) - const height = heightKey === 'initial' ? initialHeight.current : heightMap[heightKey || 'auto'] + }, [heightKey, combinedRef]) + const height = heightMap[heightKey || 'auto'] return ( Date: Thu, 10 Jun 2021 19:59:00 -0400 Subject: [PATCH 3/6] fix: Imperatively-setting height allows us to realign the internal and external contract; 'height' is not widened to 'string' in the former, because arbitrary pixel values are no longer passed via props. --- src/Overlay.tsx | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/Overlay.tsx b/src/Overlay.tsx index eee527a96a4..b7f9cb4c65e 100644 --- a/src/Overlay.tsx +++ b/src/Overlay.tsx @@ -9,7 +9,7 @@ import {useCombinedRefs} from './hooks/useCombinedRefs' type StyledOverlayProps = { width?: keyof typeof widthMap - height?: string + height?: keyof typeof heightMap visibility?: 'visible' | 'hidden' } @@ -39,7 +39,7 @@ const StyledOverlay = styled.div props.height}; + height: ${props => heightMap[props.height || 'auto']}; width: ${props => widthMap[props.width || 'auto']}; border-radius: 12px; overflow: hidden; @@ -69,9 +69,8 @@ export type OverlayProps = { onClickOutside: (e: TouchOrMouseEvent) => void onEscape: (e: KeyboardEvent) => void visibility?: 'visible' | 'hidden' - height?: keyof typeof heightMap [additionalKey: string]: unknown -} & Omit, 'height' | 'visibility' | keyof SystemPositionProps> +} & Omit, 'visibility' | keyof SystemPositionProps> /** * An `Overlay` is a flexible floating surface, used to display transient content such as menus, @@ -96,7 +95,7 @@ const Overlay = React.forwardRef( ignoreClickRefs, onEscape, visibility, - height: heightKey, + height, ...rest }, forwardedRef @@ -114,11 +113,10 @@ const Overlay = React.forwardRef( }) useEffect(() => { - if (heightKey === 'initial' && combinedRef.current?.clientHeight) { + if (height === 'initial' && combinedRef.current?.clientHeight) { combinedRef.current.style.height = `${combinedRef.current.clientHeight}px` } - }, [heightKey, combinedRef]) - const height = heightMap[heightKey || 'auto'] + }, [height, combinedRef]) return ( From 16d5568e9cddaa5257337df1aa30207d5db4473d Mon Sep 17 00:00:00 2001 From: Clay Miller Date: Thu, 10 Jun 2021 20:16:58 -0400 Subject: [PATCH 4/6] feat: Raise breakpointed 'maxHeight' to 'OverlayProps' --- src/Overlay.tsx | 2 ++ src/stories/SelectPanel.stories.tsx | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Overlay.tsx b/src/Overlay.tsx index b7f9cb4c65e..b8f732b018a 100644 --- a/src/Overlay.tsx +++ b/src/Overlay.tsx @@ -10,6 +10,7 @@ import {useCombinedRefs} from './hooks/useCombinedRefs' type StyledOverlayProps = { width?: keyof typeof widthMap height?: keyof typeof heightMap + maxHeight?: keyof typeof heightMap visibility?: 'visible' | 'hidden' } @@ -40,6 +41,7 @@ const StyledOverlay = styled.div heightMap[props.height || 'auto']}; + max-height: ${props => props.maxHeight && heightMap[props.maxHeight]}; width: ${props => widthMap[props.width || 'auto']}; border-radius: 12px; overflow: hidden; diff --git a/src/stories/SelectPanel.stories.tsx b/src/stories/SelectPanel.stories.tsx index d0d20479535..17abac4ee69 100644 --- a/src/stories/SelectPanel.stories.tsx +++ b/src/stories/SelectPanel.stories.tsx @@ -131,7 +131,7 @@ export function SelectPanelHeightInitialWithOverflowingItemsStory(): JSX.Element onSelectedChange={setSelected} onFilterChange={setFilter} showItemDividers={true} - overlayProps={{width: 'small', height: 'initial', sx: {maxHeight: '192px'}}} + overlayProps={{width: 'small', height: 'initial', maxHeight: 'xsmall'}} /> ) @@ -163,7 +163,7 @@ export function SelectPanelHeightInitialWithUnderflowingItemsStory(): JSX.Elemen onSelectedChange={setSelected} onFilterChange={setFilter} showItemDividers={true} - overlayProps={{width: 'small', height: 'initial', sx: {maxHeight: '192px'}}} + overlayProps={{width: 'small', height: 'initial', maxHeight: 'xsmall'}} /> ) From 010c82ef6e38c63b2406e05079ac1b9ba3860cec Mon Sep 17 00:00:00 2001 From: Clay Miller Date: Fri, 11 Jun 2021 10:56:03 -0400 Subject: [PATCH 5/6] fix: Omit 'auto' and 'initial' from acceptable 'maxHeight' values --- src/Overlay.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Overlay.tsx b/src/Overlay.tsx index b8f732b018a..2f84c57137c 100644 --- a/src/Overlay.tsx +++ b/src/Overlay.tsx @@ -10,7 +10,7 @@ import {useCombinedRefs} from './hooks/useCombinedRefs' type StyledOverlayProps = { width?: keyof typeof widthMap height?: keyof typeof heightMap - maxHeight?: keyof typeof heightMap + maxHeight?: keyof Omit visibility?: 'visible' | 'hidden' } From 286c756766fdeb236c1f41da0fd250bbd02e245b Mon Sep 17 00:00:00 2001 From: Clay Miller Date: Fri, 11 Jun 2021 10:58:13 -0400 Subject: [PATCH 6/6] docs: Document 'maxHeight' prop --- src/Overlay.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Overlay.tsx b/src/Overlay.tsx index 2f84c57137c..012142df55b 100644 --- a/src/Overlay.tsx +++ b/src/Overlay.tsx @@ -85,6 +85,7 @@ export type OverlayProps = { * @param onEscape Required. Function to call when user presses `Escape`. Typically this function sets the `Overlay` visibility state to `false`. * @param width Sets the width of the `Overlay`, pick from our set list of widths, or pass `auto` to automatically set the width based on the content of the `Overlay`. `small` corresponds to `256px`, `medium` corresponds to `320px`, `large` corresponds to `480px`, `xlarge` corresponds to `640px`, `xxlarge` corresponds to `960px`. * @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 visibility Sets the visibility of the `Overlay` */ const Overlay = React.forwardRef(