Skip to content

Commit

Permalink
fix(Modal): address QA updates (#1957)
Browse files Browse the repository at this point in the history 
- add low and high emphasis
- add detail to story descriptions
- add in button component instead of clickable icon
- remove tooltips from stories
  • Loading branch information
@booc0mtaco
booc0mtaco authored May 21, 2024
1 parent dbd291e commit 0d5a414
Show file tree
Hide file tree
Showing 5 changed files with 492 additions and 332 deletions.
86 changes: 52 additions & 34 deletions src/components/Modal/Modal-v2.module.css
View file
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
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
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

0 comments on commit 0d5a414

Please sign in to comment.