feat: focus on surface if no focusable element is available (#25173)

* feat: focus on surface if no focusable element is available * chore: updates e2e tests * chore: add better warn
microsoft · Oct 13, 2022 · 12222c2 · 12222c2
1 parent c7ef69f
commit 12222c2
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 10 deletions.
diff --git a/change/@fluentui-react-dialog-1fe6ad25-f33a-4650-a51b-837a5bed695e.json b/change/@fluentui-react-dialog-1fe6ad25-f33a-4650-a51b-837a5bed695e.json
@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "feat: focus on surface if no focusable element is available",
+  "packageName": "@fluentui/react-dialog",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/react-components/react-dialog/e2e/Dialog.e2e.tsx b/packages/react-components/react-dialog/e2e/Dialog.e2e.tsx
@@ -118,7 +118,7 @@ describe('Dialog', () => {
     cy.get(dialogTriggerOpenSelector).realClick();
     cy.get(dialogTriggerCloseSelector).should('be.focused');
   });
-  it('should focus on body if no focusabled element in dialog', () => {
+  it('should focus on dialog surface if no focusable element in dialog', () => {
     mount(
       <Dialog>
         <DialogTrigger>
@@ -137,7 +137,7 @@ describe('Dialog', () => {
       </Dialog>,
     );
     cy.get(dialogTriggerOpenSelector).realClick();
-    cy.focused().should('not.exist');
+    cy.get(dialogSurfaceSelector).should('be.focused');
   });
   it('should focus back on trigger when dialog closed', () => {
     mount(

diff --git a/packages/react-components/react-dialog/src/components/DialogSurface/useDialogSurface.ts b/packages/react-components/react-dialog/src/components/DialogSurface/useDialogSurface.ts
@@ -80,6 +80,7 @@ export const useDialogSurface_unstable = (
       },
     }),
     root: getNativeElementProps(as ?? 'div', {
+      tabIndex: -1, // https://github.com/microsoft/fluentui/issues/25150
       'aria-modal': modalType !== 'non-modal',
       role: modalType === 'alert' ? 'alertdialog' : 'dialog',
       'aria-describedby': dialogContentId,

diff --git a/...ages/react-components/react-dialog/src/components/DialogSurface/useDialogSurfaceStyles.ts b/...ages/react-components/react-dialog/src/components/DialogSurface/useDialogSurfaceStyles.ts
@@ -1,6 +1,7 @@
 import { makeStyles, mergeClasses, shorthands } from '@griffel/react';
 import type { SlotClassNames } from '@fluentui/react-utilities';
 import { tokens } from '@fluentui/react-theme';
+import { createFocusOutlineStyle } from '@fluentui/react-tabster';
 import {
   MEDIA_QUERY_BREAKPOINT_SELECTOR,
   SURFACE_BORDER_RADIUS,
@@ -19,6 +20,7 @@ export const dialogSurfaceClassNames: SlotClassNames<DialogSurfaceSlots> = {
  * Styles for the root slot
  */
 const useStyles = makeStyles({
+  focusOutline: createFocusOutlineStyle(),
   root: {
     display: 'block',
     userSelect: 'unset',
@@ -72,6 +74,7 @@ export const useDialogSurfaceStyles_unstable = (state: DialogSurfaceState): Dial
   state.root.className = mergeClasses(
     dialogSurfaceClassNames.root,
     styles.root,
+    styles.focusOutline,
     isNestedDialog && styles.nestedNativeDialogBackdrop,
     state.root.className,
   );

diff --git a/...ges/react-components/react-dialog/src/stories/Dialog/DialogNoFocusableElement.stories.tsx b/...ges/react-components/react-dialog/src/stories/Dialog/DialogNoFocusableElement.stories.tsx
@@ -16,7 +16,7 @@ export const NoFocusableElement = () => {
             <DialogTitle>Dialog Title</DialogTitle>
             <DialogContent>
               <p>⛔️ A Dialog without focusable elements is not recommended!</p>
-              <p>⛔️ Escape key doesn't work</p>
+              <p>✅ Escape key works</p>
               <p>✅ Backdrop click still works to ensure this modal can be closed</p>
             </DialogContent>
           </DialogBody>
@@ -30,9 +30,8 @@ export const NoFocusableElement = () => {
           <DialogBody>
             <DialogTitle action={null}>Dialog Title</DialogTitle>
             <DialogContent>
-              <p>⛔️ A Dialog without focusable elements is not recommended!</p>
-              <p>⛔️ Escape key doesn't work</p>
-              <p>⛔️ you're trapped!</p>
+              <p>⛔️ A modal Dialog without focusable elements is not recommended!</p>
+              <p>✅ Escape key works</p>
             </DialogContent>
           </DialogBody>
         </DialogSurface>

diff --git a/packages/react-components/react-dialog/src/utils/useFocusFirstElement.ts b/packages/react-components/react-dialog/src/utils/useFocusFirstElement.ts
@@ -21,10 +21,17 @@ export function useFocusFirstElement(open: boolean, modalType: DialogModalType)
     const element = dialogRef.current && findFirstFocusable(dialogRef.current);
     if (element) {
       element.focus();
-    } else if (process.env.NODE_ENV !== 'production') {
-      triggerRef.current?.blur();
-      // eslint-disable-next-line no-console
-      console.warn('A Dialog should have at least one focusable element inside DialogSurface');
+    } else {
+      dialogRef.current?.focus(); // https://github.com/microsoft/fluentui/issues/25150
+      if (process.env.NODE_ENV !== 'production') {
+        // eslint-disable-next-line no-console
+        console.warn(
+          [
+            '@fluentui/react-dialog: a Dialog should have at least one focusable element inside DialogSurface.',
+            'Please add at least a close button either on `DialogTitle` action slot or inside `DialogActions`',
+          ].join('\n'),
+        );
+      }
     }
   }, [findFirstFocusable, open, modalType, targetDocument]);