From cad59457768c980a5e8d77e3161cfb9bbb50ed74 Mon Sep 17 00:00:00 2001 From: Andrew Holloway Date: Tue, 21 May 2024 13:25:11 -0500 Subject: [PATCH] fix(Modal): address QA updates - add low and high emphasis - add detail to story descriptions - add in button component instead of clickable icon - remove tooltips from stories --- src/components/Modal/Modal-v2.module.css | 86 ++- src/components/Modal/Modal-v2.stories.tsx | 85 ++- src/components/Modal/Modal-v2.test.tsx | 2 +- src/components/Modal/Modal-v2.tsx | 51 +- .../__snapshots__/Modal-v2.test.tsx.snap | 600 +++++++++++------- 5 files changed, 492 insertions(+), 332 deletions(-) diff --git a/src/components/Modal/Modal-v2.module.css b/src/components/Modal/Modal-v2.module.css index fa8a40ac6..1700a56de 100755 --- a/src/components/Modal/Modal-v2.module.css +++ b/src/components/Modal/Modal-v2.module.css @@ -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); + } } /** @@ -81,8 +86,6 @@ */ .modal__content { position: relative; - height: 43.125rem; - max-height: 90vh; overflow: hidden; /** @@ -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 @@ -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; } /*------------------------------------*\ diff --git a/src/components/Modal/Modal-v2.stories.tsx b/src/components/Modal/Modal-v2.stories.tsx index 6d15715d3..03b57ace9 100644 --- a/src/components/Modal/Modal-v2.stories.tsx +++ b/src/components/Modal/Modal-v2.stories.tsx @@ -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', @@ -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) =>
{Story()}
], } as Meta; @@ -87,12 +94,9 @@ export const Default: StoryObj = { - {/* This has to be manually tested since Tooltip tests are flaky in Chromatic */} - - - + @@ -102,6 +106,16 @@ export const Default: StoryObj = { ), }; +/** + * Modals can also have a more emphasized backdrop overlay + */ +export const HighEmphasis: StoryObj = { + args: { + overlayEmphasis: 'high', + }, + render: Default.render, +}; + /** * Modals can contain long, scrollable text. This is not recommended, however. */ @@ -187,12 +201,9 @@ export const WithLongTextScrollable: StoryObj = { - {/* This has to be manually tested since Tooltip tests are flaky in Chromatic */} - - - + @@ -235,12 +246,9 @@ export const ContentDefault: Story = { - {/* This has to be manually tested since Tooltip tests are flaky in Chromatic */} - - - + @@ -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', }, }; @@ -288,7 +296,7 @@ export const LayoutVertical: Story = { ...ContentDefault, args: { ...ContentDefault.args, - size: 'md', + size: 'lg', children: ( <> @@ -300,12 +308,9 @@ export const LayoutVertical: Story = { - {/* This has to be manually tested since Tooltip tests are flaky in Chromatic */} - - - + @@ -334,7 +339,7 @@ export const LayoutVerticalWithTertiary: Story = { ...ContentDefault, args: { ...ContentDefault.args, - size: 'md', + size: 'lg', children: ( <> @@ -346,12 +351,9 @@ export const LayoutVerticalWithTertiary: Story = { - {/* This has to be manually tested since Tooltip tests are flaky in Chromatic */} - - - + @@ -379,7 +381,7 @@ export const WithCriticalButton: Story = { ...ContentDefault, args: { ...ContentDefault.args, - size: 'md', + size: 'lg', children: ( <> @@ -391,12 +393,9 @@ export const WithCriticalButton: Story = { - {/* This has to be manually tested since Tooltip tests are flaky in Chromatic */} - - - + diff --git a/src/components/Modal/Modal-v2.test.tsx b/src/components/Modal/Modal-v2.test.tsx index 9af8960ad..a9db9bf49 100644 --- a/src/components/Modal/Modal-v2.test.tsx +++ b/src/components/Modal/Modal-v2.test.tsx @@ -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(() => { diff --git a/src/components/Modal/Modal-v2.tsx b/src/components/Modal/Modal-v2.tsx index 9ac0b9e79..df4247563 100644 --- a/src/components/Modal/Modal-v2.tsx +++ b/src/components/Modal/Modal-v2.tsx @@ -5,8 +5,9 @@ import React from 'react'; import type { ExtractProps } from '../../util/utility-types'; import type { Size } from '../../util/variant-types'; + +import { ButtonV2 as Button } from '../Button'; import Heading from '../Heading'; -import { IconV2 as Icon } from '../Icon'; import Text from '../Text'; import styles from './Modal-v2.module.css'; @@ -78,12 +79,17 @@ type ModalContentProps = { onClose: () => void; // Design API /** - * Max size of the modal. Defaults to 'lg'. - * Will still break responsively. + * Max size of the modal, which responds to the viewport * * **Default is `"lg"`**. */ - size?: Extract; + size?: Extract; + /** + * Emphasis used on the backgound overlay (behind the modal) + * + * **Default is `"low"`**. + */ + overlayEmphasis?: 'low' | 'high'; }; type ModalProps = ModalContentProps & { @@ -230,29 +236,27 @@ export const ModalContent = (props: ModalContentProps) => { const componentClassName = clsx( styles['modal__content'], isScrollable && styles['modal__content--scrollable'], - (size === 'md' || size === 'lg') && styles['modal__content--md'], - size === 'lg' && styles['modal__content--lg'], + size && styles[`modal__content--${size}`], className, ); - const closeIconClassName = clsx(styles['modal__close-icon']); - return (
- {/* TODO: this should be a button instance? */} {!hideCloseButton && ( - + )} - {children}
@@ -277,6 +281,7 @@ export const Modal = (props: ModalProps) => { modalContainerClassName, onClose, open, + overlayEmphasis = 'low', ...rest } = props; const { children } = rest; @@ -310,7 +315,13 @@ export const Modal = (props: ModalProps) => { // Passing onClose to the Dialog allows it to close the modal when the ESC key is triggered. onClose={onClose} > - + diff --git a/src/components/Modal/__snapshots__/Modal-v2.test.tsx.snap b/src/components/Modal/__snapshots__/Modal-v2.test.tsx.snap index f02068678..1bf94df45 100644 --- a/src/components/Modal/__snapshots__/Modal-v2.test.tsx.snap +++ b/src/components/Modal/__snapshots__/Modal-v2.test.tsx.snap @@ -2,30 +2,33 @@ exports[`Modal ContentDefault story renders snapshot 1`] = `