From 9e7869735ed523283143e30d9676ac9c7afd1039 Mon Sep 17 00:00:00 2001 From: dgreif Date: Mon, 19 Jul 2021 15:30:54 -0700 Subject: [PATCH 1/5] fix: keep AnchoredOverlay hidden until positioned --- docs/content/Overlay.mdx | 1 + src/AnchoredOverlay/AnchoredOverlay.tsx | 1 + src/Overlay.tsx | 9 ++++++--- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/content/Overlay.mdx b/docs/content/Overlay.mdx index 13661bd6271..f64d38b8b15 100644 --- a/docs/content/Overlay.mdx +++ b/docs/content/Overlay.mdx @@ -84,3 +84,4 @@ System props are deprecated in all components except [Box](/Box). Please use the | width | `'small' │ 'medium' │ 'large' │ 'xlarge' │ 'xxlarge' │ 'auto'` | `auto` | 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`. | | 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 | diff --git a/src/AnchoredOverlay/AnchoredOverlay.tsx b/src/AnchoredOverlay/AnchoredOverlay.tsx index baab28528ed..d9c05ffddbc 100644 --- a/src/AnchoredOverlay/AnchoredOverlay.tsx +++ b/src/AnchoredOverlay/AnchoredOverlay.tsx @@ -161,6 +161,7 @@ export const AnchoredOverlay: React.FC = ({ onEscape={onEscape} ref={updateOverlayRef} role="none" + visibility={position ? 'visible' : 'hidden'} height={height} width={width} {...overlayPosition} diff --git a/src/Overlay.tsx b/src/Overlay.tsx index 06666d02a94..880a3f195df 100644 --- a/src/Overlay.tsx +++ b/src/Overlay.tsx @@ -13,6 +13,7 @@ type StyledOverlayProps = { width?: keyof typeof widthMap height?: keyof typeof heightMap maxHeight?: keyof Omit + visibility?: 'visible' | 'hidden' anchorSide?: AnchorSide } @@ -71,7 +72,7 @@ const StyledOverlay = styled.div props.visibility || 'visible'}; :focus { outline: none; } @@ -85,8 +86,9 @@ export type OverlayProps = { returnFocusRef: React.RefObject onClickOutside: (e: TouchOrMouseEvent) => void onEscape: (e: KeyboardEvent) => void + visibility?: 'visible' | 'hidden' [additionalKey: string]: unknown -} & Omit, keyof SystemPositionProps> +} & Omit, 'visibility' | keyof SystemPositionProps> /** * An `Overlay` is a flexible floating surface, used to display transient content such as menus, @@ -111,6 +113,7 @@ const Overlay = React.forwardRef( returnFocusRef, ignoreClickRefs, onEscape, + visibility, height, anchorSide, ...rest @@ -156,7 +159,7 @@ const Overlay = React.forwardRef( return ( - + ) } From 68749aad86580f089d3bd07ff08db111e8d8dfd5 Mon Sep 17 00:00:00 2001 From: dgreif Date: Mon, 19 Jul 2021 21:14:34 -0700 Subject: [PATCH 2/5] fix(FilteredActionList): update focus zone when loading changes --- src/FilteredActionList/FilteredActionList.tsx | 32 +++++++++++-------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/FilteredActionList/FilteredActionList.tsx b/src/FilteredActionList/FilteredActionList.tsx index 361ff2a1f62..fdab5a81675 100644 --- a/src/FilteredActionList/FilteredActionList.tsx +++ b/src/FilteredActionList/FilteredActionList.tsx @@ -88,21 +88,27 @@ export function FilteredActionList({ [activeDescendantRef] ) - useFocusZone({ - containerRef: listContainerRef, - focusOutBehavior: 'wrap', - focusableElementFilter: element => { - return !(element instanceof HTMLInputElement) - }, - activeDescendantFocus: inputRef, - onActiveDescendantChanged: (current, previous, directlyActivated) => { - activeDescendantRef.current = current + useFocusZone( + { + containerRef: listContainerRef, + focusOutBehavior: 'wrap', + focusableElementFilter: element => { + return !(element instanceof HTMLInputElement) + }, + activeDescendantFocus: inputRef, + onActiveDescendantChanged: (current, previous, directlyActivated) => { + activeDescendantRef.current = current - if (current && scrollContainerRef.current && directlyActivated) { - scrollIntoViewingArea(current, scrollContainerRef.current) + if (current && scrollContainerRef.current && directlyActivated) { + scrollIntoViewingArea(current, scrollContainerRef.current) + } } - } - }) + }, + [ + // List ref isn't set while loading. Need to re-bind focus zone when it changes + loading + ] + ) useEffect(() => { // if items changed, we want to instantly move active descendant into view From 79fd32483e3e37e7ebb66fa006cf6fdf12549c1a Mon Sep 17 00:00:00 2001 From: dgreif Date: Mon, 19 Jul 2021 21:37:12 -0700 Subject: [PATCH 3/5] fix(Overlay): prevent animation if hidden --- src/Overlay.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Overlay.tsx b/src/Overlay.tsx index 880a3f195df..66124cf4954 100644 --- a/src/Overlay.tsx +++ b/src/Overlay.tsx @@ -143,7 +143,7 @@ const Overlay = React.forwardRef( useLayoutEffect(() => { const {x, y} = getSlideAnimationStartingVector(anchorSide) - if ((!x && !y) || !overlayRef.current?.animate) { + if ((!x && !y) || !overlayRef.current?.animate || visibility === 'hidden') { return } @@ -155,7 +155,7 @@ const Overlay = React.forwardRef( easing: slideAnimationEasing } ) - }, [anchorSide, slideAnimationDistance, slideAnimationEasing]) + }, [anchorSide, slideAnimationDistance, slideAnimationEasing, visibility]) return ( From 53a51e6d78c705834ac6129a42d677d6cf21b46e Mon Sep 17 00:00:00 2001 From: dgreif Date: Tue, 20 Jul 2021 07:57:41 -0700 Subject: [PATCH 4/5] perf(Overlay): use css var for visibility --- src/Overlay.tsx | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/Overlay.tsx b/src/Overlay.tsx index 66124cf4954..d9806db84e6 100644 --- a/src/Overlay.tsx +++ b/src/Overlay.tsx @@ -72,7 +72,7 @@ const StyledOverlay = styled.div props.visibility || 'visible'}; + visibility: var(--styled-overlay-visibility); :focus { outline: none; } @@ -113,7 +113,7 @@ const Overlay = React.forwardRef( returnFocusRef, ignoreClickRefs, onEscape, - visibility, + visibility = 'visible', height, anchorSide, ...rest @@ -159,7 +159,18 @@ const Overlay = React.forwardRef( return ( - + ) } From 50b0d357b25368b911fe3f3a71cc8f92cd22cb90 Mon Sep 17 00:00:00 2001 From: dgreif Date: Tue, 20 Jul 2021 08:11:37 -0700 Subject: [PATCH 5/5] test: add story and test for overlay placement --- src/__tests__/AnchoredOverlay.tsx | 5 + .../__snapshots__/AnchoredOverlay.tsx.snap | 238 ++++++++++++++++++ src/stories/SelectPanel.stories.tsx | 40 +++ 3 files changed, 283 insertions(+) diff --git a/src/__tests__/AnchoredOverlay.tsx b/src/__tests__/AnchoredOverlay.tsx index 66d6baef5c2..15552d90075 100644 --- a/src/__tests__/AnchoredOverlay.tsx +++ b/src/__tests__/AnchoredOverlay.tsx @@ -141,4 +141,9 @@ describe('AnchoredOverlay', () => { expect(mockCloseCallback).toHaveBeenCalledTimes(1) expect(mockCloseCallback).toHaveBeenCalledWith('escape') }) + + it('should render consistently when open', () => { + const anchoredOverlay = HTMLRender() + expect(anchoredOverlay).toMatchSnapshot() + }) }) diff --git a/src/__tests__/__snapshots__/AnchoredOverlay.tsx.snap b/src/__tests__/__snapshots__/AnchoredOverlay.tsx.snap index 9a0537de3c8..cb33f0eedb4 100644 --- a/src/__tests__/__snapshots__/AnchoredOverlay.tsx.snap +++ b/src/__tests__/__snapshots__/AnchoredOverlay.tsx.snap @@ -92,3 +92,241 @@ exports[`AnchoredOverlay renders consistently 1`] = ` `; + +exports[`AnchoredOverlay should render consistently when open 1`] = ` +Object { + "asFragment": [Function], + "baseElement": .c0 { + font-family: -apple-system,BlinkMacSystemFont,"Segoe UI",Helvetica,Arial,sans-serif,"Apple Color Emoji","Segoe UI Emoji"; + line-height: 1.5; + color: #24292e; +} + +.c1 { + position: relative; + display: inline-block; + padding: 6px 16px; + font-family: inherit; + font-weight: 600; + line-height: 20px; + white-space: nowrap; + vertical-align: middle; + cursor: pointer; + -webkit-user-select: none; + -moz-user-select: none; + -ms-user-select: none; + user-select: none; + border-radius: 6px; + -webkit-appearance: none; + -moz-appearance: none; + appearance: none; + -webkit-text-decoration: none; + text-decoration: none; + text-align: center; + font-size: 14px; + color: #24292e; + background-color: #fafbfc; + border: 1px solid rgba(27,31,35,0.15); + box-shadow: 0 1px 0 rgba(27,31,35,0.04),inset 0 1px 0 rgba(255,255,255,0.25); +} + +.c1:hover { + -webkit-text-decoration: none; + text-decoration: none; +} + +.c1:focus { + outline: none; +} + +.c1:disabled { + cursor: default; +} + +.c1:disabled svg { + opacity: 0.6; +} + +.c1:hover { + background-color: #f3f4f6; + border-color: rgba(27,31,35,0.15); +} + +.c1:focus { + border-color: rgba(27,31,35,0.15); + box-shadow: 0 0 0 3px rgba(3,102,214,0.3); +} + +.c1:active { + background-color: hsla(220,14%,94%,1); + box-shadow: inset 0 0.15em 0.3em rgba(27,31,35,0.15); +} + +.c1:disabled { + color: #959da5; + background-color: #fafbfc; + border-color: rgba(27,31,35,0.15); +} + +.c2 { + background-color: #ffffff; + box-shadow: 0 1px 3px rgba(27,31,35,0.12),0 8px 24px rgba(68,77,86,0.12); + position: absolute; + min-width: 192px; + max-width: 640px; + height: auto; + width: auto; + border-radius: 12px; + overflow: hidden; + -webkit-animation: overlay-appear 200ms cubic-bezier(0.33,1,0.68,1); + animation: overlay-appear 200ms cubic-bezier(0.33,1,0.68,1); + visibility: var(--styled-overlay-visibility); + top: 4px; + left: 0px; +} + +.c2:focus { + outline: none; +} + + +
+
+ +
+
+
+ +
+
+
+
+
+ , + "container":
+
+ +
+
+
+ +
+
+
+
+
, + "debug": [Function], + "findAllByAltText": [Function], + "findAllByDisplayValue": [Function], + "findAllByLabelText": [Function], + "findAllByPlaceholderText": [Function], + "findAllByRole": [Function], + "findAllByTestId": [Function], + "findAllByText": [Function], + "findAllByTitle": [Function], + "findByAltText": [Function], + "findByDisplayValue": [Function], + "findByLabelText": [Function], + "findByPlaceholderText": [Function], + "findByRole": [Function], + "findByTestId": [Function], + "findByText": [Function], + "findByTitle": [Function], + "getAllByAltText": [Function], + "getAllByDisplayValue": [Function], + "getAllByLabelText": [Function], + "getAllByPlaceholderText": [Function], + "getAllByRole": [Function], + "getAllByTestId": [Function], + "getAllByText": [Function], + "getAllByTitle": [Function], + "getByAltText": [Function], + "getByDisplayValue": [Function], + "getByLabelText": [Function], + "getByPlaceholderText": [Function], + "getByRole": [Function], + "getByTestId": [Function], + "getByText": [Function], + "getByTitle": [Function], + "queryAllByAltText": [Function], + "queryAllByDisplayValue": [Function], + "queryAllByLabelText": [Function], + "queryAllByPlaceholderText": [Function], + "queryAllByRole": [Function], + "queryAllByTestId": [Function], + "queryAllByText": [Function], + "queryAllByTitle": [Function], + "queryByAltText": [Function], + "queryByDisplayValue": [Function], + "queryByLabelText": [Function], + "queryByPlaceholderText": [Function], + "queryByRole": [Function], + "queryByTestId": [Function], + "queryByText": [Function], + "queryByTitle": [Function], + "rerender": [Function], + "unmount": [Function], +} +`; diff --git a/src/stories/SelectPanel.stories.tsx b/src/stories/SelectPanel.stories.tsx index b3ecefccc9c..a1755ef7a61 100644 --- a/src/stories/SelectPanel.stories.tsx +++ b/src/stories/SelectPanel.stories.tsx @@ -227,3 +227,43 @@ export function SelectPanelHeightInitialWithUnderflowingItemsAfterFetch(): JSX.E } SelectPanelHeightInitialWithUnderflowingItemsAfterFetch.storyName = 'SelectPanel, Height: Initial, Underflowing Items (After Fetch)' + +export function SelectPanelAboveTallBody(): JSX.Element { + const [selected, setSelected] = React.useState(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: 'xsmall'}} + /> +
+ This element makes the body really tall. This is to test that we do not have layout/focus issues if the Portal + is far down the page +
+ + ) +} +SelectPanelAboveTallBody.storyName = 'SelectPanel, Above a Tall Body'