Merge pull request #6990 from Sage/FE-6816/dialog-overflow

fix: resolve layout issues with sticky footer forms inside Carbon modal components (FE-6816)
Sage · Oct 23, 2024 · e88c3dd · e88c3dd
2 parents cffef0d + 0fe249d
commit e88c3dd
Show file tree

Hide file tree

Showing 40 changed files with 817 additions and 1,570 deletions.
diff --git 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
@@ -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/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.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(
-        <AdvancedColorPickerCustom
-          onBlur={() => {
-            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
@@ -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 ({