Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for role="alertdialog" to <Dialog> component #2709

Merged
merged 5 commits into from
Aug 28, 2023
Merged

Add support for role="alertdialog" to <Dialog> component #2709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9f4632e
WIP
adamwathan Aug 24, 2023
2213a8d
Add warning for unsupported roles to `<Dialog>`
thecrypticace Aug 28, 2023
548c858
Update assertions
thecrypticace Aug 28, 2023
9eb909d
Add test for React
thecrypticace Aug 28, 2023
46ae305
Add support for `role=alertdialog` to Vue
thecrypticace Aug 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
94 changes: 93 additions & 1 deletion packages/@headlessui-react/src/components/dialog/dialog.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { createPortal } from 'react-dom'
import React, { createElement, useRef, useState, Fragment, useEffect, useCallback } from 'react'
import { render } from '@testing-library/react'
import { render, screen } from '@testing-library/react'

import { Dialog } from './dialog'
import { Popover } from '../popover/popover'
Expand Down Expand Up @@ -101,6 +101,98 @@ describe('Rendering', () => {
})
)

it(
'should be able to explicitly choose role=dialog',
suppressConsoleLogs(async () => {
function Example() {
let [isOpen, setIsOpen] = useState(false)

return (
<>
<button id="trigger" onClick={() => setIsOpen(true)}>
Trigger
</button>
<Dialog open={isOpen} onClose={setIsOpen} role="dialog">
<TabSentinel />
</Dialog>
</>
)
}
render(<Example />)

assertDialog({ state: DialogState.InvisibleUnmounted })

await click(document.getElementById('trigger'))

await nextFrame()

assertDialog({ state: DialogState.Visible, attributes: { role: 'dialog' } })
})
)

it(
'should be able to explicitly choose role=alertdialog',
suppressConsoleLogs(async () => {
function Example() {
let [isOpen, setIsOpen] = useState(false)

return (
<>
<button id="trigger" onClick={() => setIsOpen(true)}>
Trigger
</button>
<Dialog open={isOpen} onClose={setIsOpen} role="alertdialog">
<TabSentinel />
</Dialog>
</>
)
}
render(<Example />)

assertDialog({ state: DialogState.InvisibleUnmounted })

await click(document.getElementById('trigger'))

await nextFrame()

assertDialog({ state: DialogState.Visible, attributes: { role: 'alertdialog' } })
})
)

it(
'should fall back to role=dialog for an invalid role',
suppressConsoleLogs(async () => {
function Example() {
let [isOpen, setIsOpen] = useState(false)

return (
<>
<button id="trigger" onClick={() => setIsOpen(true)}>
Trigger
</button>
<Dialog
open={isOpen}
onClose={setIsOpen}
// @ts-expect-error: We explicitly type role to only accept valid options — but we still want to verify runtime behaviorr
role="foobar"
>
<TabSentinel />
</Dialog>
</>
)
}
render(<Example />)

assertDialog({ state: DialogState.InvisibleUnmounted })

await click(document.getElementById('trigger'))

await nextFrame()

assertDialog({ state: DialogState.Visible, attributes: { role: 'dialog' } })
}, 'warn')
)

it(
'should complain when an `open` prop is provided without an `onClose` prop',
suppressConsoleLogs(async () => {
Expand Down
23 changes: 21 additions & 2 deletions packages/@headlessui-react/src/components/dialog/dialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ let DEFAULT_DIALOG_TAG = 'div' as const
interface DialogRenderPropArg {
open: boolean
}
type DialogPropsWeControl = 'role' | 'aria-describedby' | 'aria-labelledby' | 'aria-modal'
type DialogPropsWeControl = 'aria-describedby' | 'aria-labelledby' | 'aria-modal'

let DialogRenderFeatures = Features.RenderStrategy | Features.Static

Expand All @@ -131,6 +131,7 @@ export type DialogProps<TTag extends ElementType> = Props<
open?: boolean
onClose(value: boolean): void
initialFocus?: MutableRefObject<HTMLElement | null>
role?: 'dialog' | 'alertdialog'
__demoMode?: boolean
}
>
Expand All @@ -145,11 +146,29 @@ function DialogFn<TTag extends ElementType = typeof DEFAULT_DIALOG_TAG>(
open,
onClose,
initialFocus,
role = 'dialog',
__demoMode = false,
...theirProps
} = props
let [nestedDialogCount, setNestedDialogCount] = useState(0)

let didWarnOnRole = useRef(false)

role = (function () {
if (role === 'dialog' || role === 'alertdialog') {
return role
}

if (!didWarnOnRole.current) {
didWarnOnRole.current = true
console.warn(
`Invalid role [${role}] passed to <Dialog />. Only \`dialog\` and and \`alertdialog\` are supported. Using \`dialog\` instead.`
)
}

return 'dialog'
})()

let usesOpenClosedState = useOpenClosed()
if (open === undefined && usesOpenClosedState !== null) {
// Update the `open` prop based on the open closed state
Expand Down Expand Up @@ -339,7 +358,7 @@ function DialogFn<TTag extends ElementType = typeof DEFAULT_DIALOG_TAG>(
let ourProps = {
ref: dialogRef,
id,
role: 'dialog',
role,
'aria-modal': dialogState === DialogStates.Open ? true : undefined,
'aria-labelledby': state.titleId,
'aria-describedby': describedby,
Expand Down
8 changes: 4 additions & 4 deletions packages/@headlessui-react/src/test-utils/accessibility-assertions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1301,11 +1301,11 @@ export function assertDescriptionValue(element: HTMLElement | null, value: strin
// ---

export function getDialog(): HTMLElement | null {
return document.querySelector('[role="dialog"]')
return document.querySelector('[role="dialog"],[role="alertdialog"]')
}

export function getDialogs(): HTMLElement[] {
return Array.from(document.querySelectorAll('[role="dialog"]'))
return Array.from(document.querySelectorAll('[role="dialog"],[role="alertdialog"]'))
}

export function getDialogTitle(): HTMLElement | null {
Expand Down Expand Up @@ -1358,7 +1358,7 @@ export function assertDialog(

assertHidden(dialog)

expect(dialog).toHaveAttribute('role', 'dialog')
expect(dialog).toHaveAttribute('role', options.attributes?.['role'] ?? 'dialog')
expect(dialog).not.toHaveAttribute('aria-modal', 'true')

if (options.textContent) expect(dialog).toHaveTextContent(options.textContent)
Expand All @@ -1373,7 +1373,7 @@ export function assertDialog(

assertVisible(dialog)

expect(dialog).toHaveAttribute('role', 'dialog')
expect(dialog).toHaveAttribute('role', options.attributes?.['role'] ?? 'dialog')
expect(dialog).toHaveAttribute('aria-modal', 'true')

if (options.textContent) expect(dialog).toHaveTextContent(options.textContent)
Expand Down
99 changes: 99 additions & 0 deletions packages/@headlessui-vue/src/components/dialog/dialog.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,105 @@ describe('Rendering', () => {
})
)

it(
'should be able to explicitly choose role=dialog',
suppressConsoleLogs(async () => {
renderTemplate({
template: `
<div>
<button id="trigger" @click="setIsOpen(true)">Trigger</button>
<Dialog :open="isOpen" @close="setIsOpen" class="relative bg-blue-500" role="dialog">
<TabSentinel />
</Dialog>
</div>
`,
setup() {
let isOpen = ref(false)
return {
isOpen,
setIsOpen(value: boolean) {
isOpen.value = value
},
}
},
})

assertDialog({ state: DialogState.InvisibleUnmounted })

await click(document.getElementById('trigger'))

await nextFrame()

assertDialog({ state: DialogState.Visible, attributes: { role: 'dialog' } })
})
)

it(
'should be able to explicitly choose role=alertdialog',
suppressConsoleLogs(async () => {
renderTemplate({
template: `
<div>
<button id="trigger" @click="setIsOpen(true)">Trigger</button>
<Dialog :open="isOpen" @close="setIsOpen" class="relative bg-blue-500" role="alertdialog">
<TabSentinel />
</Dialog>
</div>
`,
setup() {
let isOpen = ref(false)
return {
isOpen,
setIsOpen(value: boolean) {
isOpen.value = value
},
}
},
})

assertDialog({ state: DialogState.InvisibleUnmounted })

await click(document.getElementById('trigger'))

await nextFrame()

assertDialog({ state: DialogState.Visible, attributes: { role: 'alertdialog' } })
})
)

it(
'should fall back to role=dialog for an invalid role',
suppressConsoleLogs(async () => {
renderTemplate({
template: `
<div>
<button id="trigger" @click="setIsOpen(true)">Trigger</button>
<Dialog :open="isOpen" @close="setIsOpen" class="relative bg-blue-500" role="foobar">
<TabSentinel />
</Dialog>
</div>
`,
setup() {
let isOpen = ref(false)
return {
isOpen,
setIsOpen(value: boolean) {
isOpen.value = value
},
}
},
})

assertDialog({ state: DialogState.InvisibleUnmounted })

await click(document.getElementById('trigger'))

await nextFrame()

assertDialog({ state: DialogState.Visible, attributes: { role: 'dialog' } })
})
)

it(
'should complain when an `open` prop is not a boolean',
suppressConsoleLogs(async () => {
Expand Down
19 changes: 18 additions & 1 deletion packages/@headlessui-vue/src/components/dialog/dialog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ export let Dialog = defineComponent({
open: { type: [Boolean, String], default: Missing },
initialFocus: { type: Object as PropType<HTMLElement | null>, default: null },
id: { type: String, default: () => `headlessui-dialog-${useId()}` },
role: { type: String as PropType<'dialog' | 'alertdialog'>, default: 'dialog' },
},
emits: { close: (_close: boolean) => true },
setup(props, { emit, attrs, slots, expose }) {
Expand All @@ -85,6 +86,22 @@ export let Dialog = defineComponent({
ready.value = true
})

let didWarnOnRole = false
let role = computed(() => {
if (props.role === 'dialog' || props.role === 'alertdialog') {
return props.role
}

if (!didWarnOnRole) {
didWarnOnRole = true
console.warn(
`Invalid role [${role}] passed to <Dialog />. Only \`dialog\` and and \`alertdialog\` are supported. Using \`dialog\` instead.`
)
}

return 'dialog'
})

let nestedDialogCount = ref(0)

let usesOpenClosedState = useOpenClosed()
Expand Down Expand Up @@ -285,7 +302,7 @@ export let Dialog = defineComponent({
...attrs,
ref: internalDialogRef,
id,
role: 'dialog',
role: role.value,
'aria-modal': dialogState.value === DialogStates.Open ? true : undefined,
'aria-labelledby': titleId.value,
'aria-describedby': describedby.value,
Expand Down
8 changes: 4 additions & 4 deletions packages/@headlessui-vue/src/test-utils/accessibility-assertions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1301,11 +1301,11 @@ export function assertDescriptionValue(element: HTMLElement | null, value: strin
// ---

export function getDialog(): HTMLElement | null {
return document.querySelector('[role="dialog"]')
return document.querySelector('[role="dialog"],[role="alertdialog"]')
}

export function getDialogs(): HTMLElement[] {
return Array.from(document.querySelectorAll('[role="dialog"]'))
return Array.from(document.querySelectorAll('[role="dialog"],[role="alertdialog"]'))
}

export function getDialogTitle(): HTMLElement | null {
Expand Down Expand Up @@ -1358,7 +1358,7 @@ export function assertDialog(

assertHidden(dialog)

expect(dialog).toHaveAttribute('role', 'dialog')
expect(dialog).toHaveAttribute('role', options.attributes?.['role'] ?? 'dialog')
expect(dialog).not.toHaveAttribute('aria-modal', 'true')

if (options.textContent) expect(dialog).toHaveTextContent(options.textContent)
Expand All @@ -1373,7 +1373,7 @@ export function assertDialog(

assertVisible(dialog)

expect(dialog).toHaveAttribute('role', 'dialog')
expect(dialog).toHaveAttribute('role', options.attributes?.['role'] ?? 'dialog')
expect(dialog).toHaveAttribute('aria-modal', 'true')

if (options.textContent) expect(dialog).toHaveTextContent(options.textContent)
Expand Down