Skip to content

Commit

Permalink
fix(heading): maintain layout when title prop is set to null
Browse files Browse the repository at this point in the history
  • Loading branch information
DobroTora committed Feb 3, 2025
1 parent 03365cf commit 3d8c627
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ export const DialogFullScreen = ({
pagesStyling={pagesStyling}
role={role}
>
{dialogTitle()}
{title || headerChildren ? dialogTitle() : null}
{closeIcon()}
<StyledContent
hasHeader={title !== undefined}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const StyledDialogFullScreen = styled.div<{ pagesStyling?: boolean }>`
}
${StyledFullScreenHeading} {
padding: 32px 32px 0;
padding: 64px 32px 0;
}
${StyledHeading} {
Expand Down
16 changes: 11 additions & 5 deletions src/components/dialog-full-screen/dialog-full-screen.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,14 @@ test("renders the element given in the `headerChildren` prop", () => {
).toBeVisible();
});

test("does not render anything if no `headerChildren` prop is provided", () => {
render(<DialogFullScreen open />);

const link = screen.queryByRole("link");

expect(link).not.toBeInTheDocument();
});

// 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(
Expand Down Expand Up @@ -278,11 +286,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}`,
});
Expand All @@ -299,6 +302,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:
Expand Down
6 changes: 4 additions & 2 deletions src/components/pages/page/page.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -38,7 +38,9 @@ const Page = ({ role, title, children, ...rest }: PageProps) => {
ref={styledPageNodeRef}
role={role}
>
<FullScreenHeading hasContent>{title}</FullScreenHeading>
{title ? (
<FullScreenHeading hasContent>{title}</FullScreenHeading>
) : null}
<StyledPageContent
data-element="carbon-page-content"
data-role="page-content"
Expand Down
24 changes: 23 additions & 1 deletion src/components/pages/pages-test.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import Box from "../box";

export default {
title: "Pages/Test",
includeStories: ["DefaultStory", "DifferentPageHeights"],
includeStories: ["DefaultStory", "DifferentPageHeights", "WithoutTitle"],
parameters: {
info: { disable: true },
chromatic: {
Expand Down Expand Up @@ -525,3 +525,25 @@ export const DifferentPageHeights = () => {
</Box>
);
};

export const WithoutTitle = () => {
const [isOpen, setIsOpen] = useState(false);
const handleCancel = () => {
setIsOpen(false);
};
const handleOpen = () => {
setIsOpen(true);
};
return (
<div>
<Button onClick={handleOpen}>Open Preview</Button>
<DialogFullScreen pagesStyling open={isOpen} onCancel={handleCancel}>
<Pages pageIndex={0}>
<Page>
<Button>Example button</Button>
</Page>
</Pages>
</DialogFullScreen>
</div>
);
};
10 changes: 10 additions & 0 deletions src/components/pages/pages.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<Pages pageIndex={0}>
<Page>First Page</Page>
</Pages>,
);

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 = () => {
Expand Down

0 comments on commit 3d8c627

Please sign in to comment.