Skip to content
Merged
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "chore(react-dialog): removes document listener to Escape keydown",
"packageName": "@fluentui/react-dialog",
"email": "bernardo.sunderhus@gmail.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,6 @@ export type DialogOpenChangeData = {
type: 'escapeKeyDown';
open: boolean;
event: React_2.KeyboardEvent;
}
/**
* document escape keydown defers from internal escape keydown events because of the synthetic event API
*/
| {
type: 'documentEscapeKeyDown';
open: boolean;
event: KeyboardEvent;
} | {
type: 'backdropClick';
open: boolean;
Expand All @@ -90,7 +82,7 @@ export type DialogOpenChangeData = {
};

// @public (undocumented)
export type DialogOpenChangeEvent = React_2.KeyboardEvent | React_2.MouseEvent | KeyboardEvent;
export type DialogOpenChangeEvent = React_2.KeyboardEvent | React_2.MouseEvent;

// @public (undocumented)
export type DialogProps = ComponentProps<Partial<DialogSlots>> & {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,10 @@ export type DialogSlots = {
backdrop?: Slot<'div'>;
};

export type DialogOpenChangeEvent = React.KeyboardEvent | React.MouseEvent | KeyboardEvent;
export type DialogOpenChangeEvent = React.KeyboardEvent | React.MouseEvent;

export type DialogOpenChangeData =
| { type: 'escapeKeyDown'; open: boolean; event: React.KeyboardEvent }
/**
* document escape keydown defers from internal escape keydown events because of the synthetic event API
*/
| { type: 'documentEscapeKeyDown'; open: boolean; event: KeyboardEvent }
| { type: 'backdropClick'; open: boolean; event: React.MouseEvent }
| { type: 'triggerClick'; open: boolean; event: React.MouseEvent };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
} from '@fluentui/react-utilities';
import { useFocusFinders } from '@fluentui/react-tabster';
import { useFluent_unstable } from '@fluentui/react-shared-contexts';
import { normalizeDefaultPrevented, isEscapeKeyDismiss, useDisableBodyScroll } from '../../utils';
import { normalizeDefaultPrevented, useDisableBodyScroll } from '../../utils';

import type { DialogProps, DialogState, DialogModalType, DialogOpenChangeData } from './Dialog.types';

Expand Down Expand Up @@ -171,21 +171,6 @@ function useFocusFirstElement({
if (targetDocument.activeElement === trigger) {
trigger.blur();
}
const listener = (event: KeyboardEvent) => {
if (isEscapeKeyDismiss(event, modalType)) {
requestOpenChange({
event,
open: false,
type: 'documentEscapeKeyDown',
});
trigger.focus();
event.stopImmediatePropagation();
}
};
targetDocument.addEventListener('keydown', listener, { passive: false });
return () => {
targetDocument.removeEventListener('keydown', listener);
};
}
}, [findFirstFocusable, requestOpenChange, open, modalType, targetDocument]);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
A `Dialog` **should** always have at least one focusable element.
In the case no focusable element is present inside a `Dialog` the only method
that would close the `Dialog` would be clicking on the `backdrop`
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import * as React from 'react';
import { Dialog, DialogTrigger, DialogSurface, DialogTitle, DialogBody } from '@fluentui/react-dialog';
import { Button } from '@fluentui/react-components';

import story from './DialogNoFocusableElement.md';

export const NoFocusableElement = () => {
return (
<>
Expand All @@ -12,8 +14,9 @@ export const NoFocusableElement = () => {
<DialogSurface>
<DialogTitle>Dialog Title</DialogTitle>
<DialogBody>
<p>⛔️ A Dialog without focusable elements is not recommended! ⛔️</p>
<p>Escape key and backdrop click still works to ensure this modal can be closed</p>
<p>⛔️ A Dialog without focusable elements is not recommended!</p>
<p>⛔️ Escape key doesn't work here</p>
<p>✅ Backdrop click still works to ensure this modal can be closed</p>
</DialogBody>
</DialogSurface>
</Dialog>
Expand All @@ -24,8 +27,9 @@ export const NoFocusableElement = () => {
<DialogSurface>
<DialogTitle closeButton={null}>Dialog Title</DialogTitle>
<DialogBody>
<p>⛔️ A Dialog without focusable elements is not recommended! ⛔️</p>
<p>⛔️ Escape key doesn't work, you're trapped! ⛔️</p>
<p>⛔️ A Dialog without focusable elements is not recommended!</p>
<p>⛔️ Escape key doesn't work</p>
<p>⛔️ you're trapped!</p>
</DialogBody>
</DialogSurface>
</Dialog>
Expand All @@ -36,7 +40,7 @@ export const NoFocusableElement = () => {
NoFocusableElement.parameters = {
docs: {
description: {
story: '',
story,
},
},
};