diff --git a/src/components/dialog-full-screen/dialog-full-screen.component.tsx b/src/components/dialog-full-screen/dialog-full-screen.component.tsx index b04334a8f1..dfe2a73b67 100644 --- a/src/components/dialog-full-screen/dialog-full-screen.component.tsx +++ b/src/components/dialog-full-screen/dialog-full-screen.component.tsx @@ -185,7 +185,8 @@ export const DialogFullScreen = ({ pagesStyling={pagesStyling} role={role} > - {dialogTitle()} + {/* Conditionally render the dialog title */} + {title || headerChildren ? dialogTitle() : null} {closeIcon()} ` css` ${StyledContent} { padding: 0; - margin-top: -25px; } ${StyledIconButton} { @@ -58,8 +57,10 @@ const StyledDialogFullScreen = styled.div<{ pagesStyling?: boolean }>` z-index: 1; } + /* If the component has a title, we need to keep the same styles. If it does not have a title, + we need the padding to compensate for loss of 32px height */ ${StyledFullScreenHeading} { - padding: 32px 32px 0; + padding: 64px 32px 0; } ${StyledHeading} { 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 1383b17f23..e448bb96c6 100644 --- a/src/components/dialog-full-screen/dialog-full-screen.test.tsx +++ b/src/components/dialog-full-screen/dialog-full-screen.test.tsx @@ -238,6 +238,15 @@ test("renders the element given in the `headerChildren` prop", () => { ).toBeVisible(); }); +test("does not render anything if no `headerChildren` prop is provided", () => { + render(); + + const link = screen.queryByRole("link"); + + /*eslint-disable */ + expect(link).toBeNull(); +}); + // test here for coverage only - disableContentPadding prop already tested in both Playwright and Chromatic test("padding is removed from the content when the `disableContentPadding` prop is passed", () => { render( @@ -278,11 +287,6 @@ test("applies the appropriate styles when the `pagesStyling` prop is set", () => expect(dialog).toHaveStyleRule("z-index", "1", { modifier: `${StyledIconButton}`, }); - - expect(dialog).toHaveStyleRule("padding", "32px 32px 0", { - modifier: `${StyledFullScreenHeading}`, - }); - expect(dialog).toHaveStyleRule("width", "auto", { modifier: `${StyledHeading}`, }); @@ -299,6 +303,9 @@ test("applies the appropriate styles when the `pagesStyling` prop is set", () => expect(dialog).toHaveStyleRule("margin", "0 0 0 3px", { modifier: `${StyledHeading} ${StyledHeader}`, }); + expect(dialog).toHaveStyleRule("padding", "64px 32px 0", { + modifier: `${StyledFullScreenHeading}`, + }); }); /* TODO: (FE-6781) Update these tests to use toHaveStyle to avoid false positives: diff --git a/src/components/heading/heading.style.ts b/src/components/heading/heading.style.ts index 813e52a916..80ee1b1f4c 100644 --- a/src/components/heading/heading.style.ts +++ b/src/components/heading/heading.style.ts @@ -12,6 +12,7 @@ import addFocusStyling from "../../style/utils/add-focus-styling"; const StyledHeading = styled.div` width: 100%; + min-height: 53px; ${margin} `; @@ -154,10 +155,8 @@ const StyledDivider = styled(Hr)` const StyledHeaderHelp = styled(Help)` display: inline-block; - margin-left: -6px; margin-right: 16px; position: relative; - top: -4px; height: 22px; `; diff --git a/src/components/pages/page/page.component.tsx b/src/components/pages/page/page.component.tsx index 0e8c9c4289..f96df57bf2 100644 --- a/src/components/pages/page/page.component.tsx +++ b/src/components/pages/page/page.component.tsx @@ -9,7 +9,7 @@ import { filterStyledSystemPaddingProps } from "../../../style/utils"; export interface PageProps extends PaddingProps { /** The title for the page, normally a Heading component. */ - title: React.ReactNode; + title?: React.ReactNode; /** This component supports children. */ children: React.ReactNode; /** The ARIA role to be applied to the component */ @@ -38,7 +38,10 @@ const Page = ({ role, title, children, ...rest }: PageProps) => { ref={styledPageNodeRef} role={role} > - {title} + {/* Conditionally rendered the heading */} + {title ? ( + {title} + ) : null} { ); }; + +// Story that demonstrates the example of the Page component not having a title. +// This will need to be given a better name and we must ensure it is captured by Chromatic. +// You'll need to use the `isChromatic` helper that we use in Dialog for this to work. +export const TEST_TEST = ({ + initialPageIndex, +}: PageStoryProps & Partial) => { + const [isOpen, setIsOpen] = useState(false); + const [pageIndex, setPageIndex] = useState(Number(initialPageIndex) || 0); + const [isDisabled, setIsDisabled] = useState(false); + const handleCancel = ( + ev: React.KeyboardEvent | React.MouseEvent, + ) => { + setIsOpen(false); + setPageIndex(0); + action("cancel")(ev); + }; + const handleOpen = ( + event: React.MouseEvent, + ) => { + setIsOpen(true); + if (!initialPageIndex) { + setPageIndex(0); + } else setPageIndex(Number(initialPageIndex)); + action("open")(event); + }; + const handleOnClick = ( + ev: React.MouseEvent, + ) => { + setIsDisabled(true); + setPageIndex(pageIndex + 1); + setTimeout(() => { + setIsDisabled(false); + }, 50); + action("click")(ev); + action("slide")(`Page index: ${pageIndex + 1}`); + }; + const handleBackClick = ( + ev: + | React.MouseEvent + | React.MouseEvent + | React.KeyboardEvent + | React.KeyboardEvent, + ) => { + setIsDisabled(true); + setTimeout(() => { + setIsDisabled(false); + }, 50); + if (!isDisabled) { + ev.preventDefault(); + setPageIndex(pageIndex - 1); + action("click")(ev); + action("slide")(`Page index: ${pageIndex + 1}`); + } + }; + return ( +
+ + + + + + + + + + + } + > + Third Page + + + +
+ ); +}; diff --git a/src/components/pages/pages.test.tsx b/src/components/pages/pages.test.tsx index 14c13ab942..92a0c5aa52 100644 --- a/src/components/pages/pages.test.tsx +++ b/src/components/pages/pages.test.tsx @@ -159,6 +159,16 @@ test("navigating to the next page should render the first page when currently on expect(screen.getByRole("heading", { name: "My First Page" })).toBeVisible(); }); +test("component renders correctly with no title prop passed", () => { + render( + + First Page + , + ); + + expect(screen.getByTestId("visible-page")).toHaveTextContent("First Page"); +}); + test("when attempting to navigate pages and there is only one page, it should not change the rendered content", async () => { const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); const WithSinglePage = () => {