Skip to content

Commit

Permalink
cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@sirineJ
sirineJ committed Dec 13, 2024
1 parent e8101fe commit e829c06
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 45 deletions.
27 changes: 17 additions & 10 deletions packages/circuit-ui/components/Modal/Modal.mdx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ The dialog component displays self-contained tasks in a modal view, requiring th

## When to use it

Generally, use the dialog component sparingly. Consider displaying more complex tasks and large amounts of information on a separate page instead.
Generally, use the modal dialog component sparingly. Consider displaying more complex tasks and large amounts of information on a separate page instead.

A modal dialog is a disruptive pattern that interrupts the user's current task. It should only be used when the task requires immediate attention or when the user needs to make a decision before continuing:

Expand All @@ -30,18 +30,25 @@ Place your dialog content directly in the `Modal` component:

```tsx
import { Modal, Body, Button, Heading } from '@sumup-oss/circuit-ui';
import {useState} from 'react';

const [open, setOpen] = useState(false);

return (
<Modal open={true} closeButtonLabel="Close" aria-labelledby="dialog-title">
{() => (
<>
<Heading as="h2" id="dialog-title">Modal title</Heading>
<Body>Modal content</Body>
<Button>Close dialog</Button>
<>
<Button onClick={() => setOpen(true)}>Open dialog</Button>
<Modal open={open} closeButtonLabel="Close" aria-labelledby="dialog-title" onClose={() => setOpen(false)}>
{() => (
<>
<Heading as="h2" id="dialog-title">Modal title</Heading>
<Body>Modal content</Body>
<Button>Close dialog</Button>
</>
)}
</Modal>
</>
)}
</Modal>
);

```
### with the `useModal` hook
First, wrap your application in the `ModalProvider`:
Expand Down Expand Up @@ -99,6 +106,6 @@ If your dialog content has a title, make sure to provide an `aria-labelledby` at

If your dialog displays a complex flow with multiple screens (for example: a complex form with multiple steps), make sure to programmatically set focus to the title upon landing on every step to convey to the user their evolution within your flow.

If your content contains interactive elements, the component will focus the first interactive element when the dialog opens by default. However, if you wish to focus a different element, you can provide the `initialFocusRef` prop with the ref of the element you want to focus. It is generally recommended to focus the least destructive element, such as a close button or a cancel button. If the element you want to focus is not interactive, don't forget to give it a tabindex with a negative value to enable its focus.
If your content contains interactive elements, the component will focus the first interactive element when the dialog opens by default. However, if you wish to focus a different element, you can provide the `initialFocusRef` prop with the ref of the element you want to focus. It is generally recommended to focus the least destructive element, such as a close or a cancel button. If the element you want to focus is not interactive, don't forget to give it a tabindex with a negative value to enable its focus.


13 changes: 0 additions & 13 deletions packages/circuit-ui/components/Modal/Modal.module.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,6 @@
}

@media (min-width: 480px) {
/* .dialog .content {
width: min-content;
min-width: 480px;
max-width: 90vw;
max-height: 90vh;
} */

.dialog::after {
height: var(--cui-spacings-giga);
border-bottom-right-radius: var(--cui-border-radius-mega);
Expand Down Expand Up @@ -110,12 +103,6 @@
}
}

/* Content */

/* .content {
overflow-y: auto;
} */

@media (max-width: 479px) {
.dialog {
padding: var(--cui-spacings-mega);
Expand Down
12 changes: 6 additions & 6 deletions packages/circuit-ui/components/Modal/Modal.spec.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import {
act,
} from '../../util/test-utils.js';

import { animationDuration, Modal } from './Modal.js';
import { ANIMATION_DURATION, Modal } from './Modal.js';
import { hasNativeDialogSupport } from './ModalService.js';

vi.mock('./ModalService.js', async (importOriginal) => {
Expand Down Expand Up @@ -104,7 +104,7 @@ describe('Modal', () => {
vi.spyOn(dialog, 'close');
rerender(<Modal {...props} />);
act(() => {
vi.advanceTimersByTime(animationDuration);
vi.advanceTimersByTime(ANIMATION_DURATION);
});
expect(dialog.close).toHaveBeenCalledOnce();
});
Expand Down Expand Up @@ -147,7 +147,7 @@ describe('Modal', () => {
const dialog = container.querySelector('dialog') as HTMLDialogElement;
await userEvent.click(dialog);
act(() => {
vi.advanceTimersByTime(animationDuration);
vi.advanceTimersByTime(ANIMATION_DURATION);
});
expect(props.onClose).not.toHaveBeenCalled();
});
Expand All @@ -165,7 +165,7 @@ describe('Modal', () => {
render(<Modal {...props} open />);
await userEvent.click(screen.getByRole('dialog', { hidden: true }));
act(() => {
vi.advanceTimersByTime(animationDuration);
vi.advanceTimersByTime(ANIMATION_DURATION);
});
expect(props.onClose).toHaveBeenCalledOnce();
});
Expand All @@ -174,7 +174,7 @@ describe('Modal', () => {
render(<Modal {...props} open />);
await userEvent.click(screen.getByRole('button', { name: 'Close' }));
act(() => {
vi.advanceTimersByTime(animationDuration);
vi.advanceTimersByTime(ANIMATION_DURATION);
});
expect(props.onClose).toHaveBeenCalledOnce();
});
Expand All @@ -188,7 +188,7 @@ describe('Modal', () => {
await userEvent.click(screen.getByRole('button', { name: 'Close' }));
expect(backdrop.classList.toString()).not.toContain('backdrop-visible');
act(() => {
vi.advanceTimersByTime(animationDuration);
vi.advanceTimersByTime(ANIMATION_DURATION);
});

expect(props.onClose).toHaveBeenCalledOnce();
Expand Down
12 changes: 10 additions & 2 deletions packages/circuit-ui/components/Modal/Modal.stories.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@
* limitations under the License.
*/

import { Fragment, type ReactNode, useState } from 'react';
import { Fragment, useState } from 'react';
import { screen, userEvent, within } from '@storybook/test';
import type { Decorator } from '@storybook/react';

import { modes } from '../../../../.storybook/modes.js';
import { Headline } from '../Headline/index.js';
import { Body } from '../Body/index.js';
import { Button } from '../Button/index.js';
Expand All @@ -31,6 +32,12 @@ export default {
tags: ['status:stable'],
parameters: {
layout: 'padded',
chromatic: {
modes: {
mobile: modes.smallMobile,
desktop: modes.desktop,
},
},
},
decorators: [
(Story) => (
Expand All @@ -41,7 +48,7 @@ export default {
] as Decorator[],
};

const defaultModalChildren = (): ReactNode => (
const defaultModalChildren = () => (
<Fragment>
<Headline id="title" as="h2" size="s" style={{ marginBottom: '1rem' }}>
Hello World!
Expand Down Expand Up @@ -103,6 +110,7 @@ export const WithUseModal = (modal: ModalProps) => {
</Button>
);
};

return (
<ModalProvider>
<ComponentWithModal />
Expand Down
14 changes: 8 additions & 6 deletions packages/circuit-ui/components/Modal/Modal.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export interface ModalProps
[key: DataAttribute]: string | undefined;
}

export const animationDuration = 300;
export const ANIMATION_DURATION = 300;

export const Modal = forwardRef<HTMLDialogElement, ModalProps>((props, ref) => {
const {
Expand Down Expand Up @@ -117,6 +117,7 @@ export const Modal = forwardRef<HTMLDialogElement, ModalProps>((props, ref) => {

const hasNativeDialog = hasNativeDialogSupport();

// set initial focus on the modal dialog content
useEffect(() => {
const dialogElement = dialogRef.current;
let timeoutId: NodeJS.Timeout;
Expand All @@ -127,7 +128,7 @@ export const Modal = forwardRef<HTMLDialogElement, ModalProps>((props, ref) => {
} else {
getFirstFocusableElement(dialogElement).focus();
}
}, animationDuration);
}, ANIMATION_DURATION);
}
return () => {
clearTimeout(timeoutId);
Expand Down Expand Up @@ -157,7 +158,7 @@ export const Modal = forwardRef<HTMLDialogElement, ModalProps>((props, ref) => {
if (lastFocusedElementRef.current) {
setTimeout(
() => lastFocusedElementRef.current?.focus(),
animationDuration,
ANIMATION_DURATION,
);
}
// restore scroll to page
Expand All @@ -167,21 +168,22 @@ export const Modal = forwardRef<HTMLDialogElement, ModalProps>((props, ref) => {
body.style.top = '';
window.scrollTo(0, Number.parseInt(scrollY || '0', 10) * -1);

// trigger close animation
// trigger closing animation
dialogElement.classList.remove(classes.show);
if (!hasNativeDialog) {
(dialogElement.nextSibling as HTMLDivElement).classList.remove(
classes['backdrop-visible'],
);
}
// trigger dialog close after animation
// trigger closing of the dialog after animation
setTimeout(() => {
if (dialogElement.open) {
dialogElement.close();
}
}, animationDuration);
}, ANIMATION_DURATION);
}, []);

// intercept and prevent polyfill modal closing if preventClose is true
function onPolyfillDialogKeydown(event: KeyboardEvent) {
if (isEscape(event)) {
event.preventDefault();
Expand Down
4 changes: 2 additions & 2 deletions packages/circuit-ui/components/Modal/ModalContext.spec.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
} from '../../util/test-utils.js';

import { ModalProvider, ModalContext } from './ModalContext.js';
import { animationDuration, type ModalProps } from './Modal.js';
import { ANIMATION_DURATION, type ModalProps } from './Modal.js';

const Modal = (props: ModalProps) => <Modal {...props} />;

Expand Down Expand Up @@ -129,7 +129,7 @@ describe('ModalDialogContext', () => {
unmount();
act(() => {
vi.runAllTimers();
vi.advanceTimersByTime(animationDuration);
vi.advanceTimersByTime(ANIMATION_DURATION);
});

expect(screen.queryByRole('dialog')).toBeNull();
Expand Down
12 changes: 6 additions & 6 deletions packages/circuit-ui/components/NotificationModal/NotificationModal.spec.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { Plus } from '@sumup-oss/icons';

import { axe, render, userEvent, screen, act } from '../../util/test-utils.js';
import { animationDuration } from '../Modal/Modal.js';
import { ANIMATION_DURATION } from '../Modal/Modal.js';

import {
NotificationModal,
Expand Down Expand Up @@ -68,7 +68,7 @@ describe('NotificationModal', () => {
};
renderNotificationModal(props);
act(() => {
vi.advanceTimersByTime(animationDuration);
vi.advanceTimersByTime(ANIMATION_DURATION);
});

const svg = screen.getByRole('img');
Expand All @@ -88,7 +88,7 @@ describe('NotificationModal', () => {
renderNotificationModal(baseNotificationModal);

act(() => {
vi.advanceTimersByTime(animationDuration);
vi.advanceTimersByTime(ANIMATION_DURATION);
});

const modalEl = await screen.findByRole('dialog');
Expand All @@ -100,7 +100,7 @@ describe('NotificationModal', () => {
it('should close the modal when clicking the close button', async () => {
renderNotificationModal(baseNotificationModal);
act(() => {
vi.advanceTimersByTime(animationDuration);
vi.advanceTimersByTime(ANIMATION_DURATION);
});

const closeButton = await screen.findByRole('button', {
Expand All @@ -109,7 +109,7 @@ describe('NotificationModal', () => {

await userEvent.click(closeButton);
act(() => {
vi.advanceTimersByTime(animationDuration);
vi.advanceTimersByTime(ANIMATION_DURATION);
});

expect(baseNotificationModal.onClose).toHaveBeenCalled();
Expand All @@ -118,7 +118,7 @@ describe('NotificationModal', () => {
it('should perform an action and close the modal when clicking an action button', async () => {
renderNotificationModal(baseNotificationModal);
act(() => {
vi.advanceTimersByTime(animationDuration);
vi.advanceTimersByTime(ANIMATION_DURATION);
});
const actionButton = await screen.findByRole('button', {
name: baseNotificationModal.actions.primary.children,
Expand Down

0 comments on commit e829c06

Please sign in to comment.