Merge pull request #6742 from Sage/FE-6570/form-scrollbar-bug

fix(dialog-full-screen): prevent horizontal overflow due to a child Form with a sticky footer
Sage · Jun 5, 2024 · 9a38bbe · 9a38bbe
2 parents 3c160fb + efd0884
commit 9a38bbe
Show file tree

Hide file tree

Showing 10 changed files with 153 additions and 207 deletions.
diff --git a/src/components/dialog-full-screen/content.style.ts b/src/components/dialog-full-screen/content.style.ts
@@ -4,70 +4,17 @@ import { StyledFormFooter, StyledFormContent } from "../form/form.style";
 
 type StyledContentProps = {
   hasHeader: boolean;
-  hasStickyFooter: boolean;
   disableContentPadding?: boolean;
 };
 
 const StyledContent = styled.div<StyledContentProps>`
-  ${({ hasStickyFooter }) =>
-    !hasStickyFooter &&
-    css`
-      overflow-y: auto;
-    `}
+  overflow-y: auto;
   padding: 0 16px;
   flex: 1;
 
-  ${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);
-    }
+  /* Delegate handling overflow to child form if it has a sticky footer */
+  &:has(${StyledFormContent}.sticky) {
+    overflow-y: inherit;
   }
 
   ${({ disableContentPadding }) => css`
@@ -82,6 +29,59 @@ const StyledContent = styled.div<StyledContentProps>`
       @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);
+        }
+      }
     `}
 
     ${disableContentPadding &&

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
@@ -10,81 +10,85 @@ import Textbox from "../textbox";
 
 export default {
   title: "Dialog Full Screen/Test",
-  includeStories: ["DefaultStory", "WithTwoDifferentNodes"],
-  parameters: {
-    info: { disable: true },
-    chromatic: {
-      disableSnapshot: true,
-    },
-  },
+  component: DialogFullScreen,
+  parameters: { themeProvider: { chromatic: { theme: "sage" } } },
 };
 
-interface DefaultProps extends Partial<DialogFullScreenProps> {
-  stickyFooter?: boolean;
-  formHeight?: number;
-}
-
-export const DefaultStory = ({
-  stickyFooter,
-  formHeight,
-  children,
-  title,
-  subtitle,
-  ...args
-}: DefaultProps) => {
-  const [isOpen, setIsOpen] = useState(true);
-
-  const handleCancel = () => {
-    setIsOpen(false);
-    action("cancel")();
-  };
+type StoryType = StoryObj<typeof DialogFullScreen>;
 
-  const handleOpen = () => {
-    setIsOpen(true);
-    action("open")();
-  };
+export const Default: StoryType = {
+  render: (args) => {
+    const { children, ...rest } = args;
+    return <DialogFullScreen {...rest}>{children}</DialogFullScreen>;
+  },
+  args: {
+    children: "Content",
+    open: true,
+    title: "Example Dialog",
+    subtitle: "Example Subtitle",
+    showCloseIcon: true,
+    onCancel: () => {},
+  },
+  decorators: [
+    (Story) => (
+      <div style={{ height: 900, width: "100%" }}>
+        <Story />
+      </div>
+    ),
+  ],
+};
 
-  return (
-    <>
-      <Button onClick={handleOpen}>Open Dialog</Button>
-      <DialogFullScreen
-        onCancel={handleCancel}
-        open={isOpen}
-        title={title}
-        subtitle={subtitle}
-        {...args}
+export const WithStickyForm: StoryType = {
+  render: (args) => {
+    const DialogForm = () => (
+      <Form
+        stickyFooter
+        leftSideButtons={<Button onClick={() => {}}>Cancel</Button>}
+        saveButton={
+          <Button buttonType="primary" type="submit">
+            Save
+          </Button>
+        }
+        onSubmit={(ev) => {
+          ev.preventDefault();
+        }}
       >
-        <Form
-          stickyFooter={stickyFooter}
-          leftSideButtons={<Button onClick={handleCancel}>Cancel</Button>}
-          saveButton={
-            <Button buttonType="primary" type="submit">
-              Save
-            </Button>
-          }
-        >
-          {children || ""}
-          <div style={{ height: formHeight }} />
-        </Form>
+        <Textbox label="Textbox" onChange={() => {}} />
+        <Textbox label="Textbox" onChange={() => {}} />
+        <Textbox label="Textbox" onChange={() => {}} />
+        <Textbox label="Textbox" onChange={() => {}} />
+        <Textbox label="Textbox" onChange={() => {}} />
+        <Textbox label="Textbox" onChange={() => {}} />
+        <Textbox label="Textbox" onChange={() => {}} />
+        <Textbox label="Textbox" onChange={() => {}} />
+        <Textbox label="Textbox" onChange={() => {}} />
+        <Textbox label="Textbox" onChange={() => {}} />
+      </Form>
+    );
+
+    return (
+      <DialogFullScreen {...args}>
+        <DialogForm />
       </DialogFullScreen>
-    </>
-  );
-};
-
-DefaultStory.storyName = "default";
-DefaultStory.args = {
-  title: "Example Dialog",
-  subtitle: "Example Subtitle",
-  children: "Text Content",
-  disableEscKey: false,
-  showCloseIcon: true,
-  formHeight: "2000px",
-  stickyFooter: false,
-  disableContentPadding: false,
-  closeButtonDataProps: {},
+    );
+  },
+  args: {
+    open: true,
+    title: "Example Dialog",
+    subtitle: "I have a sticky form!",
+    showCloseIcon: true,
+    onCancel: () => {},
+  },
+  decorators: [
+    (Story) => (
+      <div style={{ height: 900, width: "100%" }}>
+        <Story />
+      </div>
+    ),
+  ],
 };
 
-export const Nested = () => {
+export const Nested: StoryType = () => {
   const [mainDialogOpen, setMainDialogOpen] = useState(false);
 
   const [nestedDialogOpen, setNestedDialogOpen] = useState(false);
@@ -131,8 +135,11 @@ export const Nested = () => {
 };
 
 Nested.storyName = "nested";
-
-type StoryType = StoryObj<typeof DialogFullScreen>;
+Nested.parameters = {
+  chromatic: {
+    disableSnapshot: true,
+  },
+};
 
 export const WithTwoDifferentNodes: StoryType = ({
   ...props
@@ -174,5 +181,4 @@ WithTwoDifferentNodes.decorators = [
 ];
 WithTwoDifferentNodes.parameters = {
   layout: "fullscreen",
-  chromatic: { disableSnapshot: false },
 };
diff --git a/src/components/dialog-full-screen/dialog-full-screen.component.tsx b/src/components/dialog-full-screen/dialog-full-screen.component.tsx
@@ -10,7 +10,6 @@ import FocusTrap, { CustomRefObject } from "../../__internal__/focus-trap";
 import IconButton from "../icon-button";
 import Icon from "../icon";
 import useLocale from "../../hooks/__internal__/useLocale";
-import useIsStickyFooterForm from "../../hooks/__internal__/useIsStickyFooterForm";
 import useModalAria from "../../hooks/__internal__/useModalAria/useModalAria";
 import tagComponent, { TagProps } from "../../__internal__/utils/helpers/tags";
 
@@ -106,7 +105,6 @@ export const DialogFullScreen = ({
   const headingRef = useRef(null);
   const { current: titleId } = useRef(createGuid());
   const { current: subtitleId } = useRef(createGuid());
-  const hasStickyFooter = useIsStickyFooterForm(children);
 
   const isTopModal = useModalAria(dialogRef);
 
@@ -192,7 +190,6 @@ export const DialogFullScreen = ({
             data-element="content"
             ref={contentRef}
             disableContentPadding={disableContentPadding}
-            hasStickyFooter={hasStickyFooter}
           >
             {children}
           </StyledContent>

diff --git a/src/components/dialog/dialog.component.tsx b/src/components/dialog/dialog.component.tsx
@@ -25,7 +25,6 @@ import FocusTrap, { CustomRefObject } from "../../__internal__/focus-trap";
 import IconButton from "../icon-button";
 import Icon from "../icon";
 import useLocale from "../../hooks/__internal__/useLocale";
-import useIsStickyFooterForm from "../../hooks/__internal__/useIsStickyFooterForm";
 import useModalAria from "../../hooks/__internal__/useModalAria/useModalAria";
 
 const PADDING_VALUES = [0, 1, 2, 3, 4, 5, 6, 7, 8] as const;
@@ -141,7 +140,6 @@ export const Dialog = forwardRef<DialogHandle, DialogProps>(
     const listenersAdded = useRef(false);
     const { current: titleId } = useRef(createGuid());
     const { current: subtitleId } = useRef(createGuid());
-    const hasStickyFooter = useIsStickyFooterForm(children);
 
     const isTopModal = useModalAria(containerRef);
 
@@ -327,10 +325,7 @@ export const Dialog = forwardRef<DialogHandle, DialogProps>(
           >
             {dialogTitle()}
             {closeIcon()}
-            <StyledDialogContent
-              {...contentPadding}
-              hasStickyFooter={hasStickyFooter}
-            >
+            <StyledDialogContent {...contentPadding}>
               <StyledDialogInnerContent
                 ref={innerContentRef}
                 {...contentPadding}

diff --git a/src/components/dialog/dialog.style.ts b/src/components/dialog/dialog.style.ts
@@ -145,17 +145,17 @@ const StyledDialogTitle = styled.div<StyledDialogTitleProps>`
   }
 `;
 
-const StyledDialogContent = styled.div<
-  ContentPaddingInterface & { hasStickyFooter: boolean }
->`
+const StyledDialogContent = styled.div<ContentPaddingInterface>`
   box-sizing: border-box;
   display: flex;
   flex-direction: column;
-  ${({ hasStickyFooter }) =>
-    !hasStickyFooter &&
-    css`
-      overflow-y: auto;
-    `}
+  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;

diff --git a/src/components/form/form.style.ts b/src/components/form/form.style.ts
@@ -18,6 +18,7 @@ export const StyledFormContent = styled.div<StyledFormContentProps>`
   ${({ stickyFooter, isInModal }) => css`
     ${stickyFooter &&
     css`
+      /* Take responsibility for handling overflow away from parent modal */
       overflow-y: ${isInModal ? "auto" : "inherit"};
       flex: 1;
     `}