refactor with suggestions from code review

1. Use role="alertdialog" for each Toast.
2. useToast returns contentProps with role="alert" for container around the icon and the title
3. Fix live region announcement when an Action button is present, by adding aria-relevant="additions" and aria-atomic="true" to contentProps returned by useToast.
4. ToastContainer now renders using an ordered list in reverse order.
5. Fix focus ring rendering around Toast.
6. Fix focus management when the last item in the list is removed, by adding `from` and `accept` to the focusManagerf.ocusPrevious/Next method calls.

packages/@react-aria/toast/src/useToast.ts

@@ -26,8 +26,10 @@ export interface AriaToastProps<T> extends AriaLabelingProps {
 }
 
 export interface ToastAria {
-  /** Props for the toast container element. */
+  /** Props for the toast container, non-modal dialog element. */
   toastProps: DOMAttributes,
+  /** Props for the toast content alert message. */
+  contentProps: DOMAttributes,
   /** Props for the toast title element. */
   titleProps: DOMAttributes,
   /** Props for the toast description element, if any. */
@@ -94,16 +96,18 @@ export function useToast<T>(props: AriaToastProps<T>, state: ToastState<T>, ref:
     // Only move focus when there is another Toast. At this point,
     // state.visibleToasts still includes Toast being removed.
     if (state.visibleToasts?.length > 1) {
-      let nextItemFocused = focusManager.focusNext();
+      const from = document.activeElement?.closest('[role="alertdialog"]') || document.activeElement;
+      const accept = (node:Element) => node.getAttribute('role') === 'alertdialog';
+      let nextItemFocused = focusManager.focusNext({from, accept});
       if (!nextItemFocused || Object.keys(nextItemFocused).length === 0) {
-        focusManager.focusPrevious();
+        focusManager.focusPrevious({from, accept});
       }
     }
   };
 
   return {
     toastProps: {
-      role: 'alert',
+      role: 'alertdialog',
       'aria-label': props['aria-label'],
       'aria-labelledby': props['aria-labelledby'] || titleId,
       'aria-describedby': props['aria-describedby'] || descriptionId,
@@ -112,6 +116,11 @@ export function useToast<T>(props: AriaToastProps<T>, state: ToastState<T>, ref:
       'aria-hidden': animation === 'exiting' ? 'true' : undefined,
       tabIndex: 0
     },
+    contentProps: {
+      role: 'alert',
+      'aria-atomic': 'true',
+      'aria-relevant': 'additions'
+    },
     titleProps: {
       id: titleId
     },

packages/@react-aria/toast/test/useToast.test.js

@@ -27,9 +27,10 @@ describe('useToast', () => {
   };
 
   it('handles defaults', function () {
-    let {closeButtonProps, toastProps, titleProps} = renderToastHook({}, {close});
+    let {closeButtonProps, toastProps, contentProps, titleProps} = renderToastHook({}, {close});
 
-    expect(toastProps.role).toBe('alert');
+    expect(toastProps.role).toBe('alertdialog');
+    expect(contentProps.role).toBe('alert');
     expect(closeButtonProps['aria-label']).toBe('Close');
     expect(typeof closeButtonProps.onPress).toBe('function');
     expect(titleProps.id).toEqual(toastProps['aria-labelledby']);

packages/@react-spectrum/toast/package.json

@@ -46,6 +46,7 @@
     "@react-types/shared": "^3.22.1",
     "@spectrum-icons/ui": "^3.6.5",
     "@swc/helpers": "^0.5.0",
+    "react-aria": "^3.32.1",
     "use-sync-external-store": "^1.2.0"
   },
   "devDependencies": {

packages/@react-spectrum/toast/src/Toast.tsx

@@ -19,11 +19,12 @@ import InfoMedium from '@spectrum-icons/ui/InfoMedium';
 // @ts-ignore
 import intlMessages from '../intl/*.json';
 import {QueuedToast, ToastState} from '@react-stately/toast';
-import React, {useContext} from 'react';
+import React from 'react';
 import styles from '@adobe/spectrum-css-temp/components/toast/vars.css';
 import SuccessMedium from '@spectrum-icons/ui/SuccessMedium';
 import toastContainerStyles from './toastContainer.css';
-import {ToasterContext} from './Toaster';
+import {useFocusRing} from '@react-aria/focus';
+import {useId} from '@react-aria/utils';
 import {useLocalizedStringFormatter} from '@react-aria/i18n';
 import {useToast} from '@react-aria/toast';
 
@@ -66,6 +67,7 @@ function Toast(props: SpectrumToastProps, ref: DOMRef<HTMLDivElement>) {
   let domRef = useDOMRef(ref);
   let {
     closeButtonProps,
+    contentProps,
     titleProps,
     toastProps
   } = useToast(props, state, domRef);
@@ -74,7 +76,7 @@ function Toast(props: SpectrumToastProps, ref: DOMRef<HTMLDivElement>) {
   let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-spectrum/toast');
   let iconLabel = variant && variant !== 'neutral' ? stringFormatter.format(variant) : null;
   let Icon = ICONS[variant];
-  let isFocusVisible = useContext(ToasterContext);
+  let {isFocusVisible, focusProps} = useFocusRing();
 
   const handleAction = () => {
     if (onAction) {
@@ -86,10 +88,16 @@ function Toast(props: SpectrumToastProps, ref: DOMRef<HTMLDivElement>) {
     }
   };
 
+  const iconId = useId();
+  if (Icon) {
+    toastProps['aria-labelledby'] = `${iconId} ${titleProps.id}`;
+  }
+
   return (
     <div
       {...styleProps}
       {...toastProps}
+      {...focusProps}
       ref={domRef}
       className={classNames(styles,
         'spectrum-Toast',
@@ -98,7 +106,7 @@ function Toast(props: SpectrumToastProps, ref: DOMRef<HTMLDivElement>) {
         classNames(
           toastContainerStyles,
           'spectrum-Toast',
-          {'focus-ring': props.toast.key === state.visibleToasts[0]?.key && isFocusVisible}
+          {'focus-ring': isFocusVisible}
         )
       )}
       style={{
@@ -111,24 +119,27 @@ function Toast(props: SpectrumToastProps, ref: DOMRef<HTMLDivElement>) {
           state.remove(key);
         }
       }}>
-      {Icon &&
-        <Icon
-          aria-label={iconLabel}
-          UNSAFE_className={classNames(styles, 'spectrum-Toast-typeIcon')} />
-      }
-      <div className={classNames(styles, 'spectrum-Toast-body')}>
-        <div className={classNames(styles, 'spectrum-Toast-content')} {...titleProps}>{children}</div>
-        {actionLabel &&
-          <Button
-            onPress={handleAction}
-            UNSAFE_className={classNames(styles, 'spectrum-Button')}
-            variant="secondary"
-            staticColor="white">
-            {actionLabel}
-          </Button>
+      <div {...contentProps} className={classNames(toastContainerStyles, 'spectrum-Toast-contentWrapper')}>
+        {Icon &&
+          <Icon
+            id={iconId}
+            aria-label={iconLabel}
+            UNSAFE_className={classNames(styles, 'spectrum-Toast-typeIcon')} />
         }
+        <div className={classNames(styles, 'spectrum-Toast-body')} role="presentation">
+          <div className={classNames(styles, 'spectrum-Toast-content')} role="presentation" {...titleProps}>{children}</div>
+          {actionLabel &&
+            <Button
+              onPress={handleAction}
+              UNSAFE_className={classNames(styles, 'spectrum-Button')}
+              variant="secondary"
+              staticColor="white">
+              {actionLabel}
+            </Button>
+          }
+        </div>
       </div>
-      <div className={classNames(styles, 'spectrum-Toast-buttons')}>
+      <div className={classNames(styles, 'spectrum-Toast-buttons')} role="presentation">
         <ClearButton {...closeButtonProps} variant="overBackground">
           <CrossMedium />
         </ClearButton>

packages/@react-spectrum/toast/src/ToastContainer.tsx

@@ -11,8 +11,10 @@
  */
 
 import {AriaToastRegionProps} from '@react-aria/toast';
+import {classNames} from '@react-spectrum/utils';
 import React, {ReactElement, useEffect, useRef} from 'react';
 import {SpectrumToastValue, Toast} from './Toast';
+import toastContainerStyles from './toastContainer.css';
 import {Toaster} from './Toaster';
 import {ToastOptions, ToastQueue, useToastQueue} from '@react-stately/toast';
 import {useSyncExternalStore} from 'use-sync-external-store/shim/index.js';
@@ -106,12 +108,19 @@ export function ToastContainer(props: SpectrumToastContainerProps): ReactElement
   if (ref === activeToastContainer && state.visibleToasts.length > 0) {
     return (
       <Toaster state={state} {...props}>
-        {state.visibleToasts.map((toast) => (
-          <Toast
-            key={toast.key}
-            toast={toast}
-            state={state} />
-        ))}
+        <ol reversed className={classNames(toastContainerStyles, 'spectrum-Toast-list')}>
+          {state.visibleToasts.map((toast) => (
+            <li
+              key={`${toast.key}-listitem`}
+              aria-hidden={toast.animation === 'exiting' ? 'true' : undefined}
+              className={classNames(toastContainerStyles, 'spectrum-Toast-listitem')}>
+              <Toast
+                key={toast.key}
+                toast={toast}
+                state={state} />
+            </li>
+          ))}
+        </ol >
       </Toaster>
     );
   }

packages/@react-spectrum/toast/src/toastContainer.css

@@ -72,6 +72,9 @@
   --spectrum-focus-ring-gap: var(--spectrum-alias-focus-ring-gap);
   --spectrum-focus-ring-size: var(--spectrum-alias-focus-ring-size);
 
+  position: relative;
+  outline: none;
+
   &[data-animation=entering] {
     animation: slide-in 300ms;
   }
@@ -80,6 +83,23 @@
     animation: fade-out 300ms forwards;
   }
 }
+.spectrum-Toast-list {
+  display: contents;
+  list-style-type: none;
+  margin-block-start: 0;
+  margin-block-end: 0;
+  margin-inline-start: 0;
+  margin-inline-end: 0;
+  padding-inline-start: 0;
+}
+
+.spectrum-Toast-listitem {
+  display: inline-block;
+}
+
+.spectrum-Toast-contentWrapper {
+  display: contents;
+}
 
 @keyframes slide-in {
   from {

packages/@react-spectrum/toast/test/ToastContainer.test.js

@@ -64,20 +64,24 @@ describe('Toast Provider and Container', function () {
     let {getByRole, queryByRole} = renderComponent(<RenderToastButton />);
     let button = getByRole('button');
 
+    expect(queryByRole('alertdialog')).toBeNull();
     expect(queryByRole('alert')).toBeNull();
     triggerPress(button);
 
     let region = getByRole('region');
     expect(region).toHaveAttribute('aria-label', 'Notifications');
 
-    let alert = getByRole('alert');
+    let toast = getByRole('alertdialog');
+    let alert = within(toast).getByRole('alert');
+    expect(toast).toBeVisible();
     expect(alert).toBeVisible();
 
-    button = within(alert).getByRole('button');
+    button = within(toast).getByRole('button');
     expect(button).toHaveAttribute('aria-label', 'Close');
     triggerPress(button);
 
     fireAnimationEnd(alert);
+    expect(queryByRole('alertdialog')).toBeNull();
     expect(queryByRole('alert')).toBeNull();
   });
 
@@ -86,9 +90,12 @@ describe('Toast Provider and Container', function () {
     let button = getByRole('button');
     triggerPress(button);
 
-    let alert = getByRole('alert');
+    let toast = getByRole('alertdialog');
+    let alert = within(toast).getByRole('alert');
     let icon = within(alert).getByRole('img');
     expect(icon).toHaveAttribute('aria-label', 'Success');
+    let title = within(alert).getByText('Toast is default');
+    expect(toast).toHaveAttribute('aria-labelledby', `${icon.id} ${title.id}`);
   });
 
   it('removes a toast via timeout', () => {
@@ -97,7 +104,7 @@ describe('Toast Provider and Container', function () {
 
     triggerPress(button);
 
-    let toast = getByRole('alert');
+    let toast = getByRole('alertdialog');
     expect(toast).toBeVisible();
 
     act(() => jest.advanceTimersByTime(1000));
@@ -107,7 +114,7 @@ describe('Toast Provider and Container', function () {
     expect(toast).toHaveAttribute('data-animation', 'exiting');
 
     fireAnimationEnd(toast);
-    expect(queryByRole('alert')).toBeNull();
+    expect(queryByRole('alertdialog')).toBeNull();
   });
 
   it('pauses timers when hovering', async () => {
@@ -116,7 +123,7 @@ describe('Toast Provider and Container', function () {
 
     triggerPress(button);
 
-    let toast = getByRole('alert');
+    let toast = getByRole('alertdialog');
     expect(toast).toBeVisible();
 
     act(() => jest.advanceTimersByTime(1000));
@@ -131,7 +138,7 @@ describe('Toast Provider and Container', function () {
     expect(toast).toHaveAttribute('data-animation', 'exiting');
 
     fireAnimationEnd(toast);
-    expect(queryByRole('alert')).toBeNull();
+    expect(queryByRole('alertdialog')).toBeNull();
   });
 
   it('pauses timers when focusing', () => {
@@ -140,7 +147,7 @@ describe('Toast Provider and Container', function () {
 
     triggerPress(button);
 
-    let toast = getByRole('alert');
+    let toast = getByRole('alertdialog');
     expect(toast).toBeVisible();
 
     act(() => jest.advanceTimersByTime(1000));
@@ -155,7 +162,7 @@ describe('Toast Provider and Container', function () {
     expect(toast).toHaveAttribute('data-animation', 'exiting');
 
     fireAnimationEnd(toast);
-    expect(queryByRole('alert')).toBeNull();
+    expect(queryByRole('alertdialog')).toBeNull();
   });
 
   it('renders a toast with an action', () => {
@@ -164,13 +171,15 @@ describe('Toast Provider and Container', function () {
     let {getByRole, queryByRole} = renderComponent(<RenderToastButton actionLabel="Action" onAction={onAction} onClose={onClose} />);
     let button = getByRole('button');
 
-    expect(queryByRole('alert')).toBeNull();
+    expect(queryByRole('alertdialog')).toBeNull();
     triggerPress(button);
 
-    let alert = getByRole('alert');
+    let toast = getByRole('alertdialog');
+    let alert = within(toast).getByRole('alert');
+    expect(toast).toBeVisible();
     expect(alert).toBeVisible();
 
-    let buttons = within(alert).getAllByRole('button');
+    let buttons = within(toast).getAllByRole('button');
     expect(buttons[0]).toHaveTextContent('Action');
     triggerPress(buttons[0]);
 
@@ -184,22 +193,24 @@ describe('Toast Provider and Container', function () {
     let {getByRole, queryByRole} = renderComponent(<RenderToastButton actionLabel="Action" onAction={onAction} onClose={onClose} shouldCloseOnAction />);
     let button = getByRole('button');
 
-    expect(queryByRole('alert')).toBeNull();
+    expect(queryByRole('alertdialog')).toBeNull();
     triggerPress(button);
 
-    let alert = getByRole('alert');
+    let toast = getByRole('alertdialog');
+    let alert = within(toast).getByRole('alert');
+    expect(toast).toBeVisible();
     expect(alert).toBeVisible();
 
-    let buttons = within(alert).getAllByRole('button');
+    let buttons = within(toast).getAllByRole('button');
     expect(buttons[0]).toHaveTextContent('Action');
     triggerPress(buttons[0]);
 
     expect(onAction).toHaveBeenCalledTimes(1);
     expect(onClose).toHaveBeenCalledTimes(1);
 
-    expect(alert).toHaveAttribute('data-animation', 'exiting');
-    fireAnimationEnd(alert);
-    expect(queryByRole('alert')).toBeNull();
+    expect(toast).toHaveAttribute('data-animation', 'exiting');
+    fireAnimationEnd(toast);
+    expect(queryByRole('alertdialog')).toBeNull();
   });
 
   it('can focus toast region using F6', () => {
@@ -208,7 +219,7 @@ describe('Toast Provider and Container', function () {
 
     triggerPress(button);
 
-    let toast = getByRole('alert');
+    let toast = getByRole('alertdialog');
     expect(toast).toBeVisible();
 
     expect(document.activeElement).toBe(button);
@@ -225,13 +236,13 @@ describe('Toast Provider and Container', function () {
 
     triggerPress(button);
 
-    let toast = getByRole('alert');
+    let toast = getByRole('alertdialog');
     let closeButton = within(toast).getByRole('button');
     act(() => closeButton.focus());
 
     triggerPress(closeButton);
     fireAnimationEnd(toast);
-    expect(queryByRole('alert')).toBeNull();
+    expect(queryByRole('alertdialog')).toBeNull();
     expect(document.activeElement).toBe(button);
   });
 
@@ -242,19 +253,19 @@ describe('Toast Provider and Container', function () {
     triggerPress(button);
     triggerPress(button);
 
-    let toast = getAllByRole('alert')[0];
+    let toast = getAllByRole('alertdialog')[0];
     let closeButton = within(toast).getByRole('button');
     triggerPress(closeButton);
     fireAnimationEnd(toast);
 
-    toast = getByRole('alert');
+    toast = getByRole('alertdialog');
     expect(document.activeElement).toBe(toast);
 
     closeButton = within(toast).getByRole('button');
     triggerPress(closeButton);
     fireAnimationEnd(toast);
 
-    expect(queryByRole('alert')).toBeNull();
+    expect(queryByRole('alertdialog')).toBeNull();
     expect(document.activeElement).toBe(button);
   });
 
@@ -283,11 +294,14 @@ describe('Toast Provider and Container', function () {
 
     triggerPress(button);
 
-    let toast = getByRole('alert');
+    let toast = getByRole('alertdialog');
+    let alert = within(toast).getByRole('alert');
     expect(toast).toBeVisible();
+    expect(alert).toBeVisible();
 
     triggerPress(button);
     fireAnimationEnd(toast);
+    expect(queryByRole('alertdialog')).toBeNull();
     expect(queryByRole('alert')).toBeNull();
   });