Skip to content

Commit

Permalink
Action Menu correctly fires onAction callback after close (#1228)
Browse files Browse the repository at this point in the history
* take out double call of ActionMenu

* remove test

* update action menu to use combined state for useEffect hook

* Create bright-donkeys-exist.md

* remove delayed action in ActionMenu
  • Loading branch information
VanAnderson authored May 17, 2021
1 parent a40f037 commit e1bde42
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 12 deletions.
5 changes: 5 additions & 0 deletions .changeset/bright-donkeys-exist.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/components": patch
---

Action Menu correctly fires onAction callback after close.
13 changes: 1 addition & 12 deletions src/ActionMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {GroupedListProps, List, ListPropsBase} from './ActionList/List'
import {Item, ItemProps} from './ActionList/Item'
import {Divider} from './ActionList/Divider'
import Button, {ButtonProps} from './Button'
import React, {useCallback, useEffect, useMemo, useRef} from 'react'
import React, {useCallback, useMemo} from 'react'
import {AnchoredOverlay} from './AnchoredOverlay'
import {useProvidedStateOrCreate} from './hooks/useProvidedStateOrCreate'
import {OverlayProps} from './Overlay'
Expand Down Expand Up @@ -55,7 +55,6 @@ const ActionMenuBase = ({
items,
...listProps
}: ActionMenuProps): JSX.Element => {
const pendingActionRef = useRef<() => unknown>()
const [combinedOpenState, setCombinedOpenState] = useProvidedStateOrCreate(open, setOpen, false)
const onOpen = useCallback(() => setCombinedOpenState(true), [setCombinedOpenState])
const onClose = useCallback(() => setCombinedOpenState(false), [setCombinedOpenState])
Expand All @@ -78,7 +77,6 @@ const ActionMenuBase = ({
role: 'menuitem',
onAction: (props, event) => {
const actionCallback = item.onAction ?? onAction
pendingActionRef.current = () => actionCallback?.(props as ItemProps, event)
actionCallback?.(props as ItemProps, event)
if (!event.defaultPrevented) {
onClose()
Expand All @@ -88,15 +86,6 @@ const ActionMenuBase = ({
})
}, [items, onAction, onClose])

useEffect(() => {
// Wait until menu has re-rendered in a closed state before triggering action.
// This is needed in scenarios where the action will move focus, which would otherwise be captured by focus trap
if (!open && pendingActionRef.current) {
pendingActionRef.current()
pendingActionRef.current = undefined
}
}, [open])

return (
<AnchoredOverlay
renderAnchor={renderMenuAnchor}
Expand Down
1 change: 1 addition & 0 deletions src/__tests__/ActionMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ describe('ActionMenu', () => {
act(() => {
fireEvent.click(menuItem)
})

// onAction has been called with correct argument
expect(mockOnActivate).toHaveBeenCalledTimes(1)
const arg = mockOnActivate.mock.calls[0][0]
Expand Down

1 comment on commit e1bde42

@vercel
Copy link

@vercel vercel bot commented on e1bde42 May 17, 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.