Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
6 changes: 3 additions & 3 deletions packages/@adobe/react-spectrum/src/dialog/AlertDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
*/

import AlertMedium from '@spectrum-icons/ui/AlertMedium';
import {AriaLabelingProps, DOMProps, DOMRef, StyleProps} from '@react-types/shared';
import {Button, SpectrumButtonProps} from '../button/Button';
import {ButtonGroup} from '../buttongroup/ButtonGroup';
import {chain} from 'react-aria/chain';
Expand All @@ -19,7 +20,6 @@ import {Content} from '../view/Content';
import {Dialog} from './Dialog';
import {DialogContext, DialogContextValue} from './context';
import {Divider} from '../divider/Divider';
import {DOMProps, DOMRef, StyleProps} from '@react-types/shared';
import {filterDOMProps} from 'react-aria/filterDOMProps';
import {Heading} from '../text/Heading';
import intlMessages from '../../intl/dialog/*.json';
Expand All @@ -29,7 +29,7 @@ import styles from '@adobe/spectrum-css-temp/components/dialog/vars.css';
import {useLocalizedStringFormatter} from 'react-aria/useLocalizedStringFormatter';
import {useStyleProps} from '../utils/styleProps';

export interface SpectrumAlertDialogProps extends DOMProps, StyleProps {
export interface SpectrumAlertDialogProps extends AriaLabelingProps, DOMProps, StyleProps {
/** The [visual style](https://spectrum.adobe.com/page/alert-dialog/#Options) of the AlertDialog. */
variant?: 'confirmation' | 'information' | 'destructive' | 'error' | 'warning',
/** The title of the AlertDialog. */
Expand Down Expand Up @@ -100,7 +100,7 @@ export const AlertDialog = forwardRef(function AlertDialog(props: SpectrumAlertD
size="M"
role="alertdialog"
ref={ref}
{...filterDOMProps(props)}>
{...filterDOMProps(props, {labelable: true})}>
<Heading>{title}</Heading>
{(variant === 'error' || variant === 'warning') &&
<AlertMedium
Expand Down
6 changes: 3 additions & 3 deletions packages/@adobe/react-spectrum/src/dialog/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export const Dialog = React.forwardRef(function Dialog(props: SpectrumDialogProp
let domRef = useDOMRef(ref);
let gridRef = useRef(null);
let sizeVariant = sizeMap[type] || sizeMap[size];
let {dialogProps, titleProps} = useDialog(mergeProps(contextProps, props), domRef);
let {dialogProps, titleProps, contentProps} = useDialog(mergeProps(contextProps, props), domRef);

let hasHeader = useHasChild(`.${styles['spectrum-Dialog-header']}`, unwrapDOMRef(gridRef));
let hasHeading = useHasChild(`.${styles['spectrum-Dialog-heading']}`, unwrapDOMRef(gridRef));
Expand All @@ -86,11 +86,11 @@ export const Dialog = React.forwardRef(function Dialog(props: SpectrumDialogProp
header: {UNSAFE_className: classNames(styles, 'spectrum-Dialog-header', {'spectrum-Dialog-header--noHeading': !hasHeading, 'spectrum-Dialog-header--noTypeIcon': !hasTypeIcon})},
typeIcon: {UNSAFE_className: styles['spectrum-Dialog-typeIcon']},
divider: {UNSAFE_className: styles['spectrum-Dialog-divider'], size: 'M'},
content: {UNSAFE_className: styles['spectrum-Dialog-content']},
content: {UNSAFE_className: styles['spectrum-Dialog-content'], ...contentProps},
footer: {UNSAFE_className: styles['spectrum-Dialog-footer']},
buttonGroup: {UNSAFE_className: classNames(styles, 'spectrum-Dialog-buttonGroup', {'spectrum-Dialog-buttonGroup--noFooter': !hasFooter}), align: 'end'}
// eslint-disable-next-line react-hooks/exhaustive-deps
}), [hasFooter, hasHeader, titleProps]);
}), [hasFooter, hasHeader, titleProps, contentProps]);

return (
<section
Expand Down
29 changes: 29 additions & 0 deletions packages/@adobe/react-spectrum/test/dialog/AlertDialog.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,4 +203,33 @@ describe('AlertDialog', function () {
let primaryBtn = getByTestId('rsp-AlertDialog-confirmButton');
expect(primaryBtn).toBeDefined();
});

it('should have aria-describedby pointing to the content', function () {
let {getByRole} = render(
<Provider theme={theme}>
<AlertDialog variant="confirmation" title="the title" primaryActionLabel="confirm">
Content body
</AlertDialog>
</Provider>
);

let dialog = getByRole('alertdialog');
expect(dialog).toHaveAttribute('aria-describedby');
let contentId = dialog.getAttribute('aria-describedby');
let content = document.getElementById(contentId);
expect(content).not.toBeNull();
expect(content.textContent).toBe('Content body');
});

it('accepts custom aria-describedby', function () {
let {getByRole} = render(
<Provider theme={theme}>
<AlertDialog aria-describedby="content-id" variant="confirmation" title="the title" primaryActionLabel="confirm">
Content body
</AlertDialog>
</Provider>
);

expect(getByRole('alertdialog')).toHaveAttribute('aria-describedby', 'content-id');
});
});
8 changes: 6 additions & 2 deletions packages/@react-spectrum/s2/src/AlertDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@
*/

import AlertTriangle from '../s2wf-icons/S2_Icon_AlertTriangle_20_N.svg';
import {AriaLabelingProps, DOMProps, DOMRef} from '@react-types/shared';
import {Button} from './Button';
import {ButtonGroup} from './ButtonGroup';
import {CenterBaseline} from './CenterBaseline';
import {chain} from 'react-aria/chain';
import {Content, Heading} from './Content';
import {Dialog} from './Dialog';
import {DOMProps, DOMRef} from '@react-types/shared';
import {filterDOMProps} from 'react-aria/filterDOMProps';
import {forwardRef, ReactNode} from 'react';
import {IconContext} from './Icon';
// @ts-ignore
Expand All @@ -28,7 +29,7 @@ import {style} from '../style' with {type: 'macro'};
import {UnsafeStyles} from './style-utils' with {type: 'macro'};
import {useLocalizedStringFormatter} from 'react-aria/useLocalizedStringFormatter';

export interface AlertDialogProps extends DOMProps, UnsafeStyles {
export interface AlertDialogProps extends AriaLabelingProps, DOMProps, UnsafeStyles {
/**
* The [visual style](https://spectrum.adobe.com/page/alert-dialog/#Options) of the AlertDialog.
* @default 'confirmation'
Expand Down Expand Up @@ -104,8 +105,11 @@ export const AlertDialog = forwardRef(function AlertDialog(props: AlertDialogPro
buttonVariant = 'negative';
}

let domProps = filterDOMProps(props, {labelable: true});

return (
<Dialog
{...domProps}
role="alertdialog"
ref={ref}
size={props.size}
Expand Down
25 changes: 14 additions & 11 deletions packages/@react-spectrum/s2/src/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
import {Provider} from 'react-aria-components/slots';
import {style} from '../style' with {type: 'macro'};
import {StyleProps} from './style-utils';
import {Text} from 'react-aria-components/Text';
import {useDOMRef} from './useDOMRef';

// TODO: what style overrides should be allowed?
Expand Down Expand Up @@ -192,17 +193,19 @@ export const Dialog = forwardRef(function Dialog(props: DialogProps, ref: DOMRef
}
</div>
{/* Main content */}
<Provider
values={[
[ImageContext, {hidden: true}],
[HeadingContext, {isHidden: true}],
[HeaderContext, {isHidden: true}],
[ContentContext, {styles: content}],
[FooterContext, {isHidden: true}],
[ButtonGroupContext, {isHidden: true}]
]}>
{children}
</Provider>
<Text slot="description" style={{display: 'contents'}}>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Because we added a new TextContext into Dialog in RAC, this is how we should probably pick up the context in S2. Note, because of display 'contents', this should result in no layout changes visually. It does add an extra dom node though, that no one should really be trying to access.

Another option is to make use of the TextContext.Consumer and pass the props on that context directly to the ContentContext below.

Preferences?

<Provider
values={[
[ImageContext, {hidden: true}],
[HeadingContext, {isHidden: true}],
[HeaderContext, {isHidden: true}],
[ContentContext, {styles: content}],
[FooterContext, {isHidden: true}],
[ButtonGroupContext, {isHidden: true}]
]}>
{children}
</Provider>
</Text>
{/* Footer and button group */}
<div
className={style({
Expand Down
78 changes: 78 additions & 0 deletions packages/@react-spectrum/s2/test/AlertDialog.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* Copyright 2025 Adobe. All rights reserved.
* This file is licensed to you under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. You may obtain a copy
* of the License at http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under
* the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/

import {act, pointerMap, render} from '@react-spectrum/test-utils-internal';
import {ActionButton} from '../src/ActionButton';
import {AlertDialog} from '../src/AlertDialog';
import {Content} from '../src/Content';
import {DialogTrigger} from '../src/DialogTrigger';
import React from 'react';
import userEvent from '@testing-library/user-event';

describe('AlertDialog', () => {
let user;
beforeAll(() => {
jest.useFakeTimers();
user = userEvent.setup({delay: null, pointerMap});
});

afterEach(() => {
jest.clearAllMocks();
act(() => jest.runAllTimers());
});

afterAll(function () {
jest.restoreAllMocks();
});

it('automatically links to the content with aria-describedby', async () => {
let {getByRole} = render(
<DialogTrigger>
<ActionButton>Open dialog</ActionButton>
<AlertDialog title="Test" primaryActionLabel="Test">
Test content
</AlertDialog>
</DialogTrigger>
);

let trigger = getByRole('button');
await user.click(trigger);
act(() => {jest.runAllTimers();});
let dialog = getByRole('alertdialog');
expect(dialog).toBeVisible();
let description = dialog.getAttribute('aria-describedby');
expect(description).toBeDefined();
let content = document.getElementById(description!);
expect(content).toHaveTextContent('Test content');
});

it('accepts custom aria-describedby', async () => {
let {getByRole} = render(
<DialogTrigger>
<ActionButton>Open dialog</ActionButton>
<AlertDialog aria-describedby="content-id" title="Test" primaryActionLabel="Test">
<Content><p id="content-id">Test content</p><p>Extra content</p></Content>
</AlertDialog>
</DialogTrigger>
);

let trigger = getByRole('button');
await user.click(trigger);
act(() => {jest.runAllTimers();});
let dialog = getByRole('alertdialog');
expect(dialog).toBeVisible();
let description = dialog.getAttribute('aria-describedby');
expect(description).toBeDefined();
let content = document.getElementById(description!);
expect(content).toHaveTextContent('Test content');
});
});
106 changes: 106 additions & 0 deletions packages/@react-spectrum/s2/test/StandardDialog.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
* Copyright 2025 Adobe. All rights reserved.
* This file is licensed to you under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. You may obtain a copy
* of the License at http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under
* the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/

import {act, pointerMap, render} from '@react-spectrum/test-utils-internal';
import {ActionButton} from '../src/ActionButton';
import {Button} from '../src/Button';
import {ButtonGroup} from '../src/ButtonGroup';
import {Checkbox} from '../src/Checkbox';
import {Content, Footer, Header, Heading} from '../src/Content';
import {Dialog} from '../src/Dialog';
import {DialogTrigger} from '../src/DialogTrigger';
import React from 'react';
import userEvent from '@testing-library/user-event';

describe('StandardDialog', () => {
let user;
beforeAll(() => {
jest.useFakeTimers();
user = userEvent.setup({delay: null, pointerMap});
});

afterEach(() => {
jest.clearAllMocks();
act(() => jest.runAllTimers());
});

afterAll(function () {
jest.restoreAllMocks();
});

it('does not automatically add aria-describedby', async () => {
let {getByRole} = render(
<DialogTrigger>
<ActionButton>Open dialog</ActionButton>
<Dialog>
{({close}) => (
<>
<Heading slot="title">Dialog title</Heading>
<Header>Header</Header>
<Content>
This is the content of the dialog.
</Content>
<Footer><Checkbox>Don't show this again</Checkbox></Footer>
<ButtonGroup>
<Button onPress={close} variant="secondary">Cancel</Button>
<Button onPress={close} variant="accent">Save</Button>
</ButtonGroup>
</>
)}
</Dialog>
</DialogTrigger>
);

let trigger = getByRole('button');
await user.click(trigger);
act(() => {jest.runAllTimers();});
let dialog = getByRole('dialog');
expect(dialog).toBeVisible();
let description = dialog.getAttribute('aria-describedby');
expect(description).toBeNull();
});

it('accepts custom aria-describedby', async () => {
let {getByRole} = render(
<DialogTrigger>
<ActionButton>Open dialog</ActionButton>
<Dialog aria-describedby="content-id">
{({close}) => (
<>
<Heading slot="title">Dialog title</Heading>
<Header>Header</Header>
<Content>
<p id="content-id">This is the content of the dialog.</p>
<p>Extra content</p>
</Content>
<Footer><Checkbox>Don't show this again</Checkbox></Footer>
<ButtonGroup>
<Button onPress={close} variant="secondary">Cancel</Button>
<Button onPress={close} variant="accent">Save</Button>
</ButtonGroup>
</>
)}
</Dialog>
</DialogTrigger>
);

let trigger = getByRole('button');
await user.click(trigger);
act(() => {jest.runAllTimers();});
let dialog = getByRole('dialog');
expect(dialog).toBeVisible();
let description = dialog.getAttribute('aria-describedby');
expect(description).toBeDefined();
let content = document.getElementById(description!);
expect(content).toHaveTextContent('This is the content of the dialog.');
});
});
4 changes: 2 additions & 2 deletions packages/dev/s2-docs/pages/s2/Dialog.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ function Example(props) {
<Dialog {...props}/* PROPS */>
{({close}) => (
<>
<Image slot="hero" src={heroImage} />
<Image slot="hero" src={heroImage} alt="" />
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Noticed the announcement for these dialogs was bad. The images don't do anything here and are boring gradients, so I made them presentational

<Heading slot="title">Subscribe to our newsletter</Heading>
<Content>
<p>Enter your information to subscribe to our newsletter and receive updates about new features and announcements.</p>
Expand Down Expand Up @@ -76,7 +76,7 @@ function Example(props) {
<Dialog {...props}/* PROPS */>
{({close}) => (
<>
<Image slot="hero" src={heroImage} />
<Image slot="hero" src={heroImage} alt="" />
<Heading slot="title">Dialog Title</Heading>
<Header>Header</Header>
<Content>
Expand Down
Loading
Loading