Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/big-lions-mix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

Overlay: Attach escape handler to overlay container instead of document to fix stopPropagation
43 changes: 43 additions & 0 deletions src/__tests__/Overlay.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,39 @@ 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={event => {
event.stopPropagation()
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 />)
Expand Down Expand Up @@ -100,4 +133,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)
})
})
19 changes: 14 additions & 5 deletions src/hooks/useOnEscapePress.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️ great idea

* 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 () => {
Expand All @@ -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])
}
2 changes: 1 addition & 1 deletion src/hooks/useOverlay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@ export const useOverlay = ({
const overlayRef = useProvidedRefOrCreate<HTMLDivElement>(_overlayRef)
useOpenAndCloseFocus({containerRef: overlayRef, returnFocusRef, initialFocusRef, preventFocusOnOpen})
useOnOutsideClick({containerRef: overlayRef, ignoreClickRefs, onClickOutside})
useOnEscapePress(onEscape)
useOnEscapePress(onEscape, undefined, overlayRef)
return {ref: overlayRef}
}
120 changes: 119 additions & 1 deletion src/stories/Overlay.stories.tsx
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -211,3 +229,103 @@ export const OverlayOnTopOfOverlay = ({anchorSide}: OverlayProps) => {
</Box>
)
}

export const OverlayOnOverlay = () => {
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={event => {
event.stopPropagation()
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={event => {
event.stopPropagation()
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>
)
}