Skip to content

Commit

Permalink
Merge 3fa1904 into 1adea12
Browse files Browse the repository at this point in the history
  • Loading branch information
TylerJDev committed Sep 19, 2024
2 parents 1adea12 + 3fa1904 commit f901c76
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 4 deletions.
25 changes: 23 additions & 2 deletions packages/react/src/Overlay/Overlay.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useRef, useState} from 'react'
import React, {useRef, useState, type AriaRole} from 'react'
import {Overlay, Box, Text} from '..'
import {ButtonDanger, Button} from '../deprecated'
import {render, waitFor, fireEvent} from '@testing-library/react'
Expand All @@ -12,8 +12,9 @@ import {NestedOverlays, MemexNestedOverlays, MemexIssueOverlay, PositionedOverla
type TestComponentSettings = {
initialFocus?: 'button'
callback?: () => void
role?: AriaRole
}
const TestComponent = ({initialFocus, callback}: TestComponentSettings) => {
const TestComponent = ({initialFocus, callback, role}: TestComponentSettings) => {
const [isOpen, setIsOpen] = useState(false)
const buttonRef = useRef<HTMLButtonElement>(null)
const confirmButtonRef = useRef<HTMLButtonElement>(null)
Expand All @@ -39,6 +40,7 @@ const TestComponent = ({initialFocus, callback}: TestComponentSettings) => {
ignoreClickRefs={[buttonRef]}
onEscape={closeOverlay}
onClickOutside={closeOverlay}
role={role}
width="small"
>
<Box display="flex" flexDirection="column" p={2}>
Expand Down Expand Up @@ -281,4 +283,23 @@ describe('Overlay', () => {
// if stopPropagation worked, mockHandler would not have been called
expect(mockHandler).toHaveBeenCalledTimes(0)
})

it('should provide `role="dialog"` and `aria-modal="true"`, if role is not provided', async () => {
const user = userEvent.setup()
const {getByText, getByRole} = render(<TestComponent />)

await user.click(getByText('open overlay'))

expect(getByRole('dialog')).toBeInTheDocument()
})

it('should not provide `dialog` roles if role is already provided', async () => {
const user = userEvent.setup()
const {getByText, queryByRole} = render(<TestComponent role="none" />)

await user.click(getByText('open overlay'))

expect(queryByRole('dialog')).not.toBeInTheDocument()
expect(queryByRole('none')).toBeInTheDocument()
})
})
16 changes: 14 additions & 2 deletions packages/react/src/Overlay/Overlay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef'
import type {AnchorSide} from '@primer/behaviors'
import {useTheme} from '../ThemeProvider'
import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
import {useFocusTrap} from '../hooks/useFocusTrap'

type StyledOverlayProps = {
width?: keyof typeof widthMap
Expand Down Expand Up @@ -109,6 +110,7 @@ type BaseOverlayProps = {
portalContainerName?: string
preventFocusOnOpen?: boolean
role?: AriaRole
focusTrap?: boolean
children?: React.ReactNode
}

Expand All @@ -133,12 +135,13 @@ type OwnOverlayProps = Merge<StyledOverlayProps, BaseOverlayProps>
* @param bottom Optional. Vertical bottom position of the overlay, relative to its closest positioned ancestor (often its `Portal`).
* @param position Optional. Sets how an element is positioned in a document. Defaults to `absolute` positioning.
* @param portalContainerName Optional. The name of the portal container to render the Overlay into.
* @param focusTrap Optional. Determines if the `Overlay` recieves a focus trap or not. Defaults to `true`.
*/
const Overlay = React.forwardRef<HTMLDivElement, OwnOverlayProps>(
(
{
onClickOutside,
role = 'none',
role,
initialFocusRef,
returnFocusRef,
ignoreClickRefs,
Expand All @@ -155,6 +158,7 @@ const Overlay = React.forwardRef<HTMLDivElement, OwnOverlayProps>(
preventFocusOnOpen,
position,
style: styleFromProps = {},
focusTrap = true,
...rest
},
forwardedRef,
Expand Down Expand Up @@ -200,12 +204,20 @@ const Overlay = React.forwardRef<HTMLDivElement, OwnOverlayProps>(
// To be backwards compatible with the old Overlay, we need to set the left prop if x-position is not specified
const leftPosition: React.CSSProperties = left === undefined && right === undefined ? {left: 0} : {left}

const dialog = role && role !== 'dialog' ? true : false

useFocusTrap({
containerRef: overlayRef, // only if `role="dialog"`, `aria-modal="true"` is true
disabled: !focusTrap || dialog,
})

return (
<Portal containerName={portalContainerName}>
<StyledOverlay
height={height}
width={width}
role={role}
role={role || 'dialog'}
aria-modal={dialog ? undefined : 'true'}
{...rest}
ref={overlayRef}
style={
Expand Down

0 comments on commit f901c76

Please sign in to comment.