Overlay: Attach escape handler to overlay container

.changeset/big-lions-mix.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+Overlay: Attach escape handler to overlay container instead of document to fix stopPropagation

src/__tests__/Overlay.test.tsx

@@ -56,6 +56,32 @@ const TestComponent = ({initialFocus, callback}: TestComponentSettings) => {
   )
 }
 
+// https://github.com/primer/react/issues/1802
+const BugRepro1802 = ({mockHandler}: {mockHandler: (event: KeyboardEvent) => void}) => {
+  const [isOpen, setIsOpen] = useState(false)
+  const buttonRef = useRef<HTMLButtonElement>(null)
+
+  React.useEffect(() => {
+    document.addEventListener('keydown', mockHandler)
+    return () => document.removeEventListener('keydown', mockHandler)
+  }, [mockHandler])
+
+  return (
+    <ThemeProvider theme={theme}>
+      <BaseStyles>
+        <Button ref={buttonRef} onClick={() => setIsOpen(!isOpen)}>
+          open overlay
+        </Button>
+        {isOpen ? (
+          <Overlay returnFocusRef={buttonRef} onEscape={() => setIsOpen(false)} onClickOutside={() => setIsOpen(false)}>
+            <Text>Text inside Overlay</Text>
+          </Overlay>
+        ) : null}
+      </BaseStyles>
+    </ThemeProvider>
+  )
+}
+
 describe('Overlay', () => {
   it('should have no axe violations', async () => {
     const {container} = render(<TestComponent />)
@@ -100,4 +126,14 @@ describe('Overlay', () => {
     const cancelButtons = queryAllByText('Cancel')
     expect(cancelButtons).toHaveLength(0)
   })
+
+  it('should call stop propagation', () => {
+    const mockHandler = jest.fn()
+    const {getByText} = render(<BugRepro1802 mockHandler={mockHandler} />)
+    act(() => userEvent.click(getByText('open overlay')))
+    const domNode = getByText('Text inside Overlay')
+    fireEvent.keyDown(domNode, {key: 'Escape', code: 'Escape', keyCode: 27, charCode: 27})
+    // if stopPropagation worked, mockHandler would not have been called
+    expect(mockHandler).toHaveBeenCalledTimes(0)
+  })
 })

src/hooks/useOnEscapePress.ts

@@ -9,7 +9,7 @@ const handlers: ((e: KeyboardEvent) => void)[] = []
 function handleEscape(event: KeyboardEvent) {
   if (event.key === 'Escape' && !event.defaultPrevented) {
     for (let i = handlers.length - 1; i >= 0; --i) {
-      handlers[i](event)
+      if (typeof handlers[i] === 'function') handlers[i](event)
       // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
       if (event.defaultPrevented) {
         break
@@ -19,7 +19,7 @@ function handleEscape(event: KeyboardEvent) {
 }
 
 /**
- * Sets up a `keydown` listener on `window.document`. If
+ * Sets up a `keydown` listener on element passed | `window.document`. If
  * 1) The pressed key is "Escape", and
  * 2) The event has not had `.preventDefault()` called
  * The given callback will be executed.
@@ -38,16 +38,24 @@ function handleEscape(event: KeyboardEvent) {
  * @param callbackDependencies {React.DependencyList} The dependencies of the given
  * `onEscape` callback for memoization. Omit this param if the callback is already
  * memoized. See `React.useCallback` for more info on memoization.
+ *
+ * @param containerRef {React.RefObject<HTMLElement>} The overlay element to attach the
+ * handlers on. If not provided, fallback to document.
  */
 export const useOnEscapePress = (
   onEscape: (e: KeyboardEvent) => void,
-  callbackDependencies: React.DependencyList = [onEscape]
+  callbackDependencies: React.DependencyList = [onEscape],
+  containerRef?: React.RefObject<HTMLElement>
 ): void => {
   // eslint-disable-next-line react-hooks/exhaustive-deps
   const escapeCallback = useCallback(onEscape, callbackDependencies)
+
   useEffect(() => {
+    const element = containerRef?.current
+
     if (handlers.length === 0) {
-      document.addEventListener('keydown', handleEscape)
+      if (element) element.addEventListener('keydown', handleEscape)
+      else document.addEventListener('keydown', handleEscape)
     }
     handlers.push(escapeCallback)
     return () => {
@@ -56,8 +64,9 @@ export const useOnEscapePress = (
         1
       )
       if (handlers.length === 0) {
-        document.removeEventListener('keydown', handleEscape)
+        if (element) element.removeEventListener('keydown', handleEscape)
+        else document.removeEventListener('keydown', handleEscape)
       }
     }
-  }, [escapeCallback])
+  }, [escapeCallback, containerRef])
 }

src/hooks/useOverlay.tsx

@@ -29,6 +29,12 @@ export const useOverlay = ({
   const overlayRef = useProvidedRefOrCreate<HTMLDivElement>(_overlayRef)
   useOpenAndCloseFocus({containerRef: overlayRef, returnFocusRef, initialFocusRef, preventFocusOnOpen})
   useOnOutsideClick({containerRef: overlayRef, ignoreClickRefs, onClickOutside})
-  useOnEscapePress(onEscape)
+
+  /** Don't bubble Escape event up the tree  */
+  const onEscapeWithStoppedPropagation: UseOverlaySettings['onEscape'] = event => {
+    event.stopPropagation()
+    onEscape(event)
+  }
+  useOnEscapePress(onEscapeWithStoppedPropagation, undefined, overlayRef)
   return {ref: overlayRef}
 }

src/stories/Overlay.stories.tsx

@@ -1,7 +1,25 @@
 import React, {useState, useRef, useCallback} from 'react'
+import ReactDOM from 'react-dom'
 import {Meta} from '@storybook/react'
 import styled from 'styled-components'
-import {BaseStyles, Overlay, Button, Text, ButtonDanger, ThemeProvider, Box} from '..'
+import {TriangleDownIcon, PlusIcon} from '@primer/octicons-react'
+import {
+  BaseStyles,
+  Overlay,
+  Button,
+  ButtonInvisible,
+  ButtonPrimary,
+  ButtonGroup,
+  Text,
+  ButtonDanger,
+  ThemeProvider,
+  Box,
+  StyledOcticon,
+  Checkbox,
+  ChoiceInputField,
+  TextInput,
+  ActionList
+} from '..'
 import type {AnchorSide} from '@primer/behaviors'
 import {DropdownMenu, DropdownButton} from '../DropdownMenu'
 import {ItemInput} from '../ActionList/List'
@@ -211,3 +229,97 @@ export const OverlayOnTopOfOverlay = ({anchorSide}: OverlayProps) => {
     </Box>
   )
 }
+
+export const NestedOverlays = () => {
+  const [listOverlayOpen, setListOverlayOpen] = React.useState(false)
+  const [createListOverlayOpen, setCreateListOverlayOpen] = React.useState(false)
+
+  const buttonRef = useRef<HTMLButtonElement>(null)
+  const secondaryButtonRef = useRef<HTMLButtonElement>(null)
+
+  React.useEffect(() => {
+    // eslint-disable-next-line no-console
+    const handler = (event: KeyboardEvent) => console.log('global handler:', event.key)
+    document.addEventListener('keydown', handler)
+    return () => document.removeEventListener('keydown', handler)
+  }, [])
+
+  const hostElement = document.createElement('div')
+  ReactDOM.render(<div>hello</div>, hostElement)
+
+  return (
+    <div>
+      <TextInput />
+      <ButtonGroup display="block" my={2}>
+        <Button>Star</Button>
+        <Button
+          aria-label="Add this repository to a list"
+          ref={buttonRef}
+          onClick={() => setListOverlayOpen(!listOverlayOpen)}
+          sx={{paddingX: 2}}
+        >
+          <TriangleDownIcon />
+        </Button>
+      </ButtonGroup>
+      {listOverlayOpen && (
+        <Overlay
+          width="medium"
+          onEscape={() => setListOverlayOpen(false)}
+          onClickOutside={() => setListOverlayOpen(false)}
+          returnFocusRef={buttonRef}
+          ignoreClickRefs={[buttonRef]}
+          top={60}
+          left={16}
+        >
+          <Box sx={{display: 'flex', flexDirection: 'column', py: 2}}>
+            <Box sx={{paddingX: 3}}>
+              <Text color="fg.muted" sx={{fontSize: 1}}>
+                Add to list
+              </Text>
+              <Box sx={{marginY: 1}}>
+                <ChoiceInputField>
+                  <ChoiceInputField.Label>My stack</ChoiceInputField.Label>
+                  <Checkbox />
+                </ChoiceInputField>
+                <ChoiceInputField>
+                  <ChoiceInputField.Label>Want to try</ChoiceInputField.Label>
+                  <Checkbox />
+                </ChoiceInputField>
+              </Box>
+            </Box>
+            <ActionList.Divider />
+            <ButtonInvisible
+              ref={secondaryButtonRef}
+              sx={{textAlign: 'left', px: 2, mx: 2}}
+              onClick={() => setCreateListOverlayOpen(!createListOverlayOpen)}
+            >
+              <StyledOcticon icon={PlusIcon} sx={{mr: 1}} />
+              Create list
+            </ButtonInvisible>
+          </Box>
+          {createListOverlayOpen && (
+            <Overlay
+              width="large"
+              onEscape={() => setCreateListOverlayOpen(false)}
+              onClickOutside={() => setCreateListOverlayOpen(false)}
+              returnFocusRef={secondaryButtonRef}
+              ignoreClickRefs={[secondaryButtonRef]}
+              top={120}
+              left={64}
+            >
+              <Box as="form" sx={{display: 'flex', flexDirection: 'column', p: 3}}>
+                <Text color="fg.muted" sx={{fontSize: 1, mb: 3}}>
+                  Create a list to organize your starred repositories.
+                </Text>
+                <TextInput placeholder="Name this list" sx={{mb: 2}} />
+                <TextInput as="textarea" placeholder="Write a description" rows="3" sx={{mb: 2, textarea: {p: 2}}} />
+
+                <ButtonPrimary onClick={() => setCreateListOverlayOpen(!createListOverlayOpen)}>Create</ButtonPrimary>
+              </Box>
+            </Overlay>
+          )}
+        </Overlay>
+      )}
+    </div>
+  )
+}