Skip to content

feat(react-dialog): supports 1st rule of ARIA #24525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bsunderhus merged 2 commits into from
Sep 13, 2022
Merged

feat(react-dialog): supports 1st rule of ARIA #24525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a902c99
feat(react-dialog): 1st rule of ARIA for Dialog
bsunderhus Sep 7, 2022
e698797
chore: delete useDialogProps
bsunderhus Sep 8, 2022
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
7 changes: 7 additions & 0 deletions change/@fluentui-react-dialog-dc310d89-6297-481f-a1fa-2a25450bc15a.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "feat(react-dialog): 1st rule of ARIA for Dialog",

@layershifter layershifter Sep 13, 2022

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.

Suggested change
"comment": "feat(react-dialog): 1st rule of ARIA for Dialog",
"comment": "feat(react-dialog): use native dialog element",

@bsunderhus bsunderhus Sep 13, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll be addressing any Safari related issues on a separate PR, since some cases need investigation @layershifter, but in the worst case scenario of not getting native <dialog>to work out of of the box then I'll have to default to divand add dialog as an option

"packageName": "@fluentui/react-dialog",
"email": "bernardo.sunderhus@gmail.com",
"dependentChangeType": "patch"
}
50 changes: 33 additions & 17 deletions packages/react-components/react-dialog/Spec.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,19 +83,7 @@ Sample usages will be give in the following section of this document [Sample cod
The root level component serves as an interface for interaction with all possible behaviors exposed. It provides context down the hierarchy to `children` compound components to allow functionality. This component expects to receive as children either a `DialogSurface` or a `DialogTrigger` and a `DialogSurface` (or some component that will eventually render one of those compound components) in this specific order

```tsx
type DialogSlots = {
/**
* Dimmed background of dialog.
* The default backdrop is rendered as a `<div>` with styling.
* This slot expects a `<div>` element which will replace the default backdrop.
* The backdrop should have `aria-hidden="true"`.
*/
backdrop?: Slot<'div'>;
/**
* The root element of the Dialog right after Portal.
*/
root: Slot<'div'>;
};
type DialogSlots = {};

type DialogProps = ComponentProps<DialogSlots> & {
/**
Expand Down Expand Up @@ -167,14 +155,21 @@ export type DialogTriggerProps = {
The `DialogSurface` component represents the visual part of a `Dialog` as a whole, it contains everything that should be visible.

```tsx
type DialogTitleSlots = {
type DialogSurfaceSlots = {
/**
* By default this is a div.
* Dimmed background of dialog.
* The default backdrop is rendered as a `<div>` with styling.
* This slot expects a `<div>` element which will replace the default backdrop.
* The backdrop should have `aria-hidden="true"`.
*
* By default if `DialogSurface` is `<dialog>` element the backdrop is ignored,
* since native `<dialog>` element supports [::backdrop](https://developer.mozilla.org/en-US/docs/Web/CSS/::backdrop)
*/
root: Slot<'div', 'main'>;
backdrop?: Slot<'div'>;
root: NonNullable<Slot<'dialog', 'div'>>;
};

type DialogTitleProps = ComponentProps<DialogTitleSlots>;
type DialogTitleProps = ComponentProps<DialogSurfaceSlots>;
```

### DialogTitle
Expand Down Expand Up @@ -405,6 +400,27 @@ function AsyncConfirmDialog() {
}
```

### Opting out of native `<dialog>`

```tsx
const dialog = <Dialog>
<DialogTrigger>
<Button>Open Dialog</Button>
<DialogTrigger>
<DialogSurface as="div">
This is as basic as it gets.
</DialogSurface>
</Dialog>
```

```html
<!-- expected DOM output -->
<button aria-haspopup="true" class="fui-button">Open Dialog</button>
<!-- ... portal ... -->
<div aria-hidden="true" class="fui-dialog-backdrop"></div>
<div aria-modal="true" role="dialog" class="fui-dialog-content">This is as basic as it gets</div>
```

## Migration

_TBA: Link to migration guide doc_
Expand Down
30 changes: 15 additions & 15 deletions packages/react-components/react-dialog/e2e/Dialog.e2e.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe('Dialog', () => {
</DialogSurface>
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.get(dialogTriggerOpenSelector).realClick();
cy.get(dialogSurfaceSelector).should('exist');
});
it('should focus on first focusabled element when opened', () => {
Expand All @@ -90,7 +90,7 @@ describe('Dialog', () => {
</DialogSurface>
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.get(dialogTriggerOpenSelector).realClick();
cy.get(dialogTriggerCloseSelector).should('be.focused');
});
it('should focus on body if no focusabled element in dialog', () => {
Expand All @@ -109,7 +109,7 @@ describe('Dialog', () => {
</DialogSurface>
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.get(dialogTriggerOpenSelector).realClick();
cy.focused().should('not.exist');
});
it('should focus back on trigger when dialog closed', () => {
Expand All @@ -136,8 +136,8 @@ describe('Dialog', () => {
</DialogSurface>
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.get(dialogTriggerCloseSelector).click();
cy.get(dialogTriggerOpenSelector).realClick();
cy.get(dialogTriggerCloseSelector).realClick();
cy.get(dialogTriggerOpenSelector).should('be.focused');
});
it('should allow change of focus on open', () => {
Expand Down Expand Up @@ -173,7 +173,7 @@ describe('Dialog', () => {
);
};
mount(<CustomFocusedElementOnOpen />);
cy.get(dialogTriggerOpenSelector).click();
cy.get(dialogTriggerOpenSelector).realClick();
cy.get(dialogTriggerCloseSelector).should('be.focused');
});
describe('modalType = modal', () => {
Expand All @@ -199,8 +199,8 @@ describe('Dialog', () => {
</DialogSurface>
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.focused().type('{esc}');
cy.get(dialogTriggerOpenSelector).realClick();
cy.focused().realType('{esc}');
cy.get(dialogSurfaceSelector).should('not.exist');
});
it('should lock body scroll when dialog open', () => {
Expand All @@ -225,7 +225,7 @@ describe('Dialog', () => {
</DialogSurface>
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.get(dialogTriggerOpenSelector).realClick();
cy.get('body').should('have.css', 'overflow', 'hidden');
});
});
Expand All @@ -252,8 +252,8 @@ describe('Dialog', () => {
</DialogSurface>
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.focused().type('{esc}');
cy.get(dialogTriggerOpenSelector).realClick();
cy.focused().realType('{esc}');
cy.get(dialogSurfaceSelector).should('not.exist');
});
it('should not lock body scroll when dialog open', () => {
Expand All @@ -278,7 +278,7 @@ describe('Dialog', () => {
</DialogSurface>
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.get(dialogTriggerOpenSelector).realClick();
cy.get('body').should('not.have.css', 'overflow', 'hidden');
});
});
Expand All @@ -305,8 +305,8 @@ describe('Dialog', () => {
</DialogSurface>
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.focused().type('{esc}');
cy.get(dialogTriggerOpenSelector).realClick();
cy.focused().realType('{esc}');
cy.get(dialogSurfaceSelector).should('exist');
});
it('should lock body scroll when dialog open', () => {
Expand All @@ -331,7 +331,7 @@ describe('Dialog', () => {
</DialogSurface>
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.get(dialogTriggerOpenSelector).realClick();
cy.get('body').should('have.css', 'overflow', 'hidden');
});
});
Expand Down
6 changes: 3 additions & 3 deletions packages/react-components/react-dialog/e2e/DialogTitle.e2e.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('DialogTitle', () => {
</DialogSurface>
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.get(dialogTriggerOpenSelector).realClick();
cy.get(dialogActionSelector).should('not.exist');
});
});
Expand All @@ -61,7 +61,7 @@ describe('DialogTitle', () => {
</DialogSurface>
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.get(dialogTriggerOpenSelector).realClick();
cy.get(dialogActionSelector).should('exist');
});
});
Expand All @@ -88,7 +88,7 @@ describe('DialogTitle', () => {
</DialogSurface>
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.get(dialogTriggerOpenSelector).realClick();
cy.get(dialogActionSelector).should('not.exist');
});
});
Expand Down
6 changes: 3 additions & 3 deletions packages/react-components/react-dialog/e2e/DialogTrigger.e2e.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('DialogTrigger', () => {
</DialogSurface>
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.get(dialogTriggerOpenSelector).realClick();
cy.get(dialogSurfaceSelector).should('not.exist');
});
it(`should open dialog when 'aria-disabled' is false`, () => {
Expand All @@ -58,7 +58,7 @@ describe('DialogTrigger', () => {
</DialogSurface>
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.get(dialogTriggerOpenSelector).realClick();
cy.get(dialogSurfaceSelector).should('exist');
});
it('should work with any element besides <button>', () => {
Expand All @@ -83,7 +83,7 @@ describe('DialogTrigger', () => {
</DialogSurface>
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.get(dialogTriggerOpenSelector).realClick();
cy.get(dialogSurfaceSelector).should('exist');
});
});
21 changes: 14 additions & 7 deletions packages/react-components/react-dialog/etc/react-dialog.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,28 +64,35 @@ export type DialogBodyState = ComponentState<DialogBodySlots>;

// @public (undocumented)
export type DialogOpenChangeData = {
type: 'dialogCancel';
open: boolean;
event: React_2.SyntheticEvent<DialogSurfaceElement>;
} | {
type: 'escapeKeyDown';
open: boolean;
event: React_2.KeyboardEvent;
event: React_2.KeyboardEvent<DialogSurfaceElement>;
} | {
type: 'backdropClick';
open: boolean;
event: React_2.MouseEvent;
event: React_2.MouseEvent<DialogSurfaceElement>;
} | {
type: 'triggerClick';
open: boolean;
event: React_2.MouseEvent;
event: React_2.MouseEvent<DialogSurfaceElement>;
};

// @public (undocumented)
export type DialogOpenChangeEvent = React_2.KeyboardEvent | React_2.MouseEvent;
export type DialogOpenChangeEvent = DialogOpenChangeData['event'];

// @public
export type DialogOpenChangeEventHandler = (event: DialogOpenChangeEvent, data: DialogOpenChangeData) => void;

// @public (undocumented)
export type DialogProps = ComponentProps<Partial<DialogSlots>> & {
modalType?: DialogModalType;
open?: boolean;
defaultOpen?: boolean;
onOpenChange?: (event: DialogOpenChangeEvent, data: DialogOpenChangeData) => void;
onOpenChange?: DialogOpenChangeEventHandler;
children: [JSX.Element, JSX.Element] | JSX.Element;
};

Expand All @@ -105,12 +112,12 @@ export const DialogSurface: ForwardRefComponent<DialogSurfaceProps>;
export const dialogSurfaceClassNames: SlotClassNames<DialogSurfaceSlots>;

// @public
export type DialogSurfaceProps = ComponentProps<DialogSurfaceSlots>;
export type DialogSurfaceProps = Omit<ComponentProps<DialogSurfaceSlots>, 'open' | 'onCancel' | 'onClose'>;

// @public (undocumented)
export type DialogSurfaceSlots = {
backdrop?: Slot<'div'>;
root: NonNullable<Slot<'div'>>;
root: NonNullable<Slot<'dialog', 'div'>>;
};

// @public
Expand Down
45 changes: 34 additions & 11 deletions packages/react-components/react-dialog/src/components/Dialog/Dialog.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,47 @@
import type * as React from 'react';
import type { ComponentProps, ComponentState } from '@fluentui/react-utilities';
import type { DialogContextValue, DialogSurfaceContextValue } from '../../contexts';
import type { DialogSurfaceElement } from '../DialogSurface/DialogSurface.types';

export type DialogSlots = {};

export type DialogOpenChangeEvent = React.KeyboardEvent | React.MouseEvent;
export type DialogOpenChangeEvent = DialogOpenChangeData['event'];

export type DialogOpenChangeData =
| { type: 'escapeKeyDown'; open: boolean; event: React.KeyboardEvent }
| { type: 'backdropClick'; open: boolean; event: React.MouseEvent }
| { type: 'triggerClick'; open: boolean; event: React.MouseEvent };
| {
/**
* triggered when Escape key is pressed in a native `dialog`
*/
type: 'dialogCancel';
open: boolean;
event: React.SyntheticEvent<DialogSurfaceElement>;
}
Comment thread
bsunderhus marked this conversation as resolved.
| {
type: 'escapeKeyDown';
open: boolean;
event: React.KeyboardEvent<DialogSurfaceElement>;
}
| {
type: 'backdropClick';
open: boolean;
event: React.MouseEvent<DialogSurfaceElement>;
}
| {
type: 'triggerClick';
open: boolean;
event: React.MouseEvent<DialogSurfaceElement>;
};

export type DialogModalType = 'modal' | 'non-modal' | 'alert';

/**
* Callback fired when the component changes value from open state.
*
* @param event - a React's Synthetic event or a KeyboardEvent in case of `documentEscapeKeyDown`
* @param data - A data object with relevant information, such as open value and type
*/
export type DialogOpenChangeEventHandler = (event: DialogOpenChangeEvent, data: DialogOpenChangeData) => void;

export type DialogContextValues = {
dialog: DialogContextValue;
/**
Expand Down Expand Up @@ -52,13 +81,7 @@ export type DialogProps = ComponentProps<Partial<DialogSlots>> & {
* @default false
*/
defaultOpen?: boolean;
/**
* Callback fired when the component changes value from open state.
*
* @param event - a React's Synthetic event or a KeyboardEvent in case of `documentEscapeKeyDown`
* @param data - A data object with relevant information, such as open value and type
*/
onOpenChange?: (event: DialogOpenChangeEvent, data: DialogOpenChangeData) => void;
onOpenChange?: DialogOpenChangeEventHandler;
/**
* Can contain two children including {@link DialogTrigger} and {@link DialogSurface}.
* Alternatively can only contain {@link DialogSurface} if using trigger outside dialog, or controlling state.
Expand Down
Loading