simplify the click / pointer up handling on Menu Item

Thank you @MartinMa for pointing this complexity out!
This commit is contained in:
Robin Malfait
2020-09-29 12:49:30 +02:00
parent 878e417139
commit 4b2aacfdf2
3 changed files with 12 additions and 42 deletions
@@ -227,7 +227,7 @@ export function Menu<TTag extends React.ElementType = typeof DEFAULT_MENU_TAG>(
const [{ menuState, itemsRef, buttonRef }, dispatch] = reducerBag
React.useEffect(() => {
function handler(event: PointerEvent) {
function handler(event: MouseEvent) {
if (menuState !== MenuStates.Open) return
if (buttonRef.current?.contains(event.target as HTMLElement)) return
@@ -237,8 +237,8 @@ export function Menu<TTag extends React.ElementType = typeof DEFAULT_MENU_TAG>(
}
}
window.addEventListener('pointerup', handler)
return () => window.removeEventListener('pointerup', handler)
window.addEventListener('click', handler)
return () => window.removeEventListener('click', handler)
}, [menuState, itemsRef, buttonRef, d, dispatch])
const propsBag = React.useMemo(() => ({ open: menuState === MenuStates.Open }), [menuState])
@@ -498,7 +498,6 @@ type MenuItemPropsWeControl =
| 'aria-disabled'
| 'onPointerEnter'
| 'onPointerLeave'
| 'onPointerUp'
| 'onFocus'
const DEFAULT_ITEM_TAG = React.Fragment
@@ -557,21 +556,14 @@ function Item<TTag extends React.ElementType = typeof DEFAULT_ITEM_TAG>(
dispatch({ type: ActionTypes.GoToItem, focus: Focus.SpecificItem, id })
}, [disabled, active, id, dispatch])
const handlePointerUp = React.useCallback(
(event: React.PointerEvent<HTMLElement>) => {
if (disabled) return event.preventDefault()
dispatch({ type: ActionTypes.CloseMenu })
d.nextFrame(() => state.buttonRef.current?.focus())
},
[dispatch, disabled, d, state.buttonRef]
)
const handleClick = React.useCallback(
(event: { preventDefault: Function }) => {
if (disabled) return event.preventDefault()
dispatch({ type: ActionTypes.CloseMenu })
d.nextFrame(() => state.buttonRef.current?.focus())
if (onClick) return onClick(event)
},
[disabled, onClick]
[d, dispatch, state.buttonRef, disabled, onClick]
)
const propsBag = React.useMemo(() => ({ active, disabled }), [active, disabled])
@@ -586,7 +578,6 @@ function Item<TTag extends React.ElementType = typeof DEFAULT_ITEM_TAG>(
onMouseMove: handleMouseMove,
onPointerEnter: handlePointerEnter,
onPointerLeave: handlePointerLeave,
onPointerUp: handlePointerUp,
}
return render<TTag, ItemRenderPropArg>(
@@ -1,4 +1,4 @@
import { defineComponent, h, nextTick } from 'vue'
import { defineComponent, h } from 'vue'
import { render } from '../../test-utils/vue-testing-library'
import { Menu, MenuButton, MenuItems, MenuItem } from './menu'
import { suppressConsoleLogs } from '../../test-utils/suppress-console-logs'
@@ -73,13 +73,6 @@ function getMenuItems(): HTMLElement[] {
return Array.from(document.querySelectorAll('[role="menuitem"]'))
}
beforeAll(() => {
jest.spyOn(window, 'requestAnimationFrame').mockImplementation(nextTick as any)
jest.spyOn(window, 'cancelAnimationFrame').mockImplementation(jest.fn())
})
afterAll(() => jest.restoreAllMocks())
describe('Safe guards', () => {
it.each([
['MenuButton', MenuButton],
@@ -187,7 +187,7 @@ export const Menu = defineComponent({
}
onMounted(() => {
function handler(event: PointerEvent) {
function handler(event: MouseEvent) {
if (menuState.value !== MenuStates.Open) return
if (buttonRef.value?.contains(event.target as HTMLElement)) return
@@ -197,8 +197,8 @@ export const Menu = defineComponent({
}
}
window.addEventListener('pointerup', handler)
onUnmounted(() => window.removeEventListener('pointerup', handler))
window.addEventListener('click', handler)
onUnmounted(() => window.removeEventListener('click', handler))
})
// @ts-expect-error Types of property 'dataRef' are incompatible.
@@ -440,23 +440,10 @@ export const MenuItem = defineComponent({
api.goToItem(Focus.SpecificItem, id)
}
function handlePointerUp(event: PointerEvent) {
if (disabled) return event.preventDefault()
// Turns out that we can't use nextTick here. Even if we do, the `handleClick` would *not* be
// called because the closeMenu() update is *too fast* and the tree gets unmounted before it
// bubbles up. So instead of nextTick, we use the good old double requestAnimationFrame to
// wait for a "nextFrame".
requestAnimationFrame(() => {
requestAnimationFrame(() => {
api.closeMenu()
api.buttonRef.value?.focus()
})
})
}
function handleClick(event: MouseEvent) {
if (disabled) return event.preventDefault()
api.closeMenu()
nextTick(() => api.buttonRef.value?.focus())
}
return () => {
@@ -472,7 +459,6 @@ export const MenuItem = defineComponent({
onMouseMove: handleMouseMove,
onPointerEnter: handlePointerEnter,
onPointerLeave: handlePointerLeave,
onPointerUp: handlePointerUp,
}
return render({