feat(react-dialog): removes aria-haspopup (#25611)

* feat(react-dialog): removes aria-haspopup * chore: updates snapshot * chore: updates e2e trigger selectors
microsoft · Nov 14, 2022 · a510bcc · a510bcc
1 parent 2c00485
commit a510bcc
Show file tree

Hide file tree

Showing 16 changed files with 68 additions and 47 deletions.
diff --git a/change/@fluentui-react-dialog-cfbd0be5-81c5-4351-9f00-8931078078c2.json b/change/@fluentui-react-dialog-cfbd0be5-81c5-4351-9f00-8931078078c2.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "feat: removes aria-haspopup",
+  "packageName": "@fluentui/react-dialog",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/react-components/react-dialog/src/components/Dialog/Dialog.cy.tsx b/packages/react-components/react-dialog/src/components/Dialog/Dialog.cy.tsx
@@ -29,6 +29,7 @@ import {
   dialogSurfaceSelector,
   dialogTriggerCloseId,
   dialogTriggerCloseSelector,
+  dialogTriggerOpenId,
   dialogTriggerOpenSelector,
 } from '../../testing/selectors';
 
@@ -39,7 +40,7 @@ describe('Dialog', () => {
     mount(
       <Dialog>
         <DialogTrigger disableButtonEnhancement>
-          <Button>Open dialog</Button>
+          <Button id={dialogTriggerOpenId}>Open dialog</Button>
         </DialogTrigger>
         <DialogSurface>
           <DialogBody>
@@ -51,7 +52,9 @@ describe('Dialog', () => {
             </DialogContent>
             <DialogActions>
               <DialogTrigger disableButtonEnhancement>
-                <Button appearance="secondary">Close</Button>
+                <Button id={dialogTriggerCloseId} appearance="secondary">
+                  Close
+                </Button>
               </DialogTrigger>
               <Button appearance="primary">Do Something</Button>
             </DialogActions>
@@ -66,7 +69,7 @@ describe('Dialog', () => {
     mount(
       <Dialog>
         <DialogTrigger disableButtonEnhancement>
-          <Button>Open dialog</Button>
+          <Button id={dialogTriggerOpenId}>Open dialog</Button>
         </DialogTrigger>
         <DialogSurface>
           <DialogBody>
@@ -78,7 +81,9 @@ describe('Dialog', () => {
             </DialogContent>
             <DialogActions>
               <DialogTrigger disableButtonEnhancement>
-                <Button appearance="secondary">Close</Button>
+                <Button id={dialogTriggerCloseId} appearance="secondary">
+                  Close
+                </Button>
               </DialogTrigger>
               <Button appearance="primary">Do Something</Button>
             </DialogActions>
@@ -93,7 +98,7 @@ describe('Dialog', () => {
     mount(
       <Dialog>
         <DialogTrigger disableButtonEnhancement>
-          <Button>Open dialog</Button>
+          <Button id={dialogTriggerOpenId}>Open dialog</Button>
         </DialogTrigger>
         <DialogSurface>
           <DialogBody>
@@ -122,7 +127,7 @@ describe('Dialog', () => {
     mount(
       <Dialog>
         <DialogTrigger disableButtonEnhancement>
-          <Button>Open dialog</Button>
+          <Button id={dialogTriggerOpenId}>Open dialog</Button>
         </DialogTrigger>
         <DialogSurface id="dialog-surface">
           <DialogBody>
@@ -143,7 +148,7 @@ describe('Dialog', () => {
     mount(
       <Dialog>
         <DialogTrigger disableButtonEnhancement>
-          <Button>Open dialog</Button>
+          <Button id={dialogTriggerOpenId}>Open dialog</Button>
         </DialogTrigger>
         <DialogSurface>
           <DialogBody>
@@ -182,7 +187,7 @@ describe('Dialog', () => {
         //eslint-disable-next-line react/jsx-no-bind
         <Dialog open={open} onOpenChange={(event, data) => setOpen(data.open)}>
           <DialogTrigger disableButtonEnhancement>
-            <Button>Open dialog</Button>
+            <Button id={dialogTriggerOpenId}>Open dialog</Button>
           </DialogTrigger>
           <DialogSurface>
             <DialogBody>
@@ -212,7 +217,7 @@ describe('Dialog', () => {
     mount(
       <Dialog modalType="non-modal">
         <DialogTrigger disableButtonEnhancement>
-          <Button>Open dialog</Button>
+          <Button id={dialogTriggerOpenId}>Open dialog</Button>
         </DialogTrigger>
         <DialogSurface>
           <DialogBody>
@@ -270,7 +275,7 @@ describe('Dialog', () => {
       mount(
         <Dialog modalType="modal">
           <DialogTrigger disableButtonEnhancement>
-            <Button>Open dialog</Button>
+            <Button id={dialogTriggerOpenId}>Open dialog</Button>
           </DialogTrigger>
           <DialogSurface>
             <DialogBody>
@@ -282,7 +287,9 @@ describe('Dialog', () => {
               </DialogContent>
               <DialogActions>
                 <DialogTrigger disableButtonEnhancement>
-                  <Button appearance="secondary">Close</Button>
+                  <Button id={dialogTriggerCloseId} appearance="secondary">
+                    Close
+                  </Button>
                 </DialogTrigger>
                 <Button appearance="primary">Do Something</Button>
               </DialogActions>
@@ -298,7 +305,7 @@ describe('Dialog', () => {
       mount(
         <Dialog modalType="modal">
           <DialogTrigger disableButtonEnhancement>
-            <Button>Open dialog</Button>
+            <Button id={dialogTriggerOpenId}>Open dialog</Button>
           </DialogTrigger>
           <DialogSurface>
             <DialogTitle>Dialog title</DialogTitle>
@@ -309,7 +316,9 @@ describe('Dialog', () => {
             </DialogBody>
             <DialogActions>
               <DialogTrigger disableButtonEnhancement>
-                <Button appearance="secondary">Close</Button>
+                <Button id={dialogTriggerCloseId} appearance="secondary">
+                  Close
+                </Button>
               </DialogTrigger>
               <Button appearance="primary">Do Something</Button>
             </DialogActions>
@@ -325,7 +334,7 @@ describe('Dialog', () => {
       mount(
         <Dialog modalType="non-modal">
           <DialogTrigger disableButtonEnhancement>
-            <Button>Open dialog</Button>
+            <Button id={dialogTriggerOpenId}>Open dialog</Button>
           </DialogTrigger>
           <DialogSurface>
             <DialogBody>
@@ -337,7 +346,9 @@ describe('Dialog', () => {
               </DialogContent>
               <DialogActions>
                 <DialogTrigger disableButtonEnhancement>
-                  <Button appearance="secondary">Close</Button>
+                  <Button id={dialogTriggerCloseId} appearance="secondary">
+                    Close
+                  </Button>
                 </DialogTrigger>
                 <Button appearance="primary">Do Something</Button>
               </DialogActions>
@@ -353,7 +364,7 @@ describe('Dialog', () => {
       mount(
         <Dialog modalType="non-modal">
           <DialogTrigger disableButtonEnhancement>
-            <Button>Open dialog</Button>
+            <Button id={dialogTriggerOpenId}>Open dialog</Button>
           </DialogTrigger>
           <DialogSurface>
             <DialogBody>
@@ -365,7 +376,9 @@ describe('Dialog', () => {
               </DialogContent>
               <DialogActions>
                 <DialogTrigger disableButtonEnhancement>
-                  <Button appearance="secondary">Close</Button>
+                  <Button id={dialogTriggerCloseId} appearance="secondary">
+                    Close
+                  </Button>
                 </DialogTrigger>
                 <Button appearance="primary">Do Something</Button>
               </DialogActions>
@@ -382,7 +395,7 @@ describe('Dialog', () => {
       mount(
         <Dialog modalType="alert">
           <DialogTrigger disableButtonEnhancement>
-            <Button>Open dialog</Button>
+            <Button id={dialogTriggerOpenId}>Open dialog</Button>
           </DialogTrigger>
           <DialogSurface>
             <DialogBody>
@@ -394,7 +407,9 @@ describe('Dialog', () => {
               </DialogContent>
               <DialogActions>
                 <DialogTrigger disableButtonEnhancement>
-                  <Button appearance="secondary">Close</Button>
+                  <Button id={dialogTriggerCloseId} appearance="secondary">
+                    Close
+                  </Button>
                 </DialogTrigger>
                 <Button appearance="primary">Do Something</Button>
               </DialogActions>
@@ -410,7 +425,7 @@ describe('Dialog', () => {
       mount(
         <Dialog modalType="alert">
           <DialogTrigger disableButtonEnhancement>
-            <Button>Open dialog</Button>
+            <Button id={dialogTriggerOpenId}>Open dialog</Button>
           </DialogTrigger>
           <DialogSurface>
             <DialogBody>
@@ -422,7 +437,9 @@ describe('Dialog', () => {
               </DialogContent>
               <DialogActions>
                 <DialogTrigger disableButtonEnhancement>
-                  <Button appearance="secondary">Close</Button>
+                  <Button id={dialogTriggerCloseId} appearance="secondary">
+                    Close
+                  </Button>
                 </DialogTrigger>
                 <Button appearance="primary">Do Something</Button>
               </DialogActions>

diff --git a/packages/react-components/react-dialog/src/components/Dialog/useDialog.ts b/packages/react-components/react-dialog/src/components/Dialog/useDialog.ts
@@ -54,7 +54,6 @@ export const useDialog_unstable = (props: DialogProps): DialogState => {
     content: open ? content : null,
     trigger,
     requestOpenChange,
-    dialogContentId: useId('dialog-content-'),
     dialogTitleId: useId('dialog-title-'),
     isNestedDialog: useHasParentContext(DialogContext),
     dialogRef: focusRef,

diff --git a/packages/react-components/react-dialog/src/components/Dialog/useDialogContextValues.ts b/packages/react-components/react-dialog/src/components/Dialog/useDialogContextValues.ts
@@ -2,7 +2,7 @@ import type { DialogContextValue, DialogSurfaceContextValue } from '../../contex
 import type { DialogContextValues, DialogState } from './Dialog.types';
 
 export function useDialogContextValues_unstable(state: DialogState): DialogContextValues {
-  const { modalType, open, dialogContentId, dialogRef, dialogTitleId, isNestedDialog, requestOpenChange } = state;
+  const { modalType, open, dialogRef, dialogTitleId, isNestedDialog, requestOpenChange } = state;
 
   /**
    * This context is created with "@fluentui/react-context-selector",
@@ -12,7 +12,6 @@ export function useDialogContextValues_unstable(state: DialogState): DialogConte
     open,
     modalType,
     dialogRef,
-    dialogContentId,
     dialogTitleId,
     isNestedDialog,
     requestOpenChange,

diff --git a/packages/react-components/react-dialog/src/components/DialogContent/useDialogContent.ts b/packages/react-components/react-dialog/src/components/DialogContent/useDialogContent.ts
@@ -1,6 +1,5 @@
 import * as React from 'react';
 import { getNativeElementProps } from '@fluentui/react-utilities';
-import { useDialogContext_unstable } from '../../contexts';
 import { DialogContentProps, DialogContentState } from './DialogContent.types';
 
 /**
@@ -16,14 +15,12 @@ export const useDialogContent_unstable = (
   props: DialogContentProps,
   ref: React.Ref<HTMLElement>,
 ): DialogContentState => {
-  const dialogContentId = useDialogContext_unstable(ctx => ctx.dialogContentId);
   return {
     components: {
       root: 'div',
     },
     root: getNativeElementProps(props.as ?? 'div', {
       ref,
-      id: dialogContentId,
       ...props,
     }),
   };

diff --git a/packages/react-components/react-dialog/src/components/DialogSurface/useDialogSurface.ts b/packages/react-components/react-dialog/src/components/DialogSurface/useDialogSurface.ts
@@ -35,7 +35,6 @@ export const useDialogSurface_unstable = (
   const open = useDialogContext_unstable(ctx => ctx.open);
   const requestOpenChange = useDialogContext_unstable(ctx => ctx.requestOpenChange);
   const dialogTitleID = useDialogContext_unstable(ctx => ctx.dialogTitleId);
-  const dialogContentId = useDialogContext_unstable(ctx => ctx.dialogContentId);
 
   const handledBackdropClick = useEventCallback((event: React.MouseEvent<HTMLDivElement>) => {
     if (isResolvedShorthand(props.backdrop)) {
@@ -83,7 +82,6 @@ export const useDialogSurface_unstable = (
       tabIndex: -1, // https://github.com/microsoft/fluentui/issues/25150
       'aria-modal': modalType !== 'non-modal',
       role: modalType === 'alert' ? 'alertdialog' : 'dialog',
-      'aria-describedby': dialogContentId,
       'aria-labelledby': props['aria-label'] ? undefined : dialogTitleID,
       ...props,
       ...modalAttributes,

diff --git a/packages/react-components/react-dialog/src/components/DialogTitle/DialogTitle.cy.tsx b/packages/react-components/react-dialog/src/components/DialogTitle/DialogTitle.cy.tsx
@@ -14,7 +14,7 @@ import {
   DialogTrigger,
 } from '@fluentui/react-dialog';
 import { Button } from '@fluentui/react-components';
-import { dialogActionSelector, dialogTriggerOpenSelector } from '../../testing/selectors';
+import { dialogActionSelector, dialogTriggerOpenId, dialogTriggerOpenSelector } from '../../testing/selectors';
 
 const mount = (element: JSX.Element) => mountBase(<FluentProvider theme={teamsLightTheme}>{element}</FluentProvider>);
 
@@ -24,7 +24,7 @@ describe('DialogTitle', () => {
       mount(
         <Dialog modalType="modal">
           <DialogTrigger disableButtonEnhancement>
-            <Button>Open dialog</Button>
+            <Button id={dialogTriggerOpenId}>Open dialog</Button>
           </DialogTrigger>
           <DialogSurface>
             <DialogBody>
@@ -53,7 +53,7 @@ describe('DialogTitle', () => {
       mount(
         <Dialog modalType="non-modal">
           <DialogTrigger disableButtonEnhancement>
-            <Button>Open dialog</Button>
+            <Button id={dialogTriggerOpenId}>Open dialog</Button>
           </DialogTrigger>
           <DialogSurface>
             <DialogBody>
@@ -82,7 +82,7 @@ describe('DialogTitle', () => {
       mount(
         <Dialog modalType="alert">
           <DialogTrigger disableButtonEnhancement>
-            <Button>Open dialog</Button>
+            <Button id={dialogTriggerOpenId}>Open dialog</Button>
           </DialogTrigger>
           <DialogSurface>
             <DialogBody>

diff --git a/packages/react-components/react-dialog/src/components/DialogTrigger/DialogTrigger.test.tsx b/packages/react-components/react-dialog/src/components/DialogTrigger/DialogTrigger.test.tsx
@@ -56,7 +56,7 @@ describe('DialogTrigger', () => {
     expect(ref.mock.calls[0]).toMatchInlineSnapshot(`
       Array [
         <button
-          aria-haspopup="dialog"
+          aria-expanded="false"
           data-tabster="{\\"deloser\\":{}}"
         >
           Trigger
@@ -86,7 +86,7 @@ describe('DialogTrigger', () => {
     expect(cb.mock.calls[0]).toMatchInlineSnapshot(`
       Array [
         <button
-          aria-haspopup="dialog"
+          aria-expanded="false"
           data-tabster="{\\"deloser\\":{}}"
         >
           Trigger

diff --git a/...nents/react-dialog/src/components/DialogTrigger/__snapshots__/DialogTrigger.test.tsx.snap b/...nents/react-dialog/src/components/DialogTrigger/__snapshots__/DialogTrigger.test.tsx.snap
@@ -2,7 +2,7 @@
 
 exports[`DialogTrigger renders a default state 1`] = `
 <button
-  aria-haspopup="dialog"
+  aria-expanded={false}
   data-tabster="{\\"deloser\\":{}}"
   onClick={[Function]}
 >

diff --git a/packages/react-components/react-dialog/src/components/DialogTrigger/useDialogTrigger.ts b/packages/react-components/react-dialog/src/components/DialogTrigger/useDialogTrigger.ts
@@ -19,6 +19,7 @@ export const useDialogTrigger_unstable = (props: DialogTriggerProps): DialogTrig
   const child = getTriggerChild(children);
 
   const requestOpenChange = useDialogContext_unstable(ctx => ctx.requestOpenChange);
+  const open = useDialogContext_unstable(ctx => ctx.open);
 
   const { triggerAttributes } = useModalAttributes();
 
@@ -37,7 +38,7 @@ export const useDialogTrigger_unstable = (props: DialogTriggerProps): DialogTrig
 
   const triggerChildProps = {
     ...child?.props,
-    'aria-haspopup': action === 'close' ? undefined : 'dialog',
+    'aria-expanded': open,
     ref: child?.ref,
     onClick: handleClick,
     ...triggerAttributes,

diff --git a/packages/react-components/react-dialog/src/contexts/dialogContext.ts b/packages/react-components/react-dialog/src/contexts/dialogContext.ts
@@ -6,7 +6,6 @@ import type { DialogModalType, DialogOpenChangeData } from '../Dialog';
 
 export type DialogContextValue = {
   open: boolean;
-  dialogContentId?: string;
   dialogTitleId?: string;
   isNestedDialog: boolean;
   dialogRef: React.Ref<DialogSurfaceElement>;

diff --git a/packages/react-components/react-dialog/src/testing/selectors.ts b/packages/react-components/react-dialog/src/testing/selectors.ts
@@ -1,10 +1,11 @@
 import { dialogSurfaceClassNames, dialogTitleClassNames } from '@fluentui/react-dialog';
 
+export const dialogTriggerOpenId = 'open-btn';
 export const dialogTriggerCloseId = 'close-btn';
 
 export const dialogBackdropSelector = `.${dialogSurfaceClassNames.backdrop}`;
 export const dialogSurfaceSelector = `.${dialogSurfaceClassNames.root}`;
-export const dialogTriggerOpenSelector = `[aria-haspopup="dialog"]`;
+export const dialogTriggerOpenSelector = `#${dialogTriggerOpenId}`;
 export const dialogTriggerCloseSelector = `#${dialogTriggerCloseId}`;
 export const dialogTitleSelector = `.${dialogTitleClassNames.root}`;
 export const dialogActionSelector = `.${dialogTitleClassNames.action}`;
diff --git a/packages/react-components/react-dialog/stories/Dialog/DialogBestPractices.md b/packages/react-components/react-dialog/stories/Dialog/DialogBestPractices.md
@@ -8,7 +8,7 @@
 - Dialog boxes consist of a header (`DialogTitle`), content (`DialogContent`), and footer (`DialogActions`), which should all be included inside a body (`DialogBody`).
 - Validate that people’s entries are acceptable before closing the dialog. Show an inline validation error near the field they must correct.
 - Modal dialogs should be used very sparingly—only when it’s critical that people make a choice or provide information before they can proceed. Thee dialogs are generally used for irreversible or potentially destructive tasks. They’re typically paired with an backdrop without a light dismiss.
-- `DialogSurface` by default has `aria-describedby="dialog-content-id"`, which might not be ideal with complex `DialogContent`, on those scenarios (for example on [with form](#with-form)), it is recommended to set `aria-describedby={undefined}`
+- Add a `aria-describedby` attribute on `DialogSurface` pointing to the dialog content on short confirmation like dialogs.
 
 ### Don't