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.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..87a4924542 100644 --- a/src/components/dialog-full-screen/content.style.ts +++ b/src/components/dialog-full-screen/content.style.ts @@ -1,92 +1,62 @@ 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; }; +const generatePaddingVariables = (px: number) => css` + padding-top: 0; + padding-left: ${px}px; + padding-right: ${px}px; + padding-bottom: 0; +`; + +const stickyFormOverrides = (px: number) => css` + ${StyledForm}.sticky { + margin-left: calc(-1 * ${px}px); + margin-right: calc(-1 * ${px}px); + + ${StyledFormContent} { + ${generatePaddingVariables(px)}; + } + } +`; + const StyledContent = styled.div` + box-sizing: border-box; + display: flex; + flex-direction: column; overflow-y: auto; - padding: 0 16px; + flex: 1; + width: 100%; - /* Delegate handling overflow to child form if it has a sticky footer */ - &:has(${StyledFormContent}.sticky) { - overflow-y: inherit; - } + ${generatePaddingVariables(16)} + ${stickyFormOverrides(16)} ${({ disableContentPadding }) => css` ${!disableContentPadding && css` @media screen and (min-width: 600px) { - padding: 0 24px; + ${generatePaddingVariables(24)} + ${stickyFormOverrides(24)} } @media screen and (min-width: 960px) { - padding: 0 32px; + ${generatePaddingVariables(32)} + ${stickyFormOverrides(32)} } @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; - } - } - - ${StyledFormFooter}.sticky { - padding: 16px; - - margin-right: -16px; - margin-left: -16px; - width: calc(100% + 32px); - - @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); - } + ${generatePaddingVariables(40)} + ${stickyFormOverrides(40)} } `} ${disableContentPadding && css` - padding: 0; + ${generatePaddingVariables(0)} + ${stickyFormOverrides(0)} `} `} 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/dialog-test.stories.tsx b/src/components/dialog/dialog-test.stories.tsx index 57c478132a..1c7ef73d3e 100644 --- a/src/components/dialog/dialog-test.stories.tsx +++ b/src/components/dialog/dialog-test.stories.tsx @@ -15,6 +15,15 @@ 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"; + export default { title: "Dialog/Test", component: Dialog, @@ -293,3 +302,161 @@ MaxSizeTestNonOverflowedForm.parameters = { chromatic: { disableSnapshot: false, viewports: [1200, 900] }, layout: "fullscreen", }; + +export const DialogWithLongHeaderContent: StoryType = { + 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 + + + + + + + ), +}; diff --git a/src/components/dialog/dialog.component.tsx b/src/components/dialog/dialog.component.tsx index 8d727fcb1f..444cc1b719 100644 --- a/src/components/dialog/dialog.component.tsx +++ b/src/components/dialog/dialog.component.tsx @@ -18,7 +18,6 @@ import { StyledDialog, StyledDialogTitle, StyledDialogContent, - StyledDialogInnerContent, } from "./dialog.style"; import { DialogSizes, TOP_MARGIN } from "./dialog.config"; @@ -195,7 +194,7 @@ export const Dialog = forwardRef( containerRef.current.style.top = `${midPointY}px`; containerRef.current.style.left = `${midPointX}px`; - }, [size]); + }, [isDialogMaximised]); useResizeObserver(innerContentRef, centerDialog, !open); @@ -341,14 +340,9 @@ export const Dialog = forwardRef( {...contentPadding} data-role="dialog-content" tabIndex={-1} + ref={innerContentRef} > - - {children} - + {children} diff --git a/src/components/dialog/dialog.stories.tsx b/src/components/dialog/dialog.stories.tsx index 69b0463950..be7a444bc1 100644 --- a/src/components/dialog/dialog.stories.tsx +++ b/src/components/dialog/dialog.stories.tsx @@ -20,13 +20,12 @@ 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: { @@ -37,8 +36,8 @@ const meta: Meta = { decorators: [ (Story) => ( <> - {defaultOpenState ? ( - + {isChromatic() ? ( + ) : ( @@ -52,6 +51,8 @@ const meta: Meta = { export default meta; type Story = StoryObj; +const defaultOpenState = isChromatic(); + export const DefaultStory: Story = () => { const [isOpen, setIsOpen] = useState(defaultOpenState); return ( diff --git a/src/components/dialog/dialog.style.ts b/src/components/dialog/dialog.style.ts index ea646ca7e9..e3a0090596 100644 --- a/src/components/dialog/dialog.style.ts +++ b/src/components/dialog/dialog.style.ts @@ -1,16 +1,5 @@ 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, @@ -24,6 +13,12 @@ import { DialogSizes, } from "./dialog.config"; import { ContentPaddingInterface } from "./dialog.component"; +import resolvePaddingSides from "../../style/utils/resolve-padding-sides"; +import { + StyledFormContent, + StyledForm, + StyledFormFooter, +} from "../form/form.style"; const dialogSizes = { auto: "fit-content", @@ -36,17 +31,6 @@ 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; @@ -54,18 +38,12 @@ type StyledDialogProps = { 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 StyledDialog = styled.div.attrs(({ size }: StyledDialogProps) => { + const isDialogMaximised = size === "maximise"; + return { + isDialogMaximised, + }; +})` box-shadow: var(--boxShadow300); display: flex; flex-direction: column; @@ -74,6 +52,7 @@ const StyledDialog = styled.div.attrs( top: 50%; z-index: ${({ theme }) => theme.zIndex.modal}; max-height: ${({ topMargin }) => `calc(100vh - ${topMargin}px)`}; + ${({ isDialogMaximised }) => isDialogMaximised && "height: 100%"}; &:focus { @@ -99,32 +78,7 @@ const StyledDialog = styled.div.attrs( 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; @@ -168,28 +122,44 @@ const StyledDialogTitle = styled.div` } `; -const StyledDialogContent = styled.div` - box-sizing: border-box; - display: flex; - flex-direction: column; - overflow-y: auto; +const StyledDialogContent = styled.div((props) => { + const { + paddingTop = `${CONTENT_TOP_PADDING}px`, + paddingRight = `${HORIZONTAL_PADDING}px`, + paddingBottom = `${CONTENT_BOTTOM_PADDING}px`, + paddingLeft = `${HORIZONTAL_PADDING}px`, + } = resolvePaddingSides(props); - /* Delegate handling overflow to child form if it has a sticky footer */ - &:has(${StyledFormContent}.sticky) { - overflow-y: inherit; - } + const negate = (value: string) => `calc(-1 * ${value})`; - width: 100%; - flex: 1; - padding: 0px ${HORIZONTAL_PADDING}px ${CONTENT_BOTTOM_PADDING}px; - ${paddingFn} -`; + return css` + box-sizing: border-box; + display: flex; + flex-direction: column; + overflow-y: auto; + + width: 100%; + flex: 1; + + ${StyledForm}.sticky { + margin-top: ${negate(paddingTop)}; + margin-right: ${negate(paddingRight)}; + margin-bottom: ${negate(paddingBottom)}; + margin-left: ${negate(paddingLeft)}; + + ${StyledFormContent} { + padding: ${paddingTop} ${paddingRight} ${paddingBottom} ${paddingLeft}; + } + + ${StyledFormFooter} { + border-bottom-right-radius: var(--borderRadius200); + border-bottom-left-radius: var(--borderRadius200); + } + } -const StyledDialogInnerContent = styled.div` - position: relative; - flex: 1; - padding-top: ${calculatePaddingTopInnerContent}; -`; + padding: ${paddingTop} ${paddingRight} ${paddingBottom} ${paddingLeft}; + `; +}); StyledDialog.defaultProps = { theme: baseTheme, @@ -199,9 +169,4 @@ StyledDialogContent.defaultProps = { theme: baseTheme, }; -export { - StyledDialog, - StyledDialogTitle, - StyledDialogContent, - StyledDialogInnerContent, -}; +export { StyledDialog, StyledDialogTitle, StyledDialogContent }; diff --git a/src/components/dialog/dialog.test.tsx b/src/components/dialog/dialog.test.tsx index c129fd587a..c55490e722 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()); @@ -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(); @@ -397,6 +383,15 @@ test("dialog does not position itself such that it goes off the left edge of the 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", () => { render( @@ -405,10 +400,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.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..1f24d554c1 100644 --- a/src/components/form/form.style.ts +++ b/src/components/form/form.style.ts @@ -15,21 +15,18 @@ interface StyledFormContentProps { export const StyledFormContent = styled.div` ${({ stickyFooter, isInModal }) => css` - ${stickyFooter && - css` - /* Take responsibility for handling overflow away from parent modal */ - overflow-y: ${isInModal ? "auto" : "inherit"}; - flex: 1; - `} + ${stickyFooter && `flex: 1`} + ${stickyFooter && isInModal && `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 +38,7 @@ export const StyledFormFooter = styled.div` justify-content: flex-end; `} - ${({ stickyFooter, fullWidthButtons, isInModal }) => css` + ${({ stickyFooter, fullWidthButtons }) => css` ${!stickyFooter && css` margin-top: 48px; @@ -55,10 +52,7 @@ export const StyledFormFooter = styled.div` padding: 16px 32px; width: 100%; z-index: 1000; - ${!isInModal && - css` - position: sticky; - `} + position: sticky; bottom: 0; `} @@ -76,13 +70,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 +89,9 @@ export const StyledForm = styled.form` margin-bottom: var(--spacing000); } - ${({ stickyFooter, isInModal, isInSidebar }) => + flex: 1; + + ${({ stickyFooter, isInModal }) => stickyFooter && css` display: flex; @@ -108,24 +100,23 @@ export const StyledForm = styled.form` ${isInModal && css` - max-height: calc(100vh - ${HEIGHT_SPACING}px); - `} - - ${isInSidebar && - css` - min-height: 100%; + overflow-y: auto; `} `} `; -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..85cb92e483 100644 --- a/src/components/form/form.test.tsx +++ b/src/components/form/form.test.tsx @@ -187,29 +187,33 @@ test("has the correct styles when the `stickyFooter` prop is set", () => { "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"); + expect(screen.getByTestId("form-content")).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.getByRole("form")).toHaveStyle({ overflowY: "auto" }); 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..08c47171f5 100644 --- a/src/components/sidebar/sidebar.style.ts +++ b/src/components/sidebar/sidebar.style.ts @@ -1,22 +1,19 @@ 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 resolvePaddingSides from "../../style/utils/resolve-padding-sides"; +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 +21,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 +56,43 @@ const StyledSidebar = styled.div` `} `; +const StyledSidebarContent = styled.div((props) => { + const { + paddingTop = "var(--spacing300)", + paddingRight = "var(--spacing400)", + paddingBottom = "var(--spacing400)", + paddingLeft = "var(--spacing400)", + } = resolvePaddingSides(props); + + return css` + box-sizing: border-box; + display: flex; + flex-direction: column; + overflow-y: auto; + + flex: 1; + + ${StyledForm}.sticky { + margin-top: calc(-1 * ${paddingTop}); + margin-right: calc(-1 * ${paddingRight}); + margin-bottom: calc(-1 * ${paddingBottom}); + margin-left: calc(-1 * ${paddingLeft}); + + ${StyledFormContent} { + padding-top: ${paddingTop}; + padding-right: ${paddingRight}; + padding-bottom: ${paddingBottom}; + padding-left: ${paddingLeft}; + } + } + + padding: ${paddingTop} ${paddingRight} ${paddingBottom} ${paddingLeft}; + ${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={ - - } - > -