Skip to content
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

fix(Modal): address QA updates #1957

Merged
merged 1 commit into from
May 21, 2024
Merged

fix(Modal): address QA updates #1957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
86 changes: 52 additions & 34 deletions src/components/Modal/Modal-v2.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,13 @@
* The inverted background of the modal to provide contrast with the actual modal.
*/
.modal__overlay {
background-color: var(--eds-theme-color-background-utility-overlay-low-emphasis);
opacity: 0.5;
&.modal__overlay--emphasis-high {
background-color: rgb(from var(--eds-theme-color-background-utility-overlay-high-emphasis) r g b / 0.8);
}

&.modal__overlay--emphasis-low {
background-color: rgb(from var(--eds-theme-color-background-utility-overlay-low-emphasis) r g b / 0.5);
}
}

/**
Expand Down Expand Up @@ -81,8 +86,6 @@
*/
.modal__content {
position: relative;
height: 43.125rem;
max-height: 90vh;
overflow: hidden;

/**
Expand All @@ -95,31 +98,67 @@
display: flex;
flex-direction: column;

width: 22.5rem;

background-color: var(--eds-theme-color-background-utility-container);
}

/**
* The medium modal size used for the md modal.
* Also used for the lg modal size for when the screen size is smaller than 75rem.
* The large modal size used for the lg/default modal.
*/
.modal__content--md {
.modal__content--lg {
@media all and (min-width: $eds-bp-xs) {
width: 100%;
height: 100%;
}

@media all and (min-width: $eds-bp-sm) {
max-height: 40rem;
max-width: 50rem;
}

@media all and (min-width: $eds-bp-md) {
width: 42rem;
max-height: 40rem;
max-width: 50rem;
}

@media all and (min-width: $eds-bp-lg) {
max-height: 40rem;
max-width: 60rem;
}

@media all and (min-width: $eds-bp-xl) {
max-height: 40rem;
max-width: 60rem;
--modal-horizontal-padding: 4rem;
}
}

/**
* The large modal size used for the lg/default modal.
* The small modal size used for the modal.
*/
.modal__content--lg {
.modal__content--sm {
@media all and (min-width: $eds-bp-xs) {
max-height: 30rem;
max-width: 35rem;
}

@media all and (min-width: $eds-bp-sm) {
max-height: 30rem;
max-width: 25rem;
}

@media all and (min-width: $eds-bp-md) {
max-height: 30rem;
max-width: 25rem;
}

@media all and (min-width: $eds-bp-xl) {
width: 64rem;
max-height: 30rem;
max-width: 35rem;
--modal-horizontal-padding: 4rem;
}
}


/**
* Allows scrolling of the modal content except for sticky footer.
* This functionality is our intended scroll behavior but consuming teams can
Expand All @@ -133,30 +172,9 @@
* The modal close button.
*/
.modal__close-button {
border: 0;
background-color: transparent;

position: absolute;
top: 0;
right: 0;

width: 3rem;
height: 3rem;

cursor: pointer;

z-index: 1;

color: var(--eds-theme-color-icon-utility-default-secondary);
}

/**
* The modal close icon that resides in the close button.
*/
.modal__close-icon {
position: absolute;
top: 0.5rem;
right: 0.5rem;
}

/*------------------------------------*\
Expand Down
85 changes: 42 additions & 43 deletions src/components/Modal/Modal-v2.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { Heading, Text } from '../../';
import { chromaticViewports, storybookViewports } from '../../util/viewports';
import { ButtonV2 as Button } from '../Button';
import { ButtonGroupV2 as ButtonGroup } from '../ButtonGroup';
import { TooltipV2 as Tooltip } from '../Tooltip';

export default {
title: 'Components/V2/Modal',
Expand Down Expand Up @@ -42,6 +41,14 @@ export default {
'Toggles scrollable variant of the modal. If modal is scrollable, footer is not, and vice versa.',
type: 'boolean',
},
size: {
description: 'Max size of the modal, which responds to the viewport',
control: {
type: 'select',
},
options: ['sm', 'lg'],
defaultValue: 'lg',
},
},
decorators: [(Story) => <div className="p-8">{Story()}</div>],
} as Meta<typeof Modal>;
Expand Down Expand Up @@ -87,12 +94,9 @@ export const Default: StoryObj<Args> = {
</Modal.Body>
<Modal.Footer>
<ButtonGroup>
{/* This has to be manually tested since Tooltip tests are flaky in Chromatic */}
<Tooltip text="Tooltip should spawn on top of modal">
<Button onClick={() => {}} rank="primary">
Primary Action
</Button>
</Tooltip>
<Button onClick={() => {}} rank="primary">
Primary Action
</Button>
<Button onClick={() => {}} rank="secondary">
Secondary
</Button>
Expand All @@ -102,6 +106,16 @@ export const Default: StoryObj<Args> = {
),
};

/**
* Modals can also have a more emphasized backdrop overlay
*/
export const HighEmphasis: StoryObj<Args> = {
args: {
overlayEmphasis: 'high',
},
render: Default.render,
};

/**
* Modals can contain long, scrollable text. This is not recommended, however.
*/
Expand Down Expand Up @@ -187,12 +201,9 @@ export const WithLongTextScrollable: StoryObj<InteractiveArgs> = {
</Modal.Body>
<Modal.Footer>
<ButtonGroup>
{/* This has to be manually tested since Tooltip tests are flaky in Chromatic */}
<Tooltip text="Tooltip should spawn on top of modal">
<Button onClick={() => {}} rank="primary">
Primary Action
</Button>
</Tooltip>
<Button onClick={() => {}} rank="primary">
Primary Action
</Button>
<Button onClick={() => {}} rank="secondary">
Secondary
</Button>
Expand Down Expand Up @@ -235,12 +246,9 @@ export const ContentDefault: Story = {
</Modal.Body>
<Modal.Footer>
<ButtonGroup>
{/* This has to be manually tested since Tooltip tests are flaky in Chromatic */}
<Tooltip text="Tooltip should spawn on top of modal">
<Button onClick={() => {}} rank="primary">
Primary Action
</Button>
</Tooltip>
<Button onClick={() => {}} rank="primary">
Primary Action
</Button>
<Button onClick={() => {}} rank="secondary">
Secondary
</Button>
Expand All @@ -259,13 +267,13 @@ export const ContentDefault: Story = {

/**
* `Modal` provides `size`, which allows control over the natural width of the modal. This does not affect the contents
* of the modal.
* of the modal except to wrap text.
*/
export const Medium: Story = {
export const Large: Story = {
...ContentDefault,
args: {
...ContentDefault.args,
size: 'md',
size: 'lg',
},
};

Expand All @@ -288,7 +296,7 @@ export const LayoutVertical: Story = {
...ContentDefault,
args: {
...ContentDefault.args,
size: 'md',
size: 'lg',
children: (
<>
<Modal.Header>
Expand All @@ -300,12 +308,9 @@ export const LayoutVertical: Story = {
</Modal.Body>
<Modal.Footer>
<ButtonGroup buttonLayout="vertical">
{/* This has to be manually tested since Tooltip tests are flaky in Chromatic */}
<Tooltip text="Tooltip should spawn on top of modal">
<Button isFullWidth onClick={() => {}} rank="primary">
Primary Action
</Button>
</Tooltip>
<Button isFullWidth onClick={() => {}} rank="primary">
Primary Action
</Button>
<Button isFullWidth onClick={() => {}} rank="secondary">
Secondary
</Button>
Expand Down Expand Up @@ -334,7 +339,7 @@ export const LayoutVerticalWithTertiary: Story = {
...ContentDefault,
args: {
...ContentDefault.args,
size: 'md',
size: 'lg',
children: (
<>
<Modal.Header>
Expand All @@ -346,12 +351,9 @@ export const LayoutVerticalWithTertiary: Story = {
</Modal.Body>
<Modal.Footer>
<ButtonGroup buttonLayout="vertical">
{/* This has to be manually tested since Tooltip tests are flaky in Chromatic */}
<Tooltip text="Tooltip should spawn on top of modal">
<Button isFullWidth onClick={() => {}} rank="primary">
Primary Action
</Button>
</Tooltip>
<Button isFullWidth onClick={() => {}} rank="primary">
Primary Action
</Button>
<Button isFullWidth onClick={() => {}} rank="tertiary">
Tertiary
</Button>
Expand Down Expand Up @@ -379,7 +381,7 @@ export const WithCriticalButton: Story = {
...ContentDefault,
args: {
...ContentDefault.args,
size: 'md',
size: 'lg',
children: (
<>
<Modal.Header>
Expand All @@ -391,12 +393,9 @@ export const WithCriticalButton: Story = {
</Modal.Body>
<Modal.Footer>
<ButtonGroup>
{/* This has to be manually tested since Tooltip tests are flaky in Chromatic */}
<Tooltip text="Tooltip should spawn on top of modal">
<Button onClick={() => {}} rank="primary" variant="critical">
Critical Action
</Button>
</Tooltip>
<Button onClick={() => {}} rank="primary" variant="critical">
Critical Action
</Button>
</ButtonGroup>
</Modal.Footer>
</>
Expand Down
2 changes: 1 addition & 1 deletion src/components/Modal/Modal-v2.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ describe('Modal', () => {
});
await user.click(openModalButton);
const closeButton = await screen.findByRole('button', {
name: 'close modal',
name: 'close',
});
await user.click(closeButton);
await waitFor(() => {
Expand Down
Loading
Loading