fix: prevent sticky footer form content from overflowing in Carbon mo…

…dal components Fixes a layout issue, where content inside of a sticky footer `Form` would overflow outside of a parent Carbon modal component, such as `Dialog`, `DialogFullscreen` and `Sidebar`, at smaller screen sizes. Utilises CSS Flexbox more internally, to improve how each component adapts to changes in screen size. closes #6969
Sage · Oct 23, 2024 · cb77fb7 · cb77fb7
1 parent cffef0d
commit cb77fb7
Show file tree

Hide file tree

Showing 32 changed files with 651 additions and 1,025 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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(<AlertComponent title="title">{text}</AlertComponent>);
 
-      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
@@ -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(<ConfirmComponent height={heightstring} />);
+      await page.setViewportSize({ width: 600, height: 1000 });
+      await mount(<ConfirmComponent height={height} />);
 
-      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(<ConfirmComponent height="1200px" />);
 
-      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
@@ -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(
-        <Confirm open height="60px" onConfirm={() => {}}>
-          <DateInputCustom disablePortal={state} />
-        </Confirm>
-      );
+  test("date picker does not float above the rest of the page, when disablePortal prop is true", async ({
+    mount,
+    page,
+  }) => {
+    await mount(
+      <div
+        id="clipping-container"
+        style={{
+          position: "relative",
+          overflow: "hidden",
+          border: "1px solid black",
+        }}
+      >
+        <DateInputCustom disablePortal />
+      </div>
+    );
 
-      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(
+      <div
+        id="clipping-container"
+        style={{
+          position: "relative",
+          overflow: "hidden",
+          border: "1px solid black",
+        }}
+      >
+        <DateInputCustom disablePortal={false} />
+      </div>
+    );
+    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
@@ -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<StyledContentProps>`
+  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
@@ -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
@@ -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
     </DialogFullScreen>
   );
 
-  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(
-    <DialogFullScreen open>
-      <Form stickyFooter />
-    </DialogFullScreen>
-  );
-
-  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(<DialogFullScreen open title="Test" />);