diff --git a/.storybook/modes.ts b/.storybook/modes.ts index 3e7c9b8d46..851e19b696 100644 --- a/.storybook/modes.ts +++ b/.storybook/modes.ts @@ -8,4 +8,19 @@ export const allModes = { width: 900, }, }, + xsm: { + viewport: "xsm", + }, + sm: { + viewport: "sm", + }, + md: { + viewport: "md", + }, + lg: { + viewport: "lg", + }, + xl: { + viewport: "xl", + }, }; diff --git a/.storybook/preview.ts b/.storybook/preview.ts index a9a479e7fd..e2ea271f1d 100644 --- a/.storybook/preview.ts +++ b/.storybook/preview.ts @@ -9,41 +9,11 @@ import isChromatic from "./isChromatic"; import { Preview } from "@storybook/react"; const customViewports = { - extraSmall: { - name: "Smart Phones", - styles: { - width: "320px", - height: "599px", - }, - }, - small: { - name: "Portrait Tablets", - styles: { - width: "600px", - height: "959px", - }, - }, - medium: { - name: "Landscape Tablets & Low-Res Laptops", - styles: { - width: "960px", - height: "1259px", - }, - }, - large: { - name: "High-Res Laptops & Monitors", - styles: { - width: "1260px", - height: "1920px", - }, - }, - extraLarge: { - name: "Ultra High-Res Monitors", - styles: { - width: "1921px", - height: "2500px", - }, - }, + xsm: { name: "Extra small", styles: { width: "320px", height: "900px" } }, + sm: { name: "Small", styles: { width: "640px", height: "900px" } }, + md: { name: "Medium", styles: { width: "768px", height: "900px" } }, + lg: { name: "Large", styles: { width: "1024px", height: "900px" } }, + xl: { name: "Extra large", styles: { width: "1280px", height: "900px" } }, }; const parameters = { diff --git a/CHANGELOG.md b/CHANGELOG.md index ef4f57b9f6..5920efcd5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,12 @@ +### [144.0.1](https://github.com/Sage/carbon/compare/v144.0.0...v144.0.1) (2024-10-23) + + +### Bug Fixes + +* **dialog:** ensure height never exceeds 90% of the viewport height ([619a651](https://github.com/Sage/carbon/commit/619a6516ac3b0ac05e55219e79df529b491a9031)) +* prevent sticky footer form content from overflowing in Carbon modal components ([cb77fb7](https://github.com/Sage/carbon/commit/cb77fb78f72d866458df49964c08f567b971a110)), closes [#6969](https://github.com/Sage/carbon/issues/6969) +* resolve layout issues with sticky footer forms inside Carbon modal components ([0fe249d](https://github.com/Sage/carbon/commit/0fe249db3d2e598f73eaccd3388e6feea61c2c80)) + ## [144.0.0](https://github.com/Sage/carbon/compare/v143.2.5...v144.0.0) (2024-10-21) diff --git a/package-lock.json b/package-lock.json index 81ab8874c6..b65ff91a10 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "carbon-react", - "version": "144.0.0", + "version": "144.0.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "carbon-react", - "version": "144.0.0", + "version": "144.0.1", "hasInstallScript": true, "license": "Apache-2.0", "dependencies": { diff --git a/package.json b/package.json index 6e29ff8b89..8a83b4cd2c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "carbon-react", - "version": "144.0.0", + "version": "144.0.1", "description": "A library of reusable React components for easily building user interfaces.", "files": [ "lib", diff --git a/playwright/components/alert/index.ts b/playwright/components/alert/index.ts index 2045f454b9..ddf73759d1 100644 --- a/playwright/components/alert/index.ts +++ b/playwright/components/alert/index.ts @@ -27,15 +27,4 @@ const alertDialog = (page: Page) => { return page.locator(ALERT_DIALOG); }; -const alertChildren = (page: Page) => { - return page.locator('[data-component="alert"] div:nth-of-type(2) div'); -}; - -export { - alert, - alertCrossIcon, - alertTitle, - alertSubtitle, - alertDialog, - alertChildren, -}; +export { alert, alertCrossIcon, alertTitle, alertSubtitle, alertDialog }; diff --git a/playwright/components/select/index.ts b/playwright/components/select/index.ts index 3b79b12fe3..c5f2f19bc4 100644 --- a/playwright/components/select/index.ts +++ b/playwright/components/select/index.ts @@ -14,7 +14,6 @@ import { SELECT_LIST_SCROLLABLE_WRAPPER, } from "./locators"; import { PILL_PREVIEW } from "../pill/locators"; -import { ALERT_DIALOG } from "../dialog/locators"; import { getDataElementByValue } from ".."; // component preview locators @@ -102,8 +101,5 @@ export const filterableSelectAddElementButton = (page: Page) => export const filterableSelectButtonIcon = (page: Page) => filterableSelectAddElementButton(page).locator("span:nth-child(2)"); -export const filterableSelectAddNewButton = (page: Page) => - page.locator(ALERT_DIALOG).locator("div:nth-child(3) > div > button"); - export const selectResetButton = (page: Page) => page.locator(SELECT_RESET_BUTTON); diff --git a/src/components/advanced-color-picker/advanced-color-picker.pw.tsx b/src/components/advanced-color-picker/advanced-color-picker.pw.tsx index d6da3c6ea5..74154473a5 100644 --- a/src/components/advanced-color-picker/advanced-color-picker.pw.tsx +++ b/src/components/advanced-color-picker/advanced-color-picker.pw.tsx @@ -396,25 +396,6 @@ test.describe( await elementToBlur.blur(); expect(callbackCount).toBe(0); }); - - test("should call onBlur callback when a blur event is triggered", async ({ - mount, - page, - }) => { - let callbackCount = 0; - await mount( - { - callbackCount += 1; - }} - /> - ); - - const elementToFocus = simpleColorPickerInput(page, 7); - await elementToFocus.focus(); - await page.locator("body").click({ position: { x: 0, y: 0 } }); - expect(callbackCount).toBe(1); - }); } ); diff --git a/src/components/advanced-color-picker/advanced-color-picker.style.ts b/src/components/advanced-color-picker/advanced-color-picker.style.ts index dca9255175..2861f67ea9 100644 --- a/src/components/advanced-color-picker/advanced-color-picker.style.ts +++ b/src/components/advanced-color-picker/advanced-color-picker.style.ts @@ -3,10 +3,7 @@ import { margin } from "styled-system"; import StyledAdvancedColorPickerCell from "./advanced-color-picker-cell.style"; import { StyledColorOptions } from "../simple-color-picker/simple-color-picker.style"; import { StyledSimpleColor } from "../simple-color-picker/simple-color/simple-color.style"; -import { - StyledDialogContent, - StyledDialogInnerContent, -} from "../dialog/dialog.style"; +import { StyledDialogContent } from "../dialog/dialog.style"; import Dialog from "../dialog/dialog.component"; import StyledIconButton from "../icon-button/icon-button.style"; import checkerBoardSvg from "../simple-color-picker/simple-color/checker-board.svg"; @@ -59,10 +56,6 @@ const DialogStyle = styled(Dialog)` padding: var(--spacing200); } - ${StyledDialogInnerContent} { - padding: 0; - } - ${StyledColorOptions} { max-width: 285px; ${StyledSimpleColor} { diff --git a/src/components/alert/alert.pw.tsx b/src/components/alert/alert.pw.tsx index 63e4aa0520..71b68e6973 100644 --- a/src/components/alert/alert.pw.tsx +++ b/src/components/alert/alert.pw.tsx @@ -4,7 +4,6 @@ import { alertCrossIcon, alertTitle, alertSubtitle, - alertChildren, alertDialog, } from "../../../playwright/components/alert"; import { @@ -50,9 +49,7 @@ test.describe("should render Alert component", () => { test(`with ${text} as children`, async ({ mount, page }) => { await mount({text}); - const children = alertChildren(page); - const alertChildrenText = await children.textContent(); - expect(alertChildrenText).toEqual(text); + await expect(page.getByText(text)).toBeVisible(); }); }); diff --git a/src/components/confirm/confirm.pw.tsx b/src/components/confirm/confirm.pw.tsx index 375a3f7869..5d36dcd14b 100644 --- a/src/components/confirm/confirm.pw.tsx +++ b/src/components/confirm/confirm.pw.tsx @@ -14,6 +14,7 @@ import { assertCssValueIsApproximately, checkAccessibility, checkDialogIsInDOM, + getStyle, waitForAnimationEnd, } from "../../../playwright/support/helper"; import { SIZE, CHARACTERS } from "../../../playwright/support/constants"; @@ -105,28 +106,30 @@ test.describe("should render Confirm component", () => { }); }); - heights.forEach(([heightnumber, heightstring]) => { - test(`should check Confirm height is ${heightstring}px`, async ({ + ["0px", "100px", "500px"].forEach((height) => { + test(`height of Confirm dialog is ${height} when height prop is ${height}`, async ({ mount, page, }) => { - await mount(); + await page.setViewportSize({ width: 600, height: 1000 }); + await mount(); - const viewportHeight = 768; + await expect(page.getByRole("alertdialog")).toHaveCSS("height", height); + }); + }); - let resultHeight: number; - if (heightnumber >= viewportHeight - 20) { - resultHeight = viewportHeight - 20; - } else { - resultHeight = heightnumber; - } + test("Confirm dialog's height does not exceed the height of the viewport", async ({ + mount, + page, + }) => { + await page.setViewportSize({ width: 600, height: 1000 }); + await mount(); - await assertCssValueIsApproximately( - page.getByRole("alertdialog"), - "height", - resultHeight - ); - }); + const actualDialogHeight = parseInt( + await getStyle(page.getByRole("alertdialog"), "height") + ); + + expect(actualDialogHeight).toBeLessThanOrEqual(1000); }); ([ diff --git a/src/components/date/date.pw.tsx b/src/components/date/date.pw.tsx index 71b21b231c..34b8335059 100644 --- a/src/components/date/date.pw.tsx +++ b/src/components/date/date.pw.tsx @@ -1,7 +1,6 @@ import React from "react"; import { expect, test } from "@playwright/experimental-ct-react17"; import dayjs from "dayjs"; -import Confirm from "../confirm"; import { DateInputCustom, DateInputValidationNewDesign, @@ -784,26 +783,48 @@ test.describe("Functionality tests", () => { await expect(wrapper).toBeVisible(); }); - [true, false].forEach((state) => { - test(`should render with disablePortal prop ${state}`, async ({ - mount, - page, - }) => { - await mount( - {}}> - - - ); + test("date picker does not float above the rest of the page, when disablePortal prop is true", async ({ + mount, + page, + }) => { + await mount( +
+ +
+ ); - const input = getDataElementByValue(page, "input"); - await input.click(); - const wrapper = dayPickerWrapper(page); - if (state) { - await expect(wrapper).not.toBeInViewport(); - } else { - await expect(wrapper).toBeInViewport(); - } - }); + const input = page.getByLabel("Date"); + await input.click(); + const datePicker = dayPickerWrapper(page); + await expect(datePicker).not.toBeInViewport(); + }); + test("date picker floats above the rest of the page, when disablePortal prop is false", async ({ + mount, + page, + }) => { + await mount( +
+ +
+ ); + const input = page.getByLabel("Date"); + await input.click(); + const datePicker = dayPickerWrapper(page); + await expect(datePicker).toBeInViewport(); }); test(`should have the expected border radius styling`, async ({ diff --git a/src/components/dialog-full-screen/content.style.ts b/src/components/dialog-full-screen/content.style.ts index 6aa4d72be0..f824076f35 100644 --- a/src/components/dialog-full-screen/content.style.ts +++ b/src/components/dialog-full-screen/content.style.ts @@ -1,94 +1,50 @@ import styled, { css } from "styled-components"; - -import { StyledFormFooter, StyledFormContent } from "../form/form.style"; +import { StyledForm, StyledFormContent } from "../form/form.style"; type StyledContentProps = { hasHeader: boolean; disableContentPadding?: boolean; }; +function computePadding() { + return css` + padding: 0 16px; + @media screen and (min-width: 600px) { + padding: 0 24px; + } + @media screen and (min-width: 960px) { + padding: 0 32px; + } + @media screen and (min-width: 1260px) { + padding: 0 40px; + } + `; +} + const StyledContent = styled.div` + box-sizing: border-box; + display: block; overflow-y: auto; - padding: 0 16px; - flex: 1; - - /* Delegate handling overflow to child form if it has a sticky footer */ - &:has(${StyledFormContent}.sticky) { - overflow-y: inherit; - } - - ${({ disableContentPadding }) => css` - ${!disableContentPadding && - css` - @media screen and (min-width: 600px) { - padding: 0 24px; - } - @media screen and (min-width: 960px) { - padding: 0 32px; - } - @media screen and (min-width: 1260px) { - padding: 0 40px; - } - - ${StyledFormContent}.sticky { - padding-right: 16px; - padding-left: 16px; - margin-right: -16px; - margin-left: -16px; - @media screen and (min-width: 600px) { - padding-right: 24px; - padding-left: 24px; - margin-right: -24px; - margin-left: -24px; - } - @media screen and (min-width: 960px) { - padding-right: 32px; - padding-left: 32px; - margin-right: -32px; - margin-left: -32px; - } - @media screen and (min-width: 1260px) { - padding-right: 40px; - padding-left: 40px; - margin-right: -40px; - margin-left: -40px; - } - } + flex: 1; + width: 100%; - ${StyledFormFooter}.sticky { - padding: 16px; + ${({ disableContentPadding }) => + disableContentPadding ? "padding: 0" : computePadding()} - margin-right: -16px; - margin-left: -16px; - width: calc(100% + 32px); + &:has(${StyledForm}.sticky) { + display: flex; + flex-direction: column; + overflow-y: hidden; + padding: 0; - @media screen and (min-width: 600px) { - padding: 16px 24px; - margin-right: -24px; - margin-left: -24px; - width: calc(100% + 48px); - } - @media screen and (min-width: 960px) { - padding: 16px 32px; - margin-right: -32px; - margin-left: -32px; - width: calc(100% + 64px); - } - @media screen and (min-width: 1260px) { - padding: 16px 40px; - margin-right: -40px; - margin-left: -40px; - width: calc(100% + 80px); - } + ${StyledForm}.sticky { + ${StyledFormContent} { + ${({ disableContentPadding }) => + disableContentPadding ? "padding: 0" : computePadding()} } - `} - - ${disableContentPadding && - css` - padding: 0; - `} - `} + } + } ${({ hasHeader }) => !hasHeader && diff --git a/src/components/dialog-full-screen/dialog-full-screen-test.stories.tsx b/src/components/dialog-full-screen/dialog-full-screen-test.stories.tsx index ef67d29366..de44eddb64 100644 --- a/src/components/dialog-full-screen/dialog-full-screen-test.stories.tsx +++ b/src/components/dialog-full-screen/dialog-full-screen-test.stories.tsx @@ -24,7 +24,7 @@ export const Default: StoryType = { return {children}; }, args: { - children: "Content", + children: , open: true, title: "Example Dialog", subtitle: "Example Subtitle", @@ -216,3 +216,33 @@ WithTwoDifferentNodes.decorators = [ WithTwoDifferentNodes.parameters = { layout: "fullscreen", }; + +export const WithWrappedStickyForm: StoryType = { + args: { + children: ( + +
{}}>Cancel} + saveButton={ + + } + > + + + + + + + +
+ ), + open: true, + onCancel: () => {}, + title: "Title", + subtitle: "Subtitle", + }, + parameters: { chromatic: { disableSnapshot: true } }, +}; diff --git a/src/components/dialog-full-screen/dialog-full-screen.mdx b/src/components/dialog-full-screen/dialog-full-screen.mdx index 853a349c6c..502c03472f 100644 --- a/src/components/dialog-full-screen/dialog-full-screen.mdx +++ b/src/components/dialog-full-screen/dialog-full-screen.mdx @@ -57,10 +57,6 @@ to have a possibility to manage active `Tabs` group -### With `Box` - - - ### Overriding the first focused element By default, when a dialog is opened it will automatically focus the first element within its children that can be focussed. diff --git a/src/components/dialog-full-screen/dialog-full-screen.pw.tsx b/src/components/dialog-full-screen/dialog-full-screen.pw.tsx index 599dbd0b13..8a9afff898 100644 --- a/src/components/dialog-full-screen/dialog-full-screen.pw.tsx +++ b/src/components/dialog-full-screen/dialog-full-screen.pw.tsx @@ -561,7 +561,9 @@ test.describe("Accessibility for DialogFullScreen", () => { .getByRole("button") .filter({ hasText: "Open DialogFullScreen" }); await openButton.click(); - await checkAccessibility(page); + + // color-contrast ignored until we can investigate and fix FE-6245 + await checkAccessibility(page, page.getByRole("dialog"), "color-contrast"); }); test("should check accessibility using autoFocus", async ({ diff --git a/src/components/dialog-full-screen/dialog-full-screen.stories.tsx b/src/components/dialog-full-screen/dialog-full-screen.stories.tsx index 0e7cf05572..add20392ea 100644 --- a/src/components/dialog-full-screen/dialog-full-screen.stories.tsx +++ b/src/components/dialog-full-screen/dialog-full-screen.stories.tsx @@ -718,47 +718,6 @@ WithHideableHeaderChildren.parameters = { chromatic: { disableSnapshot: true }, }; -export const WithBox: Story = () => { - const [isOpen, setIsOpen] = useState(false); - return ( - <> - - setIsOpen(false)} - title="Title" - subtitle="Subtitle" - > - -
setIsOpen(false)}>Cancel - } - saveButton={ - - } - > - - This is an example of a full screen Dialog with a Form as content - - - - - - - - -
-
- - ); -}; -WithBox.storyName = "With Box"; -WithBox.parameters = { chromatic: { disableSnapshot: true } }; - export const FocusingADifferentFirstElement: Story = () => { const [isOpenOne, setIsOpenOne] = useState(false); const [isOpenTwo, setIsOpenTwo] = useState(false); @@ -839,7 +798,6 @@ export const OtherFocusableContainers: Story = () => { >
setIsDialogOpen(false)}>Cancel } diff --git a/src/components/dialog-full-screen/dialog-full-screen.style.ts b/src/components/dialog-full-screen/dialog-full-screen.style.ts index 6f5e69d6b3..34e0b0ddf4 100644 --- a/src/components/dialog-full-screen/dialog-full-screen.style.ts +++ b/src/components/dialog-full-screen/dialog-full-screen.style.ts @@ -8,7 +8,6 @@ import { StyledHeaderContent, StyledHeading, } from "../heading/heading.style"; -import { StyledForm } from "../form/form.style"; const StyledDialogFullScreen = styled.div<{ pagesStyling?: boolean }>` :focus { @@ -27,10 +26,6 @@ const StyledDialogFullScreen = styled.div<{ pagesStyling?: boolean }>` display: flex; flex-direction: column; - ${StyledForm} { - min-height: 100%; - } - ${StyledHeaderContent} { align-items: baseline; } diff --git a/src/components/dialog-full-screen/dialog-full-screen.test.tsx b/src/components/dialog-full-screen/dialog-full-screen.test.tsx index 3cbea9414b..f25f95a930 100644 --- a/src/components/dialog-full-screen/dialog-full-screen.test.tsx +++ b/src/components/dialog-full-screen/dialog-full-screen.test.tsx @@ -12,7 +12,6 @@ import StyledContent from "./content.style"; import StyledIconButton from "../icon-button/icon-button.style"; import { StyledHeader, StyledHeading } from "../heading/heading.style"; import Form from "../form"; -import { StyledFormContent } from "../form/form.style"; import CarbonProvider from "../carbon-provider"; const ControlledDialog = ({ @@ -244,10 +243,10 @@ test("padding is removed from the content when the `disableContentPadding` prop ); - expect(screen.getByTestId("dialog-full-screen-content")).toHaveStyleRule( - "padding", - "0" - ); + const content = screen.getByTestId("dialog-full-screen-content"); + expect(content).toHaveStyle({ + padding: "0px 0px 0px 0px", + }); }); /** Remove this when after Pages is re-written */ @@ -317,22 +316,6 @@ test("when a Form child does not have a sticky footer, overflow styling is set o ); }); -test("when a Form child has a sticky footer, no overflow styling is set", () => { - render( - - - - ); - - expect(screen.getByTestId("dialog-full-screen-content")).toHaveStyleRule( - "overflow-y", - "inherit", - { - modifier: `&:has(${StyledFormContent}.sticky)`, - } - ); -}); - test("when the `title` prop is a string, this value is set as the dialog's accessible name", () => { render(); diff --git a/src/components/dialog/components.test-pw.tsx b/src/components/dialog/components.test-pw.tsx index b5f592a384..27b28a3762 100644 --- a/src/components/dialog/components.test-pw.tsx +++ b/src/components/dialog/components.test-pw.tsx @@ -223,7 +223,6 @@ export const DefaultStory = () => { > setIsOpen(false)}>Cancel } @@ -269,7 +268,6 @@ export const Editable = () => { > setIsOpen(false)}>Cancel } @@ -335,7 +333,6 @@ export const WithHelp = () => { > setIsOpen(false)}>Cancel } @@ -470,7 +467,6 @@ export const OverridingContentPadding = () => { > setIsOpen(false)}>Cancel } @@ -520,7 +516,6 @@ export const OtherFocusableContainers = () => { > setIsDialogOpen(false)}>Cancel } @@ -592,7 +587,6 @@ export const Responsive = () => { > setIsOpen(false)}>Cancel } diff --git a/src/components/dialog/dialog-test.stories.tsx b/src/components/dialog/dialog-test.stories.tsx index 57c478132a..a7bf2595ca 100644 --- a/src/components/dialog/dialog-test.stories.tsx +++ b/src/components/dialog/dialog-test.stories.tsx @@ -15,6 +15,16 @@ import { Checkbox } from "../checkbox"; import { Select, Option } from "../select"; import TextEditor from "../text-editor"; +import Box from "../box"; +import Typography from "../typography"; +import { + FlexTileCell, + FlexTileContainer, + FlexTileDivider, + Tile, +} from "../tile"; +import { allModes } from "../../../.storybook/modes"; + export default { title: "Dialog/Test", component: Dialog, @@ -290,6 +300,197 @@ MaxSizeTestNonOverflowedForm.decorators = [ MaxSizeTestNonOverflowedForm.parameters = { themeProvider: { chromatic: { theme: "none" } }, - chromatic: { disableSnapshot: false, viewports: [1200, 900] }, + chromatic: { + disableSnapshot: false, + modes: { + lg: allModes.lg, + xsm: allModes.xsm, + }, + }, layout: "fullscreen", }; + +export const DialogWithLongHeaderContent: StoryType = { + parameters: { + chromatic: { + disableSnapshot: false, + modes: { + lg: allModes.lg, + xsm: allModes.xsm, + }, + }, + layout: "fullscreen", + }, + decorators: [ + (Story) => ( +
+ +
+ ), + ], + render: ({ size, ...args }) => ( + + + + Lorem ipsum dolor sit amet, consectetur adipiscing elit. + + + Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. + Ut enim ad minim veniam. + + + } + > + Submit} + > + + + + + + 1 + + + + + + + + 1 + + + + + + + + 1 + + + + + + + + 1 + + + + + + + + 1 + + + + + + + + 1 + + + + + + + + 1 + + + + + + + + 1 + + + + + + + + 1 + + + + + + + + 1 + + + + + + + + 1 + + + + + + + + 1 + + + + + + + + 1 + + + + + + + + 1 + + + + + + + + 1 + + + + + + + + 1 + + + + + + + ), +}; + +export const WithButton = { + render: () => { + return ( + + + + ); + }, +}; diff --git a/src/components/dialog/dialog.component.tsx b/src/components/dialog/dialog.component.tsx index 8d727fcb1f..33719685b1 100644 --- a/src/components/dialog/dialog.component.tsx +++ b/src/components/dialog/dialog.component.tsx @@ -1,26 +1,17 @@ -import React, { - useRef, - useEffect, - useLayoutEffect, - useCallback, - useImperativeHandle, - forwardRef, - useState, -} from "react"; +import React, { useRef, useImperativeHandle, forwardRef } from "react"; import createGuid from "../../__internal__/utils/helpers/guid"; import Modal, { ModalProps } from "../modal"; import Heading from "../heading"; import tagComponent, { TagProps } from "../../__internal__/utils/helpers/tags"; -import useResizeObserver from "../../hooks/__internal__/useResizeObserver"; import { StyledDialog, StyledDialogTitle, StyledDialogContent, - StyledDialogInnerContent, + DialogPositioner, } from "./dialog.style"; -import { DialogSizes, TOP_MARGIN } from "./dialog.config"; +import { DialogSizes } from "./dialog.config"; import FocusTrap, { CustomRefObject } from "../../__internal__/focus-trap"; import IconButton from "../icon-button"; @@ -139,13 +130,7 @@ export const Dialog = forwardRef( const locale = useLocale(); const containerRef = useRef(null); - const innerContentRef = useRef(null); const titleRef = useRef(null); - const [breakpointOffset, setBreakpointOffset] = useState< - number | undefined - >(undefined); - const isDialogMaximised = size === "maximise"; - const listenersAdded = useRef(false); const { current: titleId } = useRef(createGuid()); const { current: subtitleId } = useRef(createGuid()); @@ -161,122 +146,41 @@ export const Dialog = forwardRef( [] ); - const centerDialog = useCallback(() => { - /* istanbul ignore if */ - if (!containerRef.current) { - return; - } - - const { - width: dialogWidth, - height: dialogHeight, - } = containerRef.current.getBoundingClientRect(); - - let midPointY = window.innerHeight / 2; - let midPointX = window.innerWidth / 2; - - midPointY -= dialogHeight / 2; - midPointX -= dialogWidth / 2; - - if (midPointY < TOP_MARGIN) { - midPointY = TOP_MARGIN; - } - - if (midPointX < 0) { - midPointX = 0; - } - - if (isDialogMaximised) { - const breakPoint = window.innerWidth > 960 ? 32 : 16; - midPointX = breakPoint; - midPointY = breakPoint; - setBreakpointOffset(breakPoint); - } - - containerRef.current.style.top = `${midPointY}px`; - containerRef.current.style.left = `${midPointX}px`; - }, [size]); - - useResizeObserver(innerContentRef, centerDialog, !open); - - const addListeners = useCallback(() => { - /* istanbul ignore else */ - if (!listenersAdded.current) { - window.addEventListener("resize", centerDialog); - listenersAdded.current = true; - } - }, [centerDialog]); - - const removeListeners = useCallback(() => { - if (listenersAdded.current) { - window.removeEventListener("resize", centerDialog); - listenersAdded.current = false; - } - }, [centerDialog]); - - useEffect(() => { - if (open) { - addListeners(); - } - - if (!open) { - removeListeners(); - } - - return () => { - removeListeners(); - }; - }, [open, addListeners, removeListeners]); - - useLayoutEffect(() => { - if (open) { - centerDialog(); - } - }, [centerDialog, open, height]); - - const closeIcon = () => { - if (!showCloseIcon || !onCancel) return null; - - return ( - - - - ); - }; - - const dialogTitle = () => { - if (!title) return null; + const closeIcon = showCloseIcon && onCancel && ( + + + + ); - return ( - - {typeof title === "string" ? ( - - ) : ( - title - )} - - ); - }; + const dialogTitle = title && ( + + {typeof title === "string" ? ( + + ) : ( + title + )} + + ); let dialogHeight = height; @@ -314,43 +218,34 @@ export const Dialog = forwardRef( isOpen={open} additionalWrapperRefs={focusableContainers} > - - {dialogTitle()} - {closeIcon()} - + - {children} - - - + + + ); diff --git a/src/components/dialog/dialog.config.ts b/src/components/dialog/dialog.config.ts index 1d01ddf08d..9205b82ba5 100644 --- a/src/components/dialog/dialog.config.ts +++ b/src/components/dialog/dialog.config.ts @@ -9,9 +9,5 @@ export const DIALOG_SIZES = [ "extra-large", "maximise", ] as const; -export const TOP_MARGIN = 20; -export const CONTENT_TOP_PADDING = 24; -export const HORIZONTAL_PADDING = 32; -export const CONTENT_BOTTOM_PADDING = 30; export type DialogSizes = typeof DIALOG_SIZES[number]; diff --git a/src/components/dialog/dialog.stories.tsx b/src/components/dialog/dialog.stories.tsx index 69b0463950..e979413b91 100644 --- a/src/components/dialog/dialog.stories.tsx +++ b/src/components/dialog/dialog.stories.tsx @@ -1,5 +1,6 @@ import React, { useRef, useState } from "react"; import { Meta, StoryObj } from "@storybook/react"; +import { useArgs } from "@storybook/preview-api"; import isChromatic from "../../../.storybook/isChromatic"; import { allModes } from "../../../.storybook/modes"; @@ -20,25 +21,24 @@ import useMediaQuery from "../../hooks/useMediaQuery"; import type { DialogHandle } from "."; import Dialog from "."; -const defaultOpenState = isChromatic(); - const meta: Meta = { title: "Dialog", component: Dialog, parameters: { themeProvider: { chromatic: { theme: "sage" } }, + layout: isChromatic() ? "fullscreen" : "padded", controls: { disable: true }, chromatic: { modes: { - desktop: allModes.chromatic, + lg: allModes.lg, }, }, }, decorators: [ (Story) => ( <> - {defaultOpenState ? ( - + {isChromatic() ? ( + ) : ( @@ -52,96 +52,79 @@ const meta: Meta = { export default meta; type Story = StoryObj; -export const DefaultStory: Story = () => { - const [isOpen, setIsOpen] = useState(defaultOpenState); - return ( - <> - - setIsOpen(false)} - title="Title" - subtitle="Subtitle" - > -
setIsOpen(false)}>Cancel - } - saveButton={ - - } +const defaultOpenState = isChromatic(); + +export const DefaultStory: Story = { + name: "Default", + args: { + open: isChromatic(), + title: "Title", + subtitle: "Subtitle", + }, + render: function DefaultStory({ onCancel, ...args }) { + const [{ open }, updateArgs] = useArgs(); + return ( + <> + + { + onCancel?.(ev); + updateArgs({ open: false }); + }} > - - This is an example of a dialog with a Form as content - - - - - - - - - - - - - - -
- - ); +
updateArgs({ open: false })}> + Cancel + + } + saveButton={ + + } + > + + This is an example of a dialog with a Form as content + + + + + + + + + + + + + + +
+ + ); + }, }; -DefaultStory.storyName = "Default"; -export const MaxSize: Story = () => { - const [isOpen, setIsOpen] = useState(false); - return ( - <> - - setIsOpen(false)} - title="Title" - subtitle="Subtitle" - > -
setIsOpen(false)}>Cancel - } - saveButton={ - - } - > - - This is an example of a dialog with a Form as content - - - - - - - - - - - - - - -
- - ); +export const MaxSize: Story = { + ...DefaultStory, + name: "With Max Size", + args: { + ...DefaultStory.args, + size: "maximise", + }, + parameters: { + chromatic: { + modes: { + xsm: allModes.xsm, + lg: allModes.lg, + }, + }, + }, }; -MaxSize.storyName = "With Max Size"; -MaxSize.parameters = { chromatic: { disableSnapshot: true } }; export const Editable: Story = () => { const [isOpen, setIsOpen] = useState(defaultOpenState); @@ -158,7 +141,6 @@ export const Editable: Story = () => { >
setIsOpen(false)}>Cancel } @@ -226,7 +208,6 @@ export const WithHelp: Story = () => { > setIsOpen(false)}>Cancel } @@ -354,51 +335,14 @@ FocusingADifferentFirstElement.parameters = { chromatic: { disableSnapshot: true }, }; -export const OverridingContentPadding: Story = () => { - const [isOpen, setIsOpen] = useState(defaultOpenState); - return ( - <> - - setIsOpen(false)} - title="Title" - subtitle="Subtitle" - contentPadding={{ p: 0 }} - > - setIsOpen(false)}>Cancel - } - saveButton={ - - } - > - - This is an example of a dialog with a Form as content - - - - - - - - - - - - - - -
- - ); +export const OverridingContentPadding: Story = { + ...DefaultStory, + name: "Overriding Content Padding", + args: { + ...DefaultStory.args, + contentPadding: { p: 0 }, + }, }; -OverridingContentPadding.storyName = "Overriding Content Padding"; export const OtherFocusableContainers: Story = () => { const [isDialogOpen, setIsDialogOpen] = useState(false); @@ -419,7 +363,6 @@ export const OtherFocusableContainers: Story = () => { >
setIsDialogOpen(false)}>Cancel } @@ -495,7 +438,6 @@ export const Responsive: Story = () => { > setIsOpen(false)}>Cancel } @@ -617,48 +559,11 @@ export const TopModalOverride: Story = () => { }; TopModalOverride.storyName = "Top Modal Override"; -export const GreyBackground: Story = () => { - const [isOpen, setIsOpen] = useState(defaultOpenState); - return ( - <> - - setIsOpen(false)} - title="Title" - subtitle="Subtitle" - greyBackground - > - setIsOpen(false)}>Cancel - } - saveButton={ - - } - > - - This is an example of a dialog with a Form as content - - - - - - - - - - - - - - -
- - ); +export const GreyBackground: Story = { + ...DefaultStory, + name: "Grey Background", + args: { + ...DefaultStory.args, + greyBackground: true, + }, }; -GreyBackground.storyName = "Grey Background"; diff --git a/src/components/dialog/dialog.style.ts b/src/components/dialog/dialog.style.ts index ea646ca7e9..91dc9923a6 100644 --- a/src/components/dialog/dialog.style.ts +++ b/src/components/dialog/dialog.style.ts @@ -1,29 +1,18 @@ import styled, { css } from "styled-components"; import { padding as paddingFn } from "styled-system"; - import baseTheme from "../../style/themes/base"; -import { - calculateFormSpacingValues, - calculateWidthValue, -} from "../../style/utils/form-style-utils"; -import { - StyledForm, - StyledFormFooter, - StyledFormContent, -} from "../form/form.style"; import { StyledHeaderContent, StyledHeading, StyledHeadingTitle, } from "../heading/heading.style"; import StyledIconButton from "../icon-button/icon-button.style"; +import { ContentPaddingInterface, DialogProps } from "./dialog.component"; import { - HORIZONTAL_PADDING, - CONTENT_TOP_PADDING, - CONTENT_BOTTOM_PADDING, - DialogSizes, -} from "./dialog.config"; -import { ContentPaddingInterface } from "./dialog.component"; + StyledFormContent, + StyledForm, + StyledFormFooter, +} from "../form/form.style"; const dialogSizes = { auto: "fit-content", @@ -36,45 +25,43 @@ const dialogSizes = { "extra-large": "1080px", }; -const calculatePaddingTopInnerContent = ({ - py, - p, -}: { - py?: ContentPaddingInterface["py"]; - p?: ContentPaddingInterface["p"]; -}) => - [py, p].some((padding) => padding !== undefined) - ? 0 - : `${CONTENT_TOP_PADDING}px`; - -type StyledDialogProps = { - topMargin: number; - size?: DialogSizes; +type StyledDialogProps = Required> & { dialogHeight?: string; backgroundColor: string; }; -const StyledDialog = styled.div.attrs( - ({ topMargin, size }: StyledDialogProps) => { - const isDialogMaximised = size === "maximise"; - const isDialogMaximisedSmallViewport = topMargin === 32; - const isDialogMaximisedLargeViewport = topMargin === 64; - return { - isDialogMaximised, - isDialogMaximisedSmallViewport, - isDialogMaximisedLargeViewport, - }; - } -)` +const DialogPositioner = styled.div` + position: fixed; + inset: 0; + display: flex; + justify-content: center; + align-items: center; + z-index: ${({ theme }) => theme.zIndex.modal}; +`; + +const StyledDialog = styled.div` box-shadow: var(--boxShadow300); display: flex; flex-direction: column; + position: relative; border-radius: var(--borderRadius200); - position: fixed; - top: 50%; - z-index: ${({ theme }) => theme.zIndex.modal}; - max-height: ${({ topMargin }) => `calc(100vh - ${topMargin}px)`}; - ${({ isDialogMaximised }) => isDialogMaximised && "height: 100%"}; + + ${({ size }) => + size === "maximise" + ? css` + height: calc(100% - var(--spacing400)); + width: calc(100% - var(--spacing400)); + + @media screen and (min-width: 960px) { + height: calc(100% - var(--spacing800)); + width: calc(100% - var(--spacing800)); + } + ` + : css` + max-height: 90vh; + max-width: ${dialogSizes[size]}; + width: 100%; + `}; &:focus { outline: none; @@ -85,46 +72,12 @@ const StyledDialog = styled.div.attrs( background-color: ${backgroundColor}; `} - ${({ size, topMargin }) => - size && - css` - max-width: ${size === "maximise" - ? `calc(100vw - ${topMargin}px)` - : dialogSizes[size]}; - width: 100%; - `} - ${({ dialogHeight }) => dialogHeight && css` height: ${dialogHeight}px; `} - - /* We're overriding the max-height on the form content to account for a larger height when in a smaller viewport. - TODO: Remove this upon the completion of FE-6643. */ - ${StyledForm} { - ${({ isDialogMaximised, isDialogMaximisedSmallViewport }) => - isDialogMaximised && - css` - ${isDialogMaximisedSmallViewport && "max-height: calc(100vh - 184px);"} - height: 100%; - `} - - padding-bottom: 0px; - box-sizing: border-box; - } - - ${StyledFormContent}.sticky { - ${(props) => calculateFormSpacingValues(props, true)} - } - - ${StyledFormFooter}.sticky { - ${calculateWidthValue} - ${(props) => calculateFormSpacingValues(props, false)} - border-bottom-right-radius: var(--borderRadius200); - border-bottom-left-radius: var(--borderRadius200); - } - + > ${StyledIconButton} { margin: 0; position: absolute; @@ -145,7 +98,7 @@ type StyledDialogTitleProps = { const StyledDialogTitle = styled.div` background-color: var(--colorsUtilityYang100); - padding: 23px ${HORIZONTAL_PADDING}px 0; + padding: 23px 32px 0; border-bottom: 1px solid #ccd6db; border-top-right-radius: var(--borderRadius200); border-top-left-radius: var(--borderRadius200); @@ -170,28 +123,34 @@ const StyledDialogTitle = styled.div` const StyledDialogContent = styled.div` box-sizing: border-box; - display: flex; - flex-direction: column; + display: block; overflow-y: auto; - - /* Delegate handling overflow to child form if it has a sticky footer */ - &:has(${StyledFormContent}.sticky) { - overflow-y: inherit; - } - width: 100%; - flex: 1; - padding: 0px ${HORIZONTAL_PADDING}px ${CONTENT_BOTTOM_PADDING}px; + flex-grow: 1; + + padding: 24px 32px 30px; ${paddingFn} -`; -const StyledDialogInnerContent = styled.div` - position: relative; - flex: 1; - padding-top: ${calculatePaddingTopInnerContent}; + &:has(${StyledForm}.sticky) { + display: flex; + flex-direction: column; + padding: 0; + + ${StyledForm}.sticky { + ${StyledFormContent} { + padding: 24px 32px 30px; + ${paddingFn} + } + + ${StyledFormFooter} { + border-bottom-right-radius: var(--borderRadius200); + border-bottom-left-radius: var(--borderRadius200); + } + } + } `; -StyledDialog.defaultProps = { +DialogPositioner.defaultProps = { theme: baseTheme, }; @@ -200,8 +159,8 @@ StyledDialogContent.defaultProps = { }; export { + DialogPositioner, StyledDialog, StyledDialogTitle, StyledDialogContent, - StyledDialogInnerContent, }; diff --git a/src/components/dialog/dialog.test.tsx b/src/components/dialog/dialog.test.tsx index c129fd587a..955baafdee 100644 --- a/src/components/dialog/dialog.test.tsx +++ b/src/components/dialog/dialog.test.tsx @@ -8,7 +8,6 @@ import { import userEvent from "@testing-library/user-event"; import CarbonProvider from "../carbon-provider"; -import Form from "../form"; import Dialog, { DialogHandle, DialogProps } from "."; beforeEach(() => jest.useFakeTimers()); @@ -264,10 +263,10 @@ test("height prop controls the dialog's height", () => { expect(screen.getByRole("dialog")).toHaveStyle({ height: "500px" }); }); -test("dialog has correct max-height", () => { +test("maximum height of the dialog is 90% of the viewport height", () => { render(); expect(screen.getByRole("dialog")).toHaveStyle({ - maxHeight: "calc(100vh - 20px)", + maxHeight: "90vh", }); }); @@ -306,19 +305,6 @@ test("renders with grey background when greyBackground prop is passed", () => { }); }); -test("does not apply vertical overflow styling to the content container when it contains a Form with a sticky footer", () => { - render( - -
-
- ); - - const content = screen.getByTestId("dialog-content"); - - expect(content).not.toHaveStyle("overflow-y: auto"); - expect(content).not.toHaveStyle("overflow-y: scroll"); -}); - test("dialog is wrapped in a container, which has the correct class names set, when className prop is passed", () => { render(); @@ -331,70 +317,23 @@ test("dialog is wrapped in a container, which has the correct class names set, w expect(modalWrapper).toHaveClass("carbon-dialog special-dialog"); }); -test("renders dialog with 'left' set to 32px when 'size' is 'maximise' and window width is greater than 960px", () => { - window.innerWidth = 1000; - render(); - - const dialog = screen.getByRole("dialog", { name: /my dialog/i }); - expect(dialog).toHaveStyle({ left: "32px" }); -}); - -test("renders dialog with 'top' set to 32px when 'size' is 'maximise' and window width is greater than 960px", () => { - window.innerWidth = 1000; - render(); - - const dialog = screen.getByRole("dialog", { name: /my dialog/i }); - expect(dialog).toHaveStyle({ top: "32px" }); -}); - -test("renders dialog with 'left' set to 16px when 'size' is 'maximise' and window width is less than 960px", () => { - window.innerWidth = 700; - render(); - - const dialog = screen.getByRole("dialog", { name: /my dialog/i }); - expect(dialog).toHaveStyle({ left: "16px" }); -}); - -test("renders dialog with 'top' set to 16px when 'size' is 'maximise' and window width is less than 960px", () => { - window.innerWidth = 700; - render(); - - const dialog = screen.getByRole("dialog", { name: /my dialog/i }); - expect(dialog).toHaveStyle({ top: "16px" }); -}); - -test("dialog does not position itself any closer than 20px from the top of the viewport", () => { - const originalInnerHeight = window.innerHeight; - window.innerHeight = 300; - jest - .spyOn(Element.prototype, "getBoundingClientRect") - .mockReturnValue({ height: 361 } as DOMRect); - - render(); +test("dialog is positioned correctly, when size prop is maximise", () => { + render(); const dialog = screen.getByRole("dialog"); - - expect(dialog).toHaveStyle({ top: "20px" }); - - window.innerHeight = originalInnerHeight; - jest.restoreAllMocks(); + expect(dialog).toHaveStyle(` + height: calc(100% - var(--spacing400)); + width: calc(100% - var(--spacing400)); + `); }); -test("dialog does not position itself such that it goes off the left edge of the viewport", () => { - const originalInnerWidth = window.innerWidth; - window.innerWidth = 300; - jest - .spyOn(Element.prototype, "getBoundingClientRect") - .mockReturnValue({ width: 301 } as DOMRect); - - render(); - - const dialog = screen.getByRole("dialog"); - - expect(dialog).toHaveStyle({ left: "0px" }); - - window.innerWidth = originalInnerWidth; - jest.restoreAllMocks(); +test("prevents content from overflowing", () => { + render( + + Content + + ); + expect(screen.getByTestId("dialog-content")).toHaveStyle("overflow-y: auto"); }); test("no padding is rendered around dialog content, when zero padding is specified via contentPadding prop", () => { @@ -405,10 +344,8 @@ test("no padding is rendered around dialog content, when zero padding is specifi ); const content = screen.getByTestId("dialog-content"); - const innerContent = screen.getByTestId("dialog-inner-content"); expect(content).toHaveStyle({ padding: "var(--spacing000)" }); - expect(innerContent).toHaveStyle({ paddingTop: "0px" }); }); test("background scroll remains disabled when returning to outer dialog after closing inner dialog", async () => { diff --git a/src/components/form/form.component.tsx b/src/components/form/form.component.tsx index 6e1cb502e0..f57514dfbe 100644 --- a/src/components/form/form.component.tsx +++ b/src/components/form/form.component.tsx @@ -1,8 +1,6 @@ -import React, { useRef, useContext } from "react"; +import React, { useContext, useRef } from "react"; import { SpaceProps, PaddingProps } from "styled-system"; -import SidebarContext from "../sidebar/__internal__/sidebar.context"; -import ModalContext from "../modal/__internal__/modal.context"; import FormSummary from "./__internal__/form-summary.component"; import { StyledForm, @@ -13,6 +11,7 @@ import { } from "./form.style"; import { FormButtonAlignment, formSpacing } from "./form.config"; import FormSpacingProvider from "../../__internal__/form-spacing-provider"; +import ModalContext from "../modal/__internal__/modal.context"; export interface FormProps extends SpaceProps { /** Alignment of buttons */ @@ -62,11 +61,9 @@ export const Form = ({ footerPadding = {}, ...rest }: FormProps) => { - const { isInSidebar } = useContext(SidebarContext); - const { isInModal } = useContext(ModalContext); const formRef = useRef(null); const formFooterRef = useRef(null); - const hasPadding = !!Object.keys(footerPadding).length; + const { isInModal } = useContext(ModalContext); const renderFooter = !!( saveButton || @@ -76,19 +73,15 @@ export const Form = ({ warningCount ); - const classNames = `${stickyFooter ? "sticky" : ""} ${ - hasPadding ? "padded" : "" - }`.trimEnd(); - return ( {children} @@ -109,12 +101,10 @@ export const Form = ({ {leftSideButtons && ( diff --git a/src/components/form/form.pw.tsx b/src/components/form/form.pw.tsx index f551f48d9c..3909714e9c 100644 --- a/src/components/form/form.pw.tsx +++ b/src/components/form/form.pw.tsx @@ -279,7 +279,10 @@ test.describe("Accessibility tests for Form component", () => { const dialogButton = dataComponentButtonByText(page, "Open Preview"); await dialogButton.click(); - await checkAccessibility(page); + + // color-contrast ignored until we can investigate and fix FE-6245 + + await checkAccessibility(page, page.getByRole("dialog"), "color-contrast"); }); test(`should pass tests for InDialogWithStickyFooter example`, async ({ diff --git a/src/components/form/form.stories.tsx b/src/components/form/form.stories.tsx index d2e71b0c72..1d580cc6e7 100644 --- a/src/components/form/form.stories.tsx +++ b/src/components/form/form.stories.tsx @@ -78,6 +78,7 @@ export const DefaultWithStickyFooter: Story = () => ( ); DefaultWithStickyFooter.storyName = "Default with sticky footer"; DefaultWithStickyFooter.parameters = { + chromatic: { viewports: [1200, 320] }, themeProvider: { chromatic: { theme: "sage" } }, }; @@ -523,6 +524,10 @@ export const InDialogWithStickyFooter = () => { ); }; InDialogWithStickyFooter.storyName = "In Dialog with Sticky Footer"; +InDialogWithStickyFooter.parameters = { + chromatic: { viewports: [1200, 320] }, + themeProvider: { chromatic: { theme: "sage" } }, +}; export const InDialogFullScreen = () => { const [isOpen, setIsOpen] = useState(defaultOpenState); @@ -612,6 +617,10 @@ export const InDialogFullScreenWithStickyFooter = () => { }; InDialogFullScreenWithStickyFooter.storyName = "In Dialog Full Screen with Sticky Footer"; +InDialogFullScreenWithStickyFooter.parameters = { + chromatic: { viewports: [1200, 320] }, + themeProvider: { chromatic: { theme: "sage" } }, +}; export const FormAlignmentExample: Story = () => { const [date, setDate] = useState("04/04/2019"); diff --git a/src/components/form/form.style.ts b/src/components/form/form.style.ts index 359bb3d3e6..4276a8921d 100644 --- a/src/components/form/form.style.ts +++ b/src/components/form/form.style.ts @@ -13,23 +13,24 @@ interface StyledFormContentProps { isInModal?: boolean; } -export const StyledFormContent = styled.div` - ${({ stickyFooter, isInModal }) => css` - ${stickyFooter && +export const StyledFormContent = styled.div( + ({ stickyFooter, isInModal }) => + stickyFooter && + isInModal && css` - /* Take responsibility for handling overflow away from parent modal */ - overflow-y: ${isInModal ? "auto" : "inherit"}; - flex: 1; - `} - `} -`; + flex-grow: 1; + min-height: 0; + overflow-y: auto; + ` +); -interface ButtonProps extends StyledFormContentProps { - buttonAlignment?: FormButtonAlignment; +interface StyledFormFooterProps { + stickyFooter?: boolean; fullWidthButtons?: boolean; + buttonAlignment?: FormButtonAlignment; } -export const StyledFormFooter = styled.div` +export const StyledFormFooter = styled.div` align-items: center; display: flex; flex-wrap: wrap; @@ -41,7 +42,7 @@ export const StyledFormFooter = styled.div` justify-content: flex-end; `} - ${({ stickyFooter, fullWidthButtons, isInModal }) => css` + ${({ stickyFooter, fullWidthButtons }) => css` ${!stickyFooter && css` margin-top: 48px; @@ -55,10 +56,7 @@ export const StyledFormFooter = styled.div` padding: 16px 32px; width: 100%; z-index: 1000; - ${!isInModal && - css` - position: sticky; - `} + position: sticky; bottom: 0; `} @@ -76,13 +74,9 @@ StyledFormFooter.defaultProps = { theme: baseTheme, }; -// Accounts for height of the header of Modal parent, the height form footer and some additional spacing -const HEIGHT_SPACING = 216; - interface StyledFormProps extends StyledFormContentProps { height?: string; fieldSpacing: 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7; - isInSidebar?: boolean; } export const StyledForm = styled.form` @@ -99,7 +93,7 @@ export const StyledForm = styled.form` margin-bottom: var(--spacing000); } - ${({ stickyFooter, isInModal, isInSidebar }) => + ${({ stickyFooter, isInModal }) => stickyFooter && css` display: flex; @@ -108,24 +102,25 @@ export const StyledForm = styled.form` ${isInModal && css` - max-height: calc(100vh - ${HEIGHT_SPACING}px); - `} - - ${isInSidebar && - css` - min-height: 100%; + flex-grow: 1; + min-height: 0; + height: 100%; `} `} `; -export const StyledRightButtons = styled.div` +export const StyledRightButtons = styled.div<{ + buttonAlignment?: FormButtonAlignment; +}>` display: flex; gap: var(--sizing200); ${({ buttonAlignment }) => buttonAlignment === "left" && "flex-grow: 1;"} `; -export const StyledLeftButtons = styled.div` +export const StyledLeftButtons = styled.div<{ + buttonAlignment?: FormButtonAlignment; +}>` display: flex; justify-content: flex-end; gap: var(--sizing200); diff --git a/src/components/form/form.test.tsx b/src/components/form/form.test.tsx index 8a11f0c47a..3eb5cddbc4 100644 --- a/src/components/form/form.test.tsx +++ b/src/components/form/form.test.tsx @@ -186,30 +186,27 @@ test("has the correct styles when the `stickyFooter` prop is set", () => { "background-color", "var(--colorsUtilityYang100)" ); - expect(screen.getByTestId("form-content")).toHaveStyle({ - "overflow-y": "inherit", - flex: "1", - }); expect(screen.getByRole("form")).toHaveStyle({ display: "flex", "flex-direction": "column", - position: "relative", }); + expect(screen.getByRole("form")).not.toHaveStyle("overflow-y: auto"); }); -// for coverage: Form inside dialog is covered by Chromatic -test("has the correct overflow styling when a sticky footer is applied inside a modal", () => { +// for coverage: stickyFooter prop styles are covered by Chromatic and Playwright +test("applies overflow styling when `stickyFooter` is set and form is in a Dialog", () => { render( - + Save} + /> ); expect(screen.getByTestId("form-content")).toHaveStyle({ - "overflow-y": "auto", - }); - expect(screen.getByRole("form")).toHaveStyle({ - "max-height": "calc(100vh - 216px)", + overflowY: "auto", }); }); diff --git a/src/components/select/filterable-select/filterable-select.pw.tsx b/src/components/select/filterable-select/filterable-select.pw.tsx index 70de33d394..a25ff96531 100644 --- a/src/components/select/filterable-select/filterable-select.pw.tsx +++ b/src/components/select/filterable-select/filterable-select.pw.tsx @@ -31,7 +31,6 @@ import { boldedAndUnderlinedValue, dropdownButton, filterableSelectAddElementButton, - filterableSelectAddNewButton, filterableSelectButtonIcon, multiColumnsSelectListBody, multiColumnsSelectListHeader, @@ -1080,19 +1079,26 @@ test.describe("FilterableSelect component", () => { mount, page, }) => { + const newOption = "New10"; await mount(); - const newOption = "New10"; - await dropdownButton(page).click(); - await expect(selectListWrapper(page)).toBeVisible(); - const addElementButtonElement = filterableSelectAddElementButton(page); - await expect(addElementButtonElement).toBeVisible(); + // open select list + const input = page.getByRole("combobox"); + await input.click(); + await expect(page.getByRole("listbox")).toBeVisible(); + + const addElementButtonElement = page.getByRole("button", { + name: "Add a New Element", + }); await addElementButtonElement.click(); - await expect(alertDialogPreview(page)).toBeVisible(); - const addNewButtonElement = filterableSelectAddNewButton(page); - await expect(addNewButtonElement).toBeVisible(); - await addNewButtonElement.click(); - await expect(getDataElementByValue(page, "input")).toHaveValue(newOption); + + const alert = page.getByRole("dialog"); + await expect(alert).toBeVisible(); + + const alertAddNewButton = page.getByRole("button", { name: "Add new" }); + await alertAddNewButton.click(); + + await expect(input).toHaveValue(newOption); }); test("should have correct hover state of list option", async ({ diff --git a/src/components/sidebar/sidebar-test.stories.tsx b/src/components/sidebar/sidebar-test.stories.tsx index 38f4c79ea3..070fc0ec94 100644 --- a/src/components/sidebar/sidebar-test.stories.tsx +++ b/src/components/sidebar/sidebar-test.stories.tsx @@ -1,20 +1,27 @@ import React, { useState } from "react"; import { action } from "@storybook/addon-actions"; +import { Meta, StoryObj } from "@storybook/react"; +import Form from "../form"; +import Textbox from "../textbox"; import Button from "../button"; import Sidebar, { SidebarProps } from "."; import { SIDEBAR_ALIGNMENTS, SIDEBAR_SIZES } from "./sidebar.config"; import Box from "../box"; +import Typography from "../typography"; -export default { +const meta: Meta = { component: Sidebar, - includeStories: ["Default"], title: "Sidebar/Test", parameters: { - info: { disable: true }, - chromatic: { - disableSnapshot: true, - }, + themeProvider: { chromatic: { theme: "none" } }, }, + decorators: [ + (Story) => ( +
+ +
+ ), + ], argTypes: { open: { control: { disable: true } }, "aria-label": { table: { disable: true }, control: { disable: true } }, @@ -60,9 +67,26 @@ export default { type: "text", }, }, + padding: { + control: { + type: "text", + }, + }, + paddingX: { + control: { + type: "text", + }, + }, + paddingRight: { + control: { + type: "text", + }, + }, }, }; +export default meta; + export const Default = (args: Partial) => { const [isOpen, setIsOpen] = useState(true); const onCancel = () => { @@ -93,3 +117,61 @@ Default.args = { enableBackgroundUI: false, disableEscKey: false, }; +Default.parameters = { + chromatic: { + disableSnapshot: true, + }, +}; + +export const WithStickyForm: StoryObj = { + render: (args) => ( + With sticky form} + open + onCancel={() => {}} + > + Cancel} + saveButton={} + stickyFooter + onSubmit={(ev) => ev.preventDefault()} + > + + + + + + + + + + ), +}; + +export const WithForm: StoryObj = { + render: (args) => ( + With form} + open + onCancel={() => {}} + > +
Cancel} + saveButton={} + onSubmit={(ev) => ev.preventDefault()} + > + + + + + + + + +
+ ), +}; diff --git a/src/components/sidebar/sidebar.component.tsx b/src/components/sidebar/sidebar.component.tsx index 1ab02f50e0..655c7397d0 100644 --- a/src/components/sidebar/sidebar.component.tsx +++ b/src/components/sidebar/sidebar.component.tsx @@ -1,14 +1,12 @@ -import React, { useCallback, useRef, useMemo } from "react"; +import React, { useCallback, useRef } from "react"; import { PaddingProps, WidthProps } from "styled-system"; import Modal, { ModalProps } from "../modal"; -import StyledSidebar from "./sidebar.style"; +import { StyledSidebar, StyledSidebarContent } from "./sidebar.style"; import IconButton from "../icon-button"; import Icon from "../icon"; -import { FormProps } from "../form"; import FocusTrap from "../../__internal__/focus-trap"; import SidebarHeader from "./__internal__/sidebar-header"; -import Box from "../box"; import createGuid from "../../__internal__/utils/helpers/guid"; import useLocale from "../../hooks/__internal__/useLocale"; import { filterStyledSystemPaddingProps } from "../../style/utils"; @@ -119,14 +117,6 @@ export const Sidebar = React.forwardRef( ) => { const locale = useLocale(); const { current: headerId } = useRef(createGuid()); - const hasStickyFooter = useMemo( - () => - React.Children.toArray(children).some( - (child) => - React.isValidElement(child) && child.props.stickyFooter - ), - [children] - ); const sidebarRef = useRef(null); @@ -171,7 +161,6 @@ export const Sidebar = React.forwardRef( size={size} onCancel={onCancel} role={role} - {...filterStyledSystemPaddingProps(rest)} width={width} > {header && ( @@ -184,21 +173,15 @@ export const Sidebar = React.forwardRef( )} {!header && closeIcon()} - {children} - + ); diff --git a/src/components/sidebar/sidebar.config.ts b/src/components/sidebar/sidebar.config.ts index c9495d602f..6d49daa9fd 100644 --- a/src/components/sidebar/sidebar.config.ts +++ b/src/components/sidebar/sidebar.config.ts @@ -19,8 +19,3 @@ export const SIDEBAR_SIZES = [ ]; export const SIDEBAR_ALIGNMENTS = ["left", "right"]; - -export const SIDEBAR_TOP_SPACING = "27px"; -export const SIDEBAR_BOTTOM_SPACING = "var(--spacing400)"; -export const SIDEBAR_LEFT_PADDING = "var(--spacing400)"; -export const SIDEBAR_RIGHT_PADDING = "var(--spacing400)"; diff --git a/src/components/sidebar/sidebar.style.ts b/src/components/sidebar/sidebar.style.ts index b70764d9d7..8cbcd25212 100644 --- a/src/components/sidebar/sidebar.style.ts +++ b/src/components/sidebar/sidebar.style.ts @@ -1,22 +1,18 @@ import styled, { css } from "styled-components"; -import { PaddingProps } from "styled-system"; +import { PaddingProps, padding as paddingFn } from "styled-system"; import computeSizing from "../../style/utils/element-sizing"; import { SidebarProps } from "./sidebar.component"; import baseTheme from "../../style/themes/base"; import StyledIconButton from "../icon-button/icon-button.style"; -import { - calculateFormSpacingValues, - calculateWidthValue, -} from "../../style/utils/form-style-utils"; -import { StyledFormContent, StyledFormFooter } from "../form/form.style"; + import { SIDEBAR_SIZES_CSS } from "./sidebar.config"; +import { StyledForm, StyledFormContent } from "../form/form.style"; type StyledSidebarProps = Pick< SidebarProps, "onCancel" | "position" | "size" | "width" -> & - PaddingProps; +>; const StyledSidebar = styled.div` // prevents outline being added in safari @@ -24,17 +20,6 @@ const StyledSidebar = styled.div` outline: none; } - ${StyledFormContent} { - ${(props: StyledSidebarProps) => - calculateFormSpacingValues(props, true, "sidebar")} - } - - ${StyledFormFooter}.sticky { - ${calculateWidthValue} - ${(props: StyledSidebarProps) => - calculateFormSpacingValues(props, false, "sidebar")} - } - ${({ onCancel, position, size, theme, width }) => css` background: var(--colorsUtilityMajor025); border-radius: 1px; @@ -70,8 +55,32 @@ const StyledSidebar = styled.div` `} `; +const StyledSidebarContent = styled.div` + box-sizing: border-box; + display: block; + overflow-y: auto; + flex-grow: 1; + + padding: var(--spacing300) var(--spacing400) var(--spacing400); + ${paddingFn} + + &:has(${StyledForm}.sticky) { + display: flex; + flex-direction: column; + overflow-y: hidden; + padding: 0; + + ${StyledForm}.sticky { + ${StyledFormContent} { + padding: var(--spacing300) var(--spacing400) var(--spacing400); + ${paddingFn} + } + } + } +`; + StyledSidebar.defaultProps = { theme: baseTheme, }; -export default StyledSidebar; +export { StyledSidebar, StyledSidebarContent }; diff --git a/src/components/sidebar/sidebar.test.tsx b/src/components/sidebar/sidebar.test.tsx index a5feb94efa..a47f176005 100644 --- a/src/components/sidebar/sidebar.test.tsx +++ b/src/components/sidebar/sidebar.test.tsx @@ -6,12 +6,12 @@ import { waitForElementToBeRemoved, } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import { sageTheme } from "../../style/themes"; import { testStyledSystemPaddingRTL, testStyledSystemWidthRTL, } from "../../__spec_helper__/__internal__/test-utils"; import CarbonProvider from "../carbon-provider"; -import Form from "../form"; import Sidebar, { SidebarProps } from "."; @@ -288,44 +288,32 @@ test("ensures overflowing content is scrollable", () => { render(); const sidebarContent = screen.getByTestId("sidebar-content"); - expect(sidebarContent).toHaveStyle("overflow: auto"); -}); - -test("does not control styling of overflowing content when there is a child Form with a sticky footer", () => { - render( - -
- - ); - - const sidebarContent = screen.getByTestId("sidebar-content"); - expect(sidebarContent).not.toHaveStyle("overflow: auto"); + expect(sidebarContent).toHaveStyle("overflow-y: auto"); }); testStyledSystemWidthRTL( (props) => ( - - Content - + + + Content + + ), () => screen.getByRole("dialog") ); testStyledSystemPaddingRTL( (props) => ( - - Content - + + + Content + + ), () => { const sidebars = screen.getAllByTestId("sidebar-content"); // the use of Portal means there is two instances of the sidebar content return sidebars[sidebars.length - 1]; - }, - { - pt: "var(--spacing300)", - pb: "var(--spacing400)", - px: "var(--spacing400)", } ); diff --git a/src/components/textarea/components.test-pw.tsx b/src/components/textarea/components.test-pw.tsx index 08df245d32..ee6b625072 100644 --- a/src/components/textarea/components.test-pw.tsx +++ b/src/components/textarea/components.test-pw.tsx @@ -1,9 +1,6 @@ import React, { useState } from "react"; import CarbonProvider from "../carbon-provider"; import Textarea, { TextareaProps } from "."; -import Dialog from "../dialog"; -import Form from "../form"; -import Button from "../button"; import Box from "../box"; interface TextareaTestProps extends TextareaProps { @@ -51,38 +48,24 @@ export const TextareaComponent = (props: Partial) => { }; export const InScrollableContainer = () => { - const [isOpen, setIsOpen] = useState(true); const [value, setValue] = useState( - "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Faucibus in ornare quam viverra orci sagittis eu. Pellentesque nec nam aliquam sem et tortor consequat. Nibh sit amet commodo nulla. Cursus metus aliquam eleifend mi. Mi proin sed libero enim sed faucibus turpis in. Ullamcorper velit sed ullamcorper morbi tincidunt ornare massa eget. Est lorem ipsum dolor sit amet consectetur. Morbi enim nunc faucibus a pellentesque sit. Ultrices neque ornare aenean euismod elementum nisi quis eleifend quam. Dapibus ultrices in iaculis nunc sed augue lacus viverra. Feugiat vivamus at augue eget arcu dictum varius. Eget velit aliquet sagittis id consectetur purus ut faucibus. Tincidunt arcu non sodales neque sodales. Ipsum faucibus vitae aliquet nec ullamcorper sit. Faucibus a pellentesque sit amet. Amet porttitor eget dolor morbi non. Arcu non odio euismod lacinia at quis risus sed vulputate. Blandit volutpat maecenas volutpat blandit. Purus ut faucibus pulvinar elementum integer enim neque. Viverra mauris in aliquam sem fringilla ut morbi. Amet mattis vulputate enim nulla aliquet porttitor lacus luctus accumsan. Nibh mauris cursus mattis molestie a. Turpis nunc eget lorem dolor sed viverra ipsum nunc aliquet. Facilisis mauris sit amet massa vitae tortor condimentum lacinia. Consequat mauris nunc congue nisi vitae. Nisl nunc mi ipsum faucibus vitae aliquet nec ullamcorper. Eu facilisis sed odio morbi quis commodo. Ultrices vitae auctor eu augue ut lectus arcu. Ut tellus elementum sagittis vitae et leo duis ut. Sapien eget mi proin sed libero. Dictum non consectetur a erat nam at. Suspendisse interdum consectetur libero id faucibus nisl tincidunt eget nullam. Pretium fusce id velit ut tortor pretium. Donec pretium vulputate sapien nec sagittis aliquam malesuada. Semper quis lectus nulla at volutpat diam.Velit dignissim sodales ut eu sem integer. In massa tempor nec feugiat nisl pretium fusce id. Eu scelerisque felis imperdiet proin fermentum. Amet purus gravida quis blandit. Feugiat in fermentum posuere urna nec tincidunt praesent. Sit amet mauris commodo quis. Lorem sed risus ultricies tristique nulla aliquet enim tortor. Rhoncus aenean vel elit scelerisque mauris pellentesque pulvinar pellentesque. Pellentesque pulvinar pellentesque habitant morbi tristique senectus. Nibh sit amet commodo nulla facilisi nullam vehicula ipsum. Pellentesque elit ullamcorper dignissim cras tincidunt lobortis feugiat. Velit laoreet id donec ultrices tincidunt arcu non sodales neque. A scelerisque purus semper eget duis. Ut faucibus pulvinar elementum integer enim neque. Integer feugiat scelerisque varius morbi enim nunc faucibus a. Amet nulla facilisi morbi tempus iaculis urna id volutpat lacus. Egestas purus viverra accumsan in nisl nisi. Sed turpis tincidunt id aliquet risus feugiat in ante. In mollis nunc sed id semper risus in hendrerit gravida. Faucibus a pellentesque sit amet porttitor eget dolor morbi. Ornare arcu dui vivamus arcu felis bibendum ut. Tempor commodo ullamcorper a lacus vestibulum sed arcu non odio. Lacinia quis vel eros donec ac odio. Amet volutpat consequat mauris nunc congue nisi vitae. Ultrices dui sapien eget mi proin sed. Adipiscing bibendum est ultricies integer quis auctor elit. Sagittis nisl rhoncus mattis rhoncus urna neque. Integer enim neque volutpat ac tincidunt. Curabitur gravida arcu ac tortor dignissim convallis aenean et tortor." + "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Faucibus in ornare quam viverra orci sagittis eu. Pellentesque nec nam aliquam sem et tortor consequat. Nibh sit amet commodo nulla. Cursus metus aliquam eleifend mi. Mi proin sed libero enim sed faucibus turpis in. Ullamcorper velit sed ullamcorper morbi tincidunt ornare massa eget. Est lorem ipsum dolor sit amet consectetur. Morbi enim nunc faucibus a pellentesque sit. Ultrices neque ornare aenean euismod elementum nisi quis eleifend quam. Dapibus ultrices in iaculis nunc sed augue lacus viverra. Feugiat vivamus at augue eget arcu dictum varius. Eget velit aliquet sagittis id consectetur purus ut faucibus. Tincidunt arcu non sodales neque sodales. Ipsum faucibus vitae aliquet nec ullamcorper sit. Faucibus a pellentesque sit amet. Amet porttitor eget dolor morbi non. Arcu non odio euismod lacinia at quis risus sed vulputate. Blandit volutpat maecenas volutpat blandit. Purus ut faucibus pulvinar elementum integer enim neque. Viverra mauris in aliquam sem fringilla ut morbi." ); return ( - setIsOpen(false)} - title="Title" - subtitle="Subtitle" - size="small" + - setIsOpen(false)}>Cancel - } - saveButton={ - - } - > -