Skip to content

Commit

Permalink
feat(react-dialog): replace closeButton to a more generic action
Browse files Browse the repository at this point in the history
…slot (#24719)

* feat(react-dialog): replace `closeButton` to a more generic `action` slot

* chore: updates e2e tests

* chore: updates title custom action
  • Loading branch information
bsunderhus authored Sep 9, 2022
1 parent 81191d9 commit 0865ee9
Show file tree
Hide file tree
Showing 19 changed files with 185 additions and 61 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "feat(react-dialog): replace `closeButton` to a more generic `action` slot",
"packageName": "@fluentui/react-dialog",
"email": "bernardo.sunderhus@gmail.com",
"dependentChangeType": "patch"
}
11 changes: 7 additions & 4 deletions packages/react-components/react-dialog/Spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,18 @@ type DialogTitleProps = ComponentProps<DialogTitleSlots>;

### DialogTitle

The DialogTitle component will expect to have a dialog title/header and will show the close (X icon) button if specified so. Apart from styling and presenting `closeButton`, this component does not have other behavior.
The `DialogTitle` component expects to have a title/header and when `Dialog` is `non-modal` a close (X icon) button is provided through `action` slot by default.

```tsx
type DialogTitleSlots = {
/**
* By default this is a div, but can be a heading.
*/
root: Slot<'div', 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'>;
closeButton?: Slot<'button'>;
/**
* By default a Dialog with modalType='non-modal' will have a close button action
*/
action?: Slot<'div'>;
};

type DialogTitleProps = ComponentProps<DialogTitleSlots>;
Expand Down Expand Up @@ -281,7 +284,7 @@ const dialog = <Dialog type="alert">
>
<div id="fui-dialog-title-id" class="fui-dialog-title">
<span>Title</span>
<!-- closeButton -->
<!-- action -->
</div>
<div id="fui-dialog-body-id" class="fui-dialog-body">This is going to be inside the dialog</div>
<div class="fui-dialog-actions">
Expand Down Expand Up @@ -343,7 +346,7 @@ const CustomDialog = () => {
>
<div id="fui-dialog-title-id" class="fui-dialog-title">
<span>Title</span>
<!-- closeButton -->
<!-- action -->
</div>
<div id="fui-dialog-body-id" class="fui-dialog-body">This is going to be inside the dialog</div>
<div class="fui-dialog-actions">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { teamsLightTheme } from '@fluentui/react-theme';

import { Dialog, DialogActions, DialogBody, DialogSurface, DialogTitle, DialogTrigger } from '@fluentui/react-dialog';
import { Button } from '@fluentui/react-components';
import { dialogCloseButtonSelector, dialogTriggerOpenSelector } from './selectors';
import { dialogActionSelector, dialogTriggerOpenSelector } from './selectors';

const mount = (element: JSX.Element) => mountBase(<FluentProvider theme={teamsLightTheme}>{element}</FluentProvider>);

Expand Down Expand Up @@ -35,7 +35,7 @@ describe('DialogTitle', () => {
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.get(dialogCloseButtonSelector).should('not.exist');
cy.get(dialogActionSelector).should('not.exist');
});
});
describe('modalType = non-modal', () => {
Expand All @@ -62,7 +62,7 @@ describe('DialogTitle', () => {
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.get(dialogCloseButtonSelector).should('exist');
cy.get(dialogActionSelector).should('exist');
});
});
describe('modalType = alert', () => {
Expand All @@ -89,7 +89,7 @@ describe('DialogTitle', () => {
</Dialog>,
);
cy.get(dialogTriggerOpenSelector).click();
cy.get(dialogCloseButtonSelector).should('not.exist');
cy.get(dialogActionSelector).should('not.exist');
});
});
});
2 changes: 1 addition & 1 deletion packages/react-components/react-dialog/e2e/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ export const dialogSurfaceSelector = `.${dialogSurfaceClassNames.root}`;
export const dialogTriggerOpenSelector = `[aria-haspopup="dialog"]`;
export const dialogTriggerCloseSelector = `#${dialogTriggerCloseId}`;
export const dialogTitleSelector = `.${dialogTitleClassNames.root}`;
export const dialogCloseButtonSelector = `.${dialogTitleClassNames.closeButton}`;
export const dialogActionSelector = `.${dialogTitleClassNames.action}`;
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
/// <reference types="react" />

import { ARIAButtonResultProps } from '@fluentui/react-aria';
import { ARIAButtonSlotProps } from '@fluentui/react-aria';
import { ARIAButtonType } from '@fluentui/react-aria';
import type { ComponentProps } from '@fluentui/react-utilities';
import type { ComponentState } from '@fluentui/react-utilities';
Expand Down Expand Up @@ -129,7 +128,7 @@ export type DialogTitleProps = ComponentProps<DialogTitleSlots> & {};
// @public (undocumented)
export type DialogTitleSlots = {
root: Slot<'div', 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'>;
closeButton?: Slot<ARIAButtonSlotProps>;
action?: Slot<'div'>;
};

// @public
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
DIALOG_GAP,
MEDIA_QUERY_BREAKPOINT_SELECTOR,
BODY_GRID_AREA,
CLOSE_BUTTON_GRID_AREA,
TITLE_ACTION_GRID_AREA,
} from '../../contexts/constants';
import { useDialogContext_unstable } from '../../contexts/dialogContext';
import type { DialogSurfaceSlots, DialogSurfaceState } from './DialogSurface.types';
Expand Down Expand Up @@ -42,7 +42,7 @@ const useStyles = makeStyles({
gridTemplateRows: 'auto 1fr auto',
gridTemplateColumns: '1fr 1fr auto',
gridTemplateAreas: `
"${TITLE_GRID_AREA} ${TITLE_GRID_AREA} ${CLOSE_BUTTON_GRID_AREA}"
"${TITLE_GRID_AREA} ${TITLE_GRID_AREA} ${TITLE_ACTION_GRID_AREA}"
"${BODY_GRID_AREA} ${BODY_GRID_AREA} ${BODY_GRID_AREA}"
"${ACTIONS_START_GRID_AREA} ${ACTIONS_END_GRID_AREA} ${ACTIONS_END_GRID_AREA}"
`,
Expand All @@ -51,7 +51,7 @@ const useStyles = makeStyles({
maxWidth: '100vw',
gridTemplateRows: 'auto 1fr auto auto',
gridTemplateAreas: `
"${TITLE_GRID_AREA} ${TITLE_GRID_AREA} ${CLOSE_BUTTON_GRID_AREA}"
"${TITLE_GRID_AREA} ${TITLE_GRID_AREA} ${TITLE_ACTION_GRID_AREA}"
"${BODY_GRID_AREA} ${BODY_GRID_AREA} ${BODY_GRID_AREA}"
"${ACTIONS_START_GRID_AREA} ${ACTIONS_START_GRID_AREA} ${ACTIONS_START_GRID_AREA}"
"${ACTIONS_END_GRID_AREA} ${ACTIONS_END_GRID_AREA} ${ACTIONS_END_GRID_AREA}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ import type { DialogTitleProps } from './DialogTitle.types';
import type { ForwardRefComponent } from '@fluentui/react-utilities';

/**
* The `DialogTitle` component will expect to have a dialog title/header
* and will show the close (X icon) button if specified so.
* Apart from styling and presenting `closeButton`, this component does not have other behavior.
* The `DialogTitle` component expects to have a title/header
* and when `Dialog` is `non-modal` a close (X icon) button is provided through `action` slot by default.
*/
export const DialogTitle: ForwardRefComponent<DialogTitleProps> = React.forwardRef((props, ref) => {
const state = useDialogTitle_unstable(props, ref);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { ARIAButtonSlotProps } from '@fluentui/react-aria';
import type { ComponentProps, ComponentState, Slot } from '@fluentui/react-utilities';

export type DialogTitleSlots = {
/**
* By default this is a div, but can be a heading.
*/
root: Slot<'div', 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'>;
closeButton?: Slot<ARIAButtonSlotProps>;
/**
* By default a Dialog with modalType='non-modal' will have a close button action
*/
action?: Slot<'div'>;
};

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as React from 'react';
import { getSlots } from '@fluentui/react-utilities';
import type { DialogTitleState, DialogTitleSlots } from './DialogTitle.types';
import { DialogTrigger } from '../DialogTrigger';

/**
* Render the final JSX of DialogTitle
Expand All @@ -12,11 +11,7 @@ export const renderDialogTitle_unstable = (state: DialogTitleState) => {
return (
<>
<slots.root {...slotProps.root}>{slotProps.root.children}</slots.root>
{slots.closeButton && (
<DialogTrigger action="close">
<slots.closeButton {...slotProps.closeButton} />
</DialogTrigger>
)}
{slots.action && <slots.action {...slotProps.action} />}
</>
);
};
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import * as React from 'react';
import { getNativeElementProps } from '@fluentui/react-utilities';
import type { DialogTitleProps, DialogTitleState } from './DialogTitle.types';
import { useARIAButtonShorthand } from '@fluentui/react-aria';
import { useDialogContext_unstable } from '../../contexts/dialogContext';
import { Dismiss24Regular } from '@fluentui/react-icons';
import { resolveShorthand } from '@fluentui/react-utilities';
import { DialogTrigger } from '../DialogTrigger/DialogTrigger';
import { useDialogTitleInternalStyles } from './useDialogTitleStyles';

/**
* Create the state required to render DialogTitle.
Expand All @@ -15,25 +17,34 @@ import { Dismiss24Regular } from '@fluentui/react-icons';
* @param ref - reference to root HTMLElement of DialogTitle
*/
export const useDialogTitle_unstable = (props: DialogTitleProps, ref: React.Ref<HTMLElement>): DialogTitleState => {
const { as = 'div', closeButton } = props;
const { as, action } = props;
const modalType = useDialogContext_unstable(ctx => ctx.modalType);
const internalStyles = useDialogTitleInternalStyles();

return {
components: {
root: 'div',
closeButton: 'button',
action: 'div',
},
root: getNativeElementProps(as, {
root: getNativeElementProps(as ?? 'div', {
ref,
id: useDialogContext_unstable(ctx => ctx.dialogTitleID),
...props,
}),
closeButton: useARIAButtonShorthand(closeButton, {
action: resolveShorthand(action, {
required: modalType === 'non-modal',
defaultProps: {
type: 'button', // This is added because the default for type is 'submit'
'aria-label': 'close', // TODO: find a better way to add internal labels
children: <Dismiss24Regular />,
children: (
<DialogTrigger action="close">
<button
className={internalStyles.button}
// TODO: find a better way to add internal labels
aria-label="close"
>
<Dismiss24Regular />
</button>
</DialogTrigger>
),
},
}),
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { makeStyles, mergeClasses, shorthands } from '@griffel/react';
import type { DialogTitleSlots, DialogTitleState } from './DialogTitle.types';
import type { SlotClassNames } from '@fluentui/react-utilities';
import { createFocusOutlineStyle } from '@fluentui/react-tabster';
import { typographyStyles } from '@fluentui/react-theme';
import { CLOSE_BUTTON_GRID_AREA, TITLE_GRID_AREA } from '../../contexts/constants';
import { TITLE_ACTION_GRID_AREA, TITLE_GRID_AREA } from '../../contexts/constants';
import { createFocusOutlineStyle } from '@fluentui/react-tabster';

export const dialogTitleClassNames: SlotClassNames<DialogTitleSlots> = {
root: 'fui-DialogTitle',
closeButton: 'fui-DialogTitle__closeButton',
action: 'fui-DialogTitle__action',
};

/**
Expand All @@ -19,32 +19,32 @@ const useStyles = makeStyles({
...shorthands.gridArea(TITLE_GRID_AREA),
},
rootWithoutCloseButton: {
// ...shorthands.padding(DIALOG_CONTENT_PADDING, DIALOG_CONTENT_PADDING, '8px', DIALOG_CONTENT_PADDING),
...shorthands.gridArea(TITLE_GRID_AREA, TITLE_GRID_AREA, TITLE_ACTION_GRID_AREA, TITLE_ACTION_GRID_AREA),
},
rootWithCloseButton: {
// ...shorthands.padding(DIALOG_CONTENT_PADDING, '20px', '8px', DIALOG_CONTENT_PADDING),
action: {
...shorthands.gridArea(TITLE_ACTION_GRID_AREA),
},
closeButton: {
});

/**
* Styles to be applied on internal elements used by default action on non-modal Dialog
* @internal
*/
export const useDialogTitleInternalStyles = makeStyles({
button: {
position: 'relative',
lineHeight: '0',
cursor: 'pointer',
alignSelf: 'start',
...shorthands.gridArea(CLOSE_BUTTON_GRID_AREA),
},
closeButtonFocusIndicator: createFocusOutlineStyle(),
// TODO: this should be extracted to another package
resetButton: {
boxSizing: 'content-box',
backgroundColor: 'inherit',
color: 'inherit',
fontFamily: 'inherit',
fontSize: 'inherit',
lineHeight: 'normal',
lineHeight: 0,
...shorthands.overflow('visible'),
...shorthands.padding(0),
...shorthands.borderStyle('none'),
WebkitAppearance: 'button',
textAlign: 'unset',
...createFocusOutlineStyle(),
},
});

Expand All @@ -56,18 +56,11 @@ export const useDialogTitleStyles_unstable = (state: DialogTitleState): DialogTi
state.root.className = mergeClasses(
dialogTitleClassNames.root,
styles.root,
state.closeButton && styles.rootWithCloseButton,
!state.closeButton && styles.rootWithoutCloseButton,
!state.action && styles.rootWithoutCloseButton,
state.root.className,
);
if (state.closeButton) {
state.closeButton.className = mergeClasses(
dialogTitleClassNames.closeButton,
styles.resetButton,
styles.closeButton,
styles.closeButtonFocusIndicator,
state.closeButton.className,
);
if (state.action) {
state.action.className = mergeClasses(dialogTitleClassNames.action, styles.action, state.action.className);
}
return state;
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ export const SURFACE_BORDER_WIDTH = '1px';
export const ACTIONS_START_GRID_AREA = 'actions-start';
export const ACTIONS_END_GRID_AREA = 'actions-end';
export const TITLE_GRID_AREA = 'title';
export const CLOSE_BUTTON_GRID_AREA = 'close-button';
export const TITLE_ACTION_GRID_AREA = 'close-button';
export const BODY_GRID_AREA = 'body';
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const NoFocusableElement = () => {
<Button>Open non-modal dialog</Button>
</DialogTrigger>
<DialogSurface>
<DialogTitle closeButton={null}>Dialog Title</DialogTitle>
<DialogTitle action={null}>Dialog Title</DialogTitle>
<DialogBody>
<p>⛔️ A Dialog without focusable elements is not recommended!</p>
<p>⛔️ Escape key doesn't work</p>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
By default if `Dialog` has `modalType='non-modal'` a button with a close icon is provided to close the dialog as `action` slot.

This slot can be customized to add a different kind of action, that it'll be available in any kind of `Dialog`, ignoring the `modalType` property, here's an example replacing the simple close icon with a [Fluent UI Button](./?path=/docs/components-button-button--default) using the same icon.
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import * as React from 'react';
import { Dialog, DialogTrigger, DialogSurface, DialogTitle, DialogBody } from '@fluentui/react-dialog';
import { Button } from '@fluentui/react-components';
import story from './DialogTitleCustomAction.md';
import { Dismiss24Regular } from '@fluentui/react-icons';

export const CustomAction = () => {
return (
<Dialog>
<DialogTrigger>
<Button>Open dialog</Button>
</DialogTrigger>
<DialogSurface>
<DialogTitle
action={
<DialogTrigger action="close">
<Button appearance="subtle" aria-label="close" icon={<Dismiss24Regular />} />
</DialogTrigger>
}
>
Dialog title
</DialogTitle>
<DialogBody>
Lorem, ipsum dolor sit amet consectetur adipisicing elit. Aliquid, explicabo repudiandae impedit doloribus
laborum quidem maxime dolores perspiciatis non ipsam, nostrum commodi quis autem sequi, incidunt cum?
Consequuntur, repellendus nostrum?
</DialogBody>
</DialogSurface>
</Dialog>
);
};

CustomAction.parameters = {
docs: {
description: {
story,
},
},
};
Loading

0 comments on commit 0865ee9

Please sign in to comment.