Skip to content

Commit

Permalink
fix(menu-button): handle child refs safely
Browse files Browse the repository at this point in the history
  • Loading branch information
stipsan committed Jun 10, 2024
1 parent 92b12a7 commit 9ca1312
Showing 1 changed file with 30 additions and 17 deletions.
47 changes: 30 additions & 17 deletions src/core/components/menu/menuButton.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
import {ThemeColorSchemeKey} from '@sanity/ui/theme'
import {cloneElement, forwardRef, useCallback, useMemo, useState, useEffect, useRef} from 'react'
import {
cloneElement,
forwardRef,
useCallback,
useMemo,
useState,
useEffect,
useRef,
useImperativeHandle,
} from 'react'
import {Popover, PopoverProps} from '../../primitives'
import {Placement, Radius} from '../../types'
import {MenuProps} from './menu'
Expand Down Expand Up @@ -52,7 +61,7 @@ export interface MenuButtonProps {
*/
export const MenuButton = forwardRef(function MenuButton(
props: MenuButtonProps,
ref: React.ForwardedRef<HTMLButtonElement | null>,
forwardedRef: React.ForwardedRef<HTMLButtonElement | null>,
) {
const {
__unstable_disableRestoreFocusOnClose: disableRestoreFocusOnClose = false,
Expand Down Expand Up @@ -219,19 +228,7 @@ export const MenuButton = forwardRef(function MenuButton(

const menu = menuProp && cloneElement(menuProp, menuProps)

const setButtonRef = useCallback(
(el: HTMLButtonElement | null) => {
if (typeof ref === 'function') {
ref(el)
} else if (ref) {
ref.current = el
}

setButtonElement(el)
},
[ref],
)

const ref = useRef<HTMLButtonElement | null>(null)
const button = useMemo(
() =>
buttonProp &&
Expand All @@ -243,12 +240,28 @@ export const MenuButton = forwardRef(function MenuButton(
onMouseDown: handleMouseDown,
'aria-haspopup': true,
'aria-expanded': open,
ref: setButtonRef,
ref: ref,
selected: open,
}),
[buttonProp, handleButtonClick, handleButtonKeyDown, handleMouseDown, id, open, setButtonRef],
[buttonProp, handleButtonClick, handleButtonKeyDown, handleMouseDown, id, open],
)

// Forward button ref to parent
useImperativeHandle<HTMLButtonElement | null, HTMLButtonElement | null>(
forwardedRef,
() => ref.current,
)

// If there's a button then we need to set the reference element to the cloned button ref
// and if button changes we make sure to update or remove the reference element.
useEffect(() => {
if (!button) return undefined

setButtonElement(ref.current)

return () => setButtonElement(null)
}, [button])

const popoverProps: MenuButtonProps['popover'] = useMemo(
() => ({
boundaryElement: deprecated_boundaryElement,
Expand Down

0 comments on commit 9ca1312

Please sign in to comment.