Skip to content

Commit

Permalink
fix: DropdownMenu and ActionMenu hidden until positioned (#1171)
Browse files Browse the repository at this point in the history
  • Loading branch information
dgreif committed Apr 20, 2021
2 parents 8989b40 + 9a44800 commit 06e138e
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 14 deletions.
1 change: 1 addition & 0 deletions docs/content/Overlay.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,4 @@ render(<Demo />)
| onEscape | `function` | `undefined` | Required. Function to call when user presses `Escape`. Typically this function sets the `Overlay` visibility state to `false`. |
| width | `'sm', 'md', 'lg', 'xl', '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`. `sm` corresponds to `256px`, `md` corresponds to `320px`, `lg` corresponds to `480px`, and `xl` corresponds to `640px`. |
| height | `'sm', 'md', '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`. `sm` corresponds to `480px` and `md` corresponds to `640px`. |
| visibility | `'visible', 'hidden'` | `visible` | Sets the visibility of the `Overlay`. |
9 changes: 8 additions & 1 deletion src/ActionMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,13 @@ const ActionMenuBase = ({
[open]
)

const {position} = useAnchoredPosition({anchorElementRef: anchorRef, floatingElementRef: overlayRef})
const {position} = useAnchoredPosition(
{
anchorElementRef: anchorRef,
floatingElementRef: overlayRef
},
[overlayRef.current]
)

useFocusZone({containerRef: overlayRef, disabled: !open || state !== 'listFocus'}, [position])
useFocusTrap({containerRef: overlayRef, disabled: !open || state !== 'listFocus'}, [position])
Expand All @@ -100,6 +106,7 @@ const ActionMenuBase = ({
onClickOutside={onDismiss}
onEscape={onDismiss}
ref={updateOverlayRef}
visibility={position ? 'visible' : 'hidden'}
{...position}
>
<List
Expand Down
9 changes: 8 additions & 1 deletion src/DropdownMenu/DropdownMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,13 @@ export function DropdownMenu({
[open]
)

const {position} = useAnchoredPosition({anchorElementRef: anchorRef, floatingElementRef: overlayRef})
const {position} = useAnchoredPosition(
{
anchorElementRef: anchorRef,
floatingElementRef: overlayRef
},
[overlayRef.current]
)

useFocusZone({containerRef: overlayRef, disabled: !open || focusType !== 'list' || !position})
useFocusTrap({containerRef: overlayRef, disabled: !open || focusType !== 'list' || !position})
Expand Down Expand Up @@ -147,6 +153,7 @@ export function DropdownMenu({
onClickOutside={onDismiss}
onEscape={onDismiss}
ref={updateOverlayRef}
visibility={position ? 'visible' : 'hidden'}
{...position}
>
<List {...listProps} role="listbox" renderItem={renderItemWithCallbacks} />
Expand Down
17 changes: 14 additions & 3 deletions src/Overlay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {useCombinedRefs} from './hooks/useCombinedRefs'
type StyledOverlayProps = {
width?: keyof typeof widthMap
height?: keyof typeof heightMap
visibility?: 'visible' | 'hidden'
}

const heightMap = {
Expand Down Expand Up @@ -48,6 +49,7 @@ const StyledOverlay = styled.div<StyledOverlayProps & SystemCommonProps & System
opacity: 1;
}
}
visibility: ${props => props.visibility || 'visible'};
${COMMON};
${POSITION};
${sx};
Expand All @@ -58,7 +60,8 @@ export type OverlayProps = {
returnFocusRef: React.RefObject<HTMLElement>
onClickOutside: (e: TouchOrMouseEvent) => void
onEscape: (e: KeyboardEvent) => void
} & Omit<ComponentProps<typeof StyledOverlay>, keyof SystemPositionProps>
visibility?: 'visible' | 'hidden'
} & Omit<ComponentProps<typeof StyledOverlay>, 'visibility' | keyof SystemPositionProps>

/**
* An `Overlay` is a flexible floating surface, used to display transient content such as menus,
Expand All @@ -71,10 +74,11 @@ 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`. `sm` corresponds to `256px`, `md` corresponds to `320px`, `lg` corresponds to `480px`, and `xl` corresponds to `640px`.
* @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`. `sm` corresponds to `480px` and `md` corresponds to `640px`.
* @param visibility Sets the visibility of the `Overlay`
*/
const Overlay = React.forwardRef<HTMLDivElement, OverlayProps>(
(
{onClickOutside, role = 'dialog', initialFocusRef, returnFocusRef, ignoreClickRefs, onEscape, ...rest},
{onClickOutside, role = 'dialog', initialFocusRef, returnFocusRef, ignoreClickRefs, onEscape, visibility, ...rest},
forwardedRef
): ReactElement => {
const overlayRef = useRef<HTMLDivElement>(null)
Expand All @@ -90,7 +94,14 @@ const Overlay = React.forwardRef<HTMLDivElement, OverlayProps>(
})
return (
<Portal>
<StyledOverlay {...overlayProps} aria-modal="true" role={role} {...rest} ref={combinedRef} />
<StyledOverlay
{...overlayProps}
aria-modal="true"
role={role}
{...rest}
ref={combinedRef}
visibility={visibility}
/>
</Portal>
)
}
Expand Down
2 changes: 1 addition & 1 deletion src/hooks/useAnchoredPosition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export function useAnchoredPosition(
setPosition(undefined)
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [floatingElementRef.current, anchorElementRef.current, ...dependencies])
}, [floatingElementRef, anchorElementRef, ...dependencies])
return {
floatingElementRef,
anchorElementRef,
Expand Down
27 changes: 19 additions & 8 deletions src/hooks/useCombinedRefs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,27 @@ export function useCombinedRefs<T>(...refs: (ForwardedRef<T> | null | undefined)
const combinedRef = useRef<T | null>(null)

React.useEffect(() => {
for (const ref of refs) {
if (!ref) {
return
}
if (typeof ref === 'function') {
ref(combinedRef.current ?? null)
} else {
ref.current = combinedRef.current ?? null
function setRefs(current: T | null = null) {
for (const ref of refs) {
if (!ref) {
return
}
if (typeof ref === 'function') {
ref(current)
} else {
ref.current = current
}
}
}

setRefs(combinedRef.current)

return () => {
// ensure the refs get updated on unmount
// eslint-disable-next-line react-hooks/exhaustive-deps
setRefs(combinedRef.current)
}

// eslint-disable-next-line react-hooks/exhaustive-deps
}, [...refs, combinedRef.current])

Expand Down

1 comment on commit 06e138e

@vercel
Copy link

@vercel vercel bot commented on 06e138e Apr 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.