From 0bfe09d7ee247d7ce54fe3b46bd3bc332c9ede3f Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Tue, 25 Jun 2024 11:18:22 -0400 Subject: [PATCH 01/10] feat(Page): updated logic to allow section grouping --- .../react-core/src/components/Page/Page.tsx | 5 ++-- .../src/components/Page/PageBreadcrumb.tsx | 9 +++++-- .../src/components/Page/PageMainBody.tsx | 24 +++++++++++++++++++ .../src/components/Page/PageNavigation.tsx | 9 +++++-- .../src/components/Page/PageSection.tsx | 9 +++++-- .../react-core/src/components/Page/index.ts | 1 + 6 files changed, 49 insertions(+), 8 deletions(-) create mode 100644 packages/react-core/src/components/Page/PageMainBody.tsx diff --git a/packages/react-core/src/components/Page/Page.tsx b/packages/react-core/src/components/Page/Page.tsx index 5ecf9d6dfdc..632357837d2 100644 --- a/packages/react-core/src/components/Page/Page.tsx +++ b/packages/react-core/src/components/Page/Page.tsx @@ -9,6 +9,7 @@ import { PageGroup, PageGroupProps } from './PageGroup'; import { getResizeObserver } from '../../helpers/resizeObserver'; import { formatBreakpointMods, getBreakpoint, getVerticalBreakpoint } from '../../helpers/util'; import { PageContextProvider } from './PageContext'; +import { PageMainBody } from './PageMainBody'; export enum PageLayouts { vertical = 'vertical', @@ -266,7 +267,7 @@ class Page extends React.Component { if (horizontalSubnav && isHorizontalSubnavWidthLimited) { nav = (
-
{horizontalSubnav}
+ {horizontalSubnav}
); } else if (horizontalSubnav) { @@ -287,7 +288,7 @@ class Page extends React.Component { ) )} > - {isBreadcrumbWidthLimited ?
{breadcrumb}
: breadcrumb} + {isBreadcrumbWidthLimited ? {breadcrumb} : breadcrumb} ) : null; diff --git a/packages/react-core/src/components/Page/PageBreadcrumb.tsx b/packages/react-core/src/components/Page/PageBreadcrumb.tsx index c8c8ad7e6cd..8c66021e15e 100644 --- a/packages/react-core/src/components/Page/PageBreadcrumb.tsx +++ b/packages/react-core/src/components/Page/PageBreadcrumb.tsx @@ -3,6 +3,7 @@ import { css } from '@patternfly/react-styles'; import styles from '@patternfly/react-styles/css/components/Page/page'; import { formatBreakpointMods, Mods } from '../../helpers/util'; import { PageContext } from './PageContext'; +import { PageMainBody } from './PageMainBody'; export interface PageBreadcrumbProps extends React.HTMLProps { /** Additional classes to apply to the PageBreadcrumb */ @@ -26,6 +27,10 @@ export interface PageBreadcrumbProps extends React.HTMLProps { hasShadowBottom?: boolean; /** Flag indicating if the PageBreadcrumb has a scrolling overflow */ hasOverflowScroll?: boolean; + /** @beta Flag indicating whether children passed to the component should be wrapped by a PageMainBody. + * Set this to false in order to pass multiple, custom PageMainBody's as children. + */ + hasMainBodyWrapper?: boolean; /** Adds an accessible name to the breadcrumb section. Required when the hasOverflowScroll prop is set to true. */ 'aria-label'?: string; } @@ -39,6 +44,7 @@ export const PageBreadcrumb = ({ hasShadowBottom = false, hasOverflowScroll = false, 'aria-label': ariaLabel, + hasMainBodyWrapper = true, ...props }: PageBreadcrumbProps) => { const { height, getVerticalBreakpoint } = React.useContext(PageContext); @@ -65,8 +71,7 @@ export const PageBreadcrumb = ({ aria-label={ariaLabel} {...props} > - {isWidthLimited &&
{children}
} - {!isWidthLimited && children} + {hasMainBodyWrapper ? {children} : children} ); }; diff --git a/packages/react-core/src/components/Page/PageMainBody.tsx b/packages/react-core/src/components/Page/PageMainBody.tsx new file mode 100644 index 00000000000..070c4314250 --- /dev/null +++ b/packages/react-core/src/components/Page/PageMainBody.tsx @@ -0,0 +1,24 @@ +import * as React from 'react'; +import styles from '@patternfly/react-styles/css/components/Page/page'; +import { css } from '@patternfly/react-styles'; + +export interface PageMainBodyProps extends React.HTMLProps { + /** Content rendered inside the section */ + children?: React.ReactNode; + /** Additional classes added to the section */ + className?: string; + /** Section background color variant. This will only apply when the type prop has the "default" value. */ +} + +export const PageMainBody: React.FunctionComponent = ({ + className, + children, + ...props +}: PageMainBodyProps) => { + return ( +
+ {children} +
+ ); +}; +PageMainBody.displayName = 'PageMainBody'; diff --git a/packages/react-core/src/components/Page/PageNavigation.tsx b/packages/react-core/src/components/Page/PageNavigation.tsx index 33845792385..7a4c109eb90 100644 --- a/packages/react-core/src/components/Page/PageNavigation.tsx +++ b/packages/react-core/src/components/Page/PageNavigation.tsx @@ -3,6 +3,7 @@ import { css } from '@patternfly/react-styles'; import styles from '@patternfly/react-styles/css/components/Page/page'; import { formatBreakpointMods } from '../../helpers/util'; import { PageContext } from './PageContext'; +import { PageMainBody } from './PageMainBody'; export interface PageNavigationProps extends React.HTMLProps { /** Additional classes to apply to the PageNavigation */ @@ -26,6 +27,10 @@ export interface PageNavigationProps extends React.HTMLProps { hasShadowBottom?: boolean; /** Flag indicating if the PageNavigation has a scrolling overflow */ hasOverflowScroll?: boolean; + /** @beta Flag indicating whether children passed to the component should be wrapped by a PageMainBody. + * Set this to false in order to pass multiple, custom PageMainBody's as children. + */ + hasMainBodyWrapper?: boolean; /** Adds an accessible name to the page navigation when the hasOverflowScroll prop is set to true. */ 'aria-label'?: string; } @@ -39,6 +44,7 @@ export const PageNavigation = ({ hasShadowBottom = false, hasOverflowScroll = false, 'aria-label': ariaLabel, + hasMainBodyWrapper = true, ...props }: PageNavigationProps) => { const { height, getVerticalBreakpoint } = React.useContext(PageContext); @@ -64,8 +70,7 @@ export const PageNavigation = ({ {...(hasOverflowScroll && { tabIndex: 0, role: 'region', 'aria-label': ariaLabel })} {...props} > - {isWidthLimited &&
{children}
} - {!isWidthLimited && children} + {hasMainBodyWrapper ? {children} : children} ); }; diff --git a/packages/react-core/src/components/Page/PageSection.tsx b/packages/react-core/src/components/Page/PageSection.tsx index dee8237f8c7..2ad1e94e787 100644 --- a/packages/react-core/src/components/Page/PageSection.tsx +++ b/packages/react-core/src/components/Page/PageSection.tsx @@ -3,6 +3,7 @@ import styles from '@patternfly/react-styles/css/components/Page/page'; import { css } from '@patternfly/react-styles'; import { formatBreakpointMods } from '../../helpers/util'; import { PageContext } from './PageContext'; +import { PageMainBody } from './PageMainBody'; export enum PageSectionVariants { default = 'default', @@ -57,6 +58,10 @@ export interface PageSectionProps extends React.HTMLProps { hasShadowBottom?: boolean; /** Flag indicating if the PageSection has a scrolling overflow */ hasOverflowScroll?: boolean; + /** @beta Flag indicating whether children passed to the component should be wrapped by a PageMainBody. + * Set this to false in order to pass multiple, custom PageMainBody's as children. + */ + hasMainBodyWrapper?: boolean; /** Adds an accessible name to the page section. Required when the hasOverflowScroll prop is set to true. * This prop should also be passed in if a heading is not being used to describe the content of the page section. */ @@ -94,6 +99,7 @@ export const PageSection: React.FunctionComponent = ({ hasOverflowScroll = false, 'aria-label': ariaLabel, component = 'section', + hasMainBodyWrapper = true, ...props }: PageSectionProps) => { const { height, getVerticalBreakpoint } = React.useContext(PageContext); @@ -127,8 +133,7 @@ export const PageSection: React.FunctionComponent = ({ {...(hasOverflowScroll && { tabIndex: 0 })} aria-label={ariaLabel} > - {isWidthLimited &&
{children}
} - {!isWidthLimited && children} + {hasMainBodyWrapper ? {children} : children} ); }; diff --git a/packages/react-core/src/components/Page/index.ts b/packages/react-core/src/components/Page/index.ts index fd3bc29fe52..5044df11deb 100644 --- a/packages/react-core/src/components/Page/index.ts +++ b/packages/react-core/src/components/Page/index.ts @@ -1,4 +1,5 @@ export * from './Page'; +export * from './PageMainBody'; export * from './PageBreadcrumb'; export * from './PageGroup'; export * from './PageSidebar'; From a365aaab9c59d433a3d38d801a9e29aa30097df0 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Tue, 25 Jun 2024 11:33:30 -0400 Subject: [PATCH 02/10] Updated and added unit tests --- .../Page/__tests__/PageBreadcrumb.test.tsx | 15 +- .../Page/__tests__/PageMainBody.test.tsx | 30 + .../Page/__tests__/PageNavigation.test.tsx | 15 +- .../Page/__tests__/PageSection.test.tsx | 17 +- .../__snapshots__/Page.test.tsx.snap | 1340 +++++++++-------- .../PageBreadcrumb.test.tsx.snap | 36 +- .../PageNavigation.test.tsx.snap | 36 +- .../__snapshots__/PageSection.test.tsx.snap | 78 +- 8 files changed, 908 insertions(+), 659 deletions(-) create mode 100644 packages/react-core/src/components/Page/__tests__/PageMainBody.test.tsx diff --git a/packages/react-core/src/components/Page/__tests__/PageBreadcrumb.test.tsx b/packages/react-core/src/components/Page/__tests__/PageBreadcrumb.test.tsx index 63742eb0de6..8f19030b0e9 100644 --- a/packages/react-core/src/components/Page/__tests__/PageBreadcrumb.test.tsx +++ b/packages/react-core/src/components/Page/__tests__/PageBreadcrumb.test.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import { render, screen } from '@testing-library/react'; import { PageBreadcrumb } from '../PageBreadcrumb'; +import styles from '@patternfly/react-styles/css/components/Page/page'; describe('page breadcrumb', () => { test('Verify basic render', () => { @@ -32,6 +33,7 @@ describe('page breadcrumb', () => { expect(asFragment()).toMatchSnapshot(); }); + // Old snapshot tests end here. The following tests can be kept if Page test suites need a revamp test('Renders without an aria-label by default', () => { render(test); @@ -41,7 +43,7 @@ describe('page breadcrumb', () => { test('Renders with the passed aria-label applied', () => { render(test); - expect(screen.getByText('test')).toHaveAccessibleName('Test label'); + expect(screen.getByText('test').parentElement).toHaveAccessibleName('Test label'); }); test('Does not log a warning in the console by default', () => { @@ -71,4 +73,15 @@ describe('page breadcrumb', () => { expect(consoleWarning).toHaveBeenCalled(); }); + + test('Renders with PageMainBody wrapper by default', () => { + render(test); + + expect(screen.getByText('test')).toHaveClass(styles.pageMainBody); + }); + test('Does not render with PageMainBody wrapper when hasMainBodyWrapper is false', () => { + render(test); + + expect(screen.getByText('test')).not.toHaveClass(styles.pageMainBody); + }); }); diff --git a/packages/react-core/src/components/Page/__tests__/PageMainBody.test.tsx b/packages/react-core/src/components/Page/__tests__/PageMainBody.test.tsx new file mode 100644 index 00000000000..d4db0675b58 --- /dev/null +++ b/packages/react-core/src/components/Page/__tests__/PageMainBody.test.tsx @@ -0,0 +1,30 @@ +import * as React from 'react'; +import { render, screen } from '@testing-library/react'; +import { PageMainBody } from '../PageMainBody'; +import styles from '@patternfly/react-styles/css/components/Page/page'; + +test('Renders without children', () => { + render(); + + expect(screen.getByTestId('page-main-body')).toBeVisible(); +}); +test('Renders children', () => { + render(Test); + + expect(screen.getByText('Test')).toBeVisible(); +}); +test(`Renders with class ${styles.pageMainBody} by default`, () => { + render(Test); + + expect(screen.getByText('Test')).toHaveClass(styles.pageMainBody); +}); +test(`Renders with custom classes when className is passed`, () => { + render(Test); + + expect(screen.getByText('Test')).toHaveClass('custom-class'); +}); +test(`Renders with spread props`, () => { + render(Test); + + expect(screen.getByText('Test')).toHaveAttribute('id', 'custom-id'); +}); diff --git a/packages/react-core/src/components/Page/__tests__/PageNavigation.test.tsx b/packages/react-core/src/components/Page/__tests__/PageNavigation.test.tsx index 8f9d77dbe0a..4aa470acccd 100644 --- a/packages/react-core/src/components/Page/__tests__/PageNavigation.test.tsx +++ b/packages/react-core/src/components/Page/__tests__/PageNavigation.test.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import { render, screen } from '@testing-library/react'; import { PageNavigation } from '../PageNavigation'; +import styles from '@patternfly/react-styles/css/components/Page/page'; describe('page navigation', () => { test('Verify basic render', () => { @@ -32,6 +33,7 @@ describe('page navigation', () => { expect(asFragment()).toMatchSnapshot(); }); + // Old snapshot tests end here. The following tests can be kept if Page test suites need a revamp test('Renders without an aria-label by default', () => { render(test); @@ -45,7 +47,7 @@ describe('page navigation', () => { ); - expect(screen.getByText('test')).toHaveAccessibleName('Test label'); + expect(screen.getByText('test').parentElement).toHaveAccessibleName('Test label'); }); test('Does not log a warning in the console by default', () => { @@ -75,4 +77,15 @@ describe('page navigation', () => { expect(consoleWarning).toHaveBeenCalled(); }); + + test('Renders with PageMainBody wrapper by default', () => { + render(test); + + expect(screen.getByText('test')).toHaveClass(styles.pageMainBody); + }); + test('Does not render with PageMainBody wrapper when hasMainBodyWrapper is false', () => { + render(test); + + expect(screen.getByText('test')).not.toHaveClass(styles.pageMainBody); + }); }); diff --git a/packages/react-core/src/components/Page/__tests__/PageSection.test.tsx b/packages/react-core/src/components/Page/__tests__/PageSection.test.tsx index ceed384957f..bc757852215 100644 --- a/packages/react-core/src/components/Page/__tests__/PageSection.test.tsx +++ b/packages/react-core/src/components/Page/__tests__/PageSection.test.tsx @@ -1,6 +1,7 @@ import * as React from 'react'; import { render, screen } from '@testing-library/react'; import { PageSection, PageSectionTypes } from '../PageSection'; +import styles from '@patternfly/react-styles/css/components/Page/page'; jest.mock('../Page'); @@ -89,6 +90,7 @@ test('Verify page section overflow scroll', () => { expect(asFragment()).toMatchSnapshot(); }); +// Old snapshot tests end here. The following tests can be kept if Page test suites need a revamp test('Renders without an aria-label by default', () => { render(test); @@ -98,7 +100,7 @@ test('Renders without an aria-label by default', () => { test('Renders with the passed aria-label applied', () => { render(test); - expect(screen.getByText('test')).toHaveAccessibleName('Test label'); + expect(screen.getByText('test').parentElement).toHaveAccessibleName('Test label'); }); test('Does not log a warning in the console by default', () => { @@ -132,7 +134,7 @@ test('Logs a warning in the console when an aria-label is not included with hasO test('Renders as a section by default', () => { render(test); - expect(screen.getByText('test')).toHaveProperty('nodeName', 'SECTION'); + expect(screen.getByText('test').parentElement).toHaveProperty('nodeName', 'SECTION'); }); test('Renders as other elements when a different element type is passed using the component prop', () => { @@ -140,3 +142,14 @@ test('Renders as other elements when a different element type is passed using th expect(screen.getByRole('main')).toHaveTextContent('test'); }); + +test('Renders with PageMainBody wrapper by default', () => { + render(test); + + expect(screen.getByText('test')).toHaveClass(styles.pageMainBody); +}); +test('Does not render with PageMainBody wrapper when hasMainBodyWrapper is false', () => { + render(test); + + expect(screen.getByText('test')).not.toHaveClass(styles.pageMainBody); +}); diff --git a/packages/react-core/src/components/Page/__tests__/__snapshots__/Page.test.tsx.snap b/packages/react-core/src/components/Page/__tests__/__snapshots__/Page.test.tsx.snap index 42de7325643..55caf19c94a 100644 --- a/packages/react-core/src/components/Page/__tests__/__snapshots__/Page.test.tsx.snap +++ b/packages/react-core/src/components/Page/__tests__/__snapshots__/Page.test.tsx.snap @@ -31,7 +31,11 @@ exports[`Page Check dark page against snapshot 1`] = `
- Section with default background +
+ Section with default background +
@@ -64,7 +68,11 @@ exports[`Page Check page horizontal layout example against snapshot 1`] = `
- Section with default background +
+ Section with default background +
@@ -97,111 +105,119 @@ exports[`Page Check page to verify breadcrumb is created - PageBreadcrumb syntax
- + + + + + Section Landing + + + + +
- Section with default background +
+ Section with default background +
@@ -338,7 +354,11 @@ exports[`Page Check page to verify breadcrumb is created 1`] = `
- Section with default background +
+ Section with default background +
@@ -374,210 +394,222 @@ exports[`Page Check page to verify grouped nav and breadcrumb - new components s
- + + + + + Section Landing + + + + +
- + + Server + + + + + +
- Section with default background +
+ Section with default background +
@@ -717,113 +749,121 @@ exports[`Page Check page to verify grouped nav and breadcrumb - old / props synt
- + + + + + Section Landing + + + + +
- Section with default background +
+ Section with default background +
@@ -856,105 +896,113 @@ exports[`Page Check page to verify nav is created - PageNavigation syntax 1`] =
- + + Server + + + + + +
- Section with default background +
+ Section with default background +
@@ -1111,7 +1159,11 @@ exports[`Page Check page to verify skip to content points to main content region
- Section with default background +
+ Section with default background +
@@ -1150,7 +1202,11 @@ exports[`Page Check page vertical layout example against snapshot 1`] = `
- Section with default background +
+ Section with default background +
@@ -1183,111 +1239,119 @@ exports[`Page Sticky bottom breadcrumb on all height breakpoints - PageBreadcrum
- + + + + + Section Landing + + + + +
- Section with default background +
+ Section with default background +
@@ -1424,7 +1488,11 @@ exports[`Page Verify sticky bottom breadcrumb on all height breakpoints 1`] = `
- Section with default background +
+ Section with default background +
@@ -1457,111 +1525,119 @@ exports[`Page Verify sticky top breadcrumb on all height breakpoints - PageBread
- + + + + + Section Landing + + + + +
- Section with default background +
+ Section with default background +
@@ -1698,7 +1774,11 @@ exports[`Page Verify sticky top breadcrumb on all height breakpoints 1`] = `
- Section with default background +
+ Section with default background +
diff --git a/packages/react-core/src/components/Page/__tests__/__snapshots__/PageBreadcrumb.test.tsx.snap b/packages/react-core/src/components/Page/__tests__/__snapshots__/PageBreadcrumb.test.tsx.snap index 628242e47e2..468121291eb 100644 --- a/packages/react-core/src/components/Page/__tests__/__snapshots__/PageBreadcrumb.test.tsx.snap +++ b/packages/react-core/src/components/Page/__tests__/__snapshots__/PageBreadcrumb.test.tsx.snap @@ -5,7 +5,11 @@ exports[`page breadcrumb Verify basic render 1`] = `
- test +
+ test +
`; @@ -15,7 +19,11 @@ exports[`page breadcrumb Verify bottom shadow 1`] = `
- test +
+ test +
`; @@ -25,7 +33,11 @@ exports[`page breadcrumb Verify bottom sticky 1`] = `
- test +
+ test +
`; @@ -50,7 +62,11 @@ exports[`page breadcrumb Verify overflow scroll 1`] = ` class="pf-v6-c-page__main-breadcrumb pf-m-overflow-scroll" tabindex="0" > - test +
+ test +
`; @@ -60,7 +76,11 @@ exports[`page breadcrumb Verify top shadow 1`] = `
- test +
+ test +
`; @@ -70,7 +90,11 @@ exports[`page breadcrumb Verify top sticky 1`] = `
- test +
+ test +
`; diff --git a/packages/react-core/src/components/Page/__tests__/__snapshots__/PageNavigation.test.tsx.snap b/packages/react-core/src/components/Page/__tests__/__snapshots__/PageNavigation.test.tsx.snap index 7363d3cff31..730b247318a 100644 --- a/packages/react-core/src/components/Page/__tests__/__snapshots__/PageNavigation.test.tsx.snap +++ b/packages/react-core/src/components/Page/__tests__/__snapshots__/PageNavigation.test.tsx.snap @@ -5,7 +5,11 @@ exports[`page navigation Verify basic render 1`] = `
- test +
+ test +
`; @@ -15,7 +19,11 @@ exports[`page navigation Verify bottom shadow 1`] = `
- test +
+ test +
`; @@ -25,7 +33,11 @@ exports[`page navigation Verify bottom sticky 1`] = `
- test +
+ test +
`; @@ -51,7 +63,11 @@ exports[`page navigation Verify overflow scroll 1`] = ` role="region" tabindex="0" > - test +
+ test +
`; @@ -61,7 +77,11 @@ exports[`page navigation Verify top shadow 1`] = `
- test +
+ test +
`; @@ -71,7 +91,11 @@ exports[`page navigation Verify top sticky 1`] = `
- test +
+ test +
`; diff --git a/packages/react-core/src/components/Page/__tests__/__snapshots__/PageSection.test.tsx.snap b/packages/react-core/src/components/Page/__tests__/__snapshots__/PageSection.test.tsx.snap index 23e77cf82b0..8d5c9ba118c 100644 --- a/packages/react-core/src/components/Page/__tests__/__snapshots__/PageSection.test.tsx.snap +++ b/packages/react-core/src/components/Page/__tests__/__snapshots__/PageSection.test.tsx.snap @@ -4,7 +4,11 @@ exports[`Check page main breadcrumb section against snapshot 1`] = `
+ > +
+
`; @@ -12,7 +16,11 @@ exports[`Check page main nav section against snapshot 1`] = `
+ > +
+
`; @@ -20,7 +28,11 @@ exports[`Check page main subnav section against snapshot 1`] = `
+ > +
+
`; @@ -28,7 +40,11 @@ exports[`Check page main tabs section against snapshot 1`] = `
+ > +
+
`; @@ -48,7 +64,11 @@ exports[`Check page section with fill and no padding example against snapshot 1`
+ > +
+
`; @@ -56,7 +76,11 @@ exports[`Check page section with fill example against snapshot 1`] = `
+ > +
+
`; @@ -76,7 +100,11 @@ exports[`Check page section with no fill example against snapshot 1`] = `
+ > +
+
`; @@ -84,7 +112,11 @@ exports[`Check page section with no padding example against snapshot 1`] = `
+ > +
+
`; @@ -93,7 +125,11 @@ exports[`Verify page section bottom shadow 1`] = `
- test +
+ test +
`; @@ -103,7 +139,11 @@ exports[`Verify page section bottom sticky 1`] = `
- test +
+ test +
`; @@ -114,7 +154,11 @@ exports[`Verify page section overflow scroll 1`] = ` class="pf-v6-c-page__main-section pf-m-overflow-scroll" tabindex="0" > - test +
+ test +
`; @@ -124,7 +168,11 @@ exports[`Verify page section top shadow 1`] = `
- test +
+ test +
`; @@ -134,7 +182,11 @@ exports[`Verify page section top sticky 1`] = `
- test +
+ test +
`; From 7c57c3c99f6e3a54801348f5e3df30e9167a5925 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Tue, 25 Jun 2024 11:40:15 -0400 Subject: [PATCH 03/10] Added snapshot test --- .../src/components/Page/__tests__/PageMainBody.test.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/react-core/src/components/Page/__tests__/PageMainBody.test.tsx b/packages/react-core/src/components/Page/__tests__/PageMainBody.test.tsx index d4db0675b58..3a55fe479de 100644 --- a/packages/react-core/src/components/Page/__tests__/PageMainBody.test.tsx +++ b/packages/react-core/src/components/Page/__tests__/PageMainBody.test.tsx @@ -28,3 +28,7 @@ test(`Renders with spread props`, () => { expect(screen.getByText('Test')).toHaveAttribute('id', 'custom-id'); }); +test('Matches snapshot', () => { + const { asFragment } = render(Test); + expect(asFragment()).toMatchSnapshot(); +}) From 39dda5259ac6bc891d81e87b2566662be4c44422 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Tue, 25 Jun 2024 13:25:37 -0400 Subject: [PATCH 04/10] Removed PageNavigation component --- .../react-core/src/components/Page/Page.tsx | 4 +- .../src/components/Page/PageMainBody.tsx | 13 +- .../src/components/Page/PageNavigation.tsx | 77 ---- .../src/components/Page/PageSection.tsx | 4 +- .../components/Page/__tests__/Page.test.tsx | 52 +-- .../Page/__tests__/PageMainBody.test.tsx | 2 +- .../Page/__tests__/PageNavigation.test.tsx | 91 ----- .../__snapshots__/Page.test.tsx.snap | 329 +++++------------- .../__snapshots__/PageMainBody.test.tsx.snap | 11 + .../PageNavigation.test.tsx.snap | 101 ------ .../__snapshots__/PageSection.test.tsx.snap | 2 +- .../src/components/Page/examples/Page.md | 13 +- .../Page/examples/PageGroupSection.tsx | 41 +-- .../react-core/src/components/Page/index.ts | 1 - 14 files changed, 145 insertions(+), 596 deletions(-) delete mode 100644 packages/react-core/src/components/Page/PageNavigation.tsx delete mode 100644 packages/react-core/src/components/Page/__tests__/PageNavigation.test.tsx create mode 100644 packages/react-core/src/components/Page/__tests__/__snapshots__/PageMainBody.test.tsx.snap delete mode 100644 packages/react-core/src/components/Page/__tests__/__snapshots__/PageNavigation.test.tsx.snap diff --git a/packages/react-core/src/components/Page/Page.tsx b/packages/react-core/src/components/Page/Page.tsx index 632357837d2..1f7d245523c 100644 --- a/packages/react-core/src/components/Page/Page.tsx +++ b/packages/react-core/src/components/Page/Page.tsx @@ -266,12 +266,12 @@ class Page extends React.Component { let nav = null; if (horizontalSubnav && isHorizontalSubnavWidthLimited) { nav = ( -
+
{horizontalSubnav}
); } else if (horizontalSubnav) { - nav =
{horizontalSubnav}
; + nav =
{horizontalSubnav}
; } const crumb = breadcrumb ? ( diff --git a/packages/react-core/src/components/Page/PageMainBody.tsx b/packages/react-core/src/components/Page/PageMainBody.tsx index 070c4314250..f7db1fcdf16 100644 --- a/packages/react-core/src/components/Page/PageMainBody.tsx +++ b/packages/react-core/src/components/Page/PageMainBody.tsx @@ -14,11 +14,10 @@ export const PageMainBody: React.FunctionComponent = ({ className, children, ...props -}: PageMainBodyProps) => { - return ( -
- {children} -
- ); -}; +}: PageMainBodyProps) => ( +
+ {children} +
+); + PageMainBody.displayName = 'PageMainBody'; diff --git a/packages/react-core/src/components/Page/PageNavigation.tsx b/packages/react-core/src/components/Page/PageNavigation.tsx deleted file mode 100644 index 7a4c109eb90..00000000000 --- a/packages/react-core/src/components/Page/PageNavigation.tsx +++ /dev/null @@ -1,77 +0,0 @@ -import * as React from 'react'; -import { css } from '@patternfly/react-styles'; -import styles from '@patternfly/react-styles/css/components/Page/page'; -import { formatBreakpointMods } from '../../helpers/util'; -import { PageContext } from './PageContext'; -import { PageMainBody } from './PageMainBody'; - -export interface PageNavigationProps extends React.HTMLProps { - /** Additional classes to apply to the PageNavigation */ - className?: string; - /** Content rendered inside of the PageNavigation */ - children?: React.ReactNode; - /** Limits the width of the PageNavigation */ - isWidthLimited?: boolean; - /** Modifier indicating if the PageBreadcrumb is sticky to the top or bottom at various breakpoints */ - stickyOnBreakpoint?: { - default?: 'top' | 'bottom'; - sm?: 'top' | 'bottom'; - md?: 'top' | 'bottom'; - lg?: 'top' | 'bottom'; - xl?: 'top' | 'bottom'; - '2xl'?: 'top' | 'bottom'; - }; - /** Flag indicating if PageNavigation should have a shadow at the top */ - hasShadowTop?: boolean; - /** Flag indicating if PageNavigation should have a shadow at the bottom */ - hasShadowBottom?: boolean; - /** Flag indicating if the PageNavigation has a scrolling overflow */ - hasOverflowScroll?: boolean; - /** @beta Flag indicating whether children passed to the component should be wrapped by a PageMainBody. - * Set this to false in order to pass multiple, custom PageMainBody's as children. - */ - hasMainBodyWrapper?: boolean; - /** Adds an accessible name to the page navigation when the hasOverflowScroll prop is set to true. */ - 'aria-label'?: string; -} - -export const PageNavigation = ({ - className = '', - children, - isWidthLimited, - stickyOnBreakpoint, - hasShadowTop = false, - hasShadowBottom = false, - hasOverflowScroll = false, - 'aria-label': ariaLabel, - hasMainBodyWrapper = true, - ...props -}: PageNavigationProps) => { - const { height, getVerticalBreakpoint } = React.useContext(PageContext); - - React.useEffect(() => { - if (hasOverflowScroll && !ariaLabel) { - /* eslint-disable no-console */ - console.warn('PageNavigation: An accessible aria-label is required when hasOverflowScroll is set to true.'); - } - }, [hasOverflowScroll, ariaLabel]); - - return ( -
- {hasMainBodyWrapper ? {children} : children} -
- ); -}; -PageNavigation.displayName = 'PageNavigation'; diff --git a/packages/react-core/src/components/Page/PageSection.tsx b/packages/react-core/src/components/Page/PageSection.tsx index 2ad1e94e787..b18d44e1bce 100644 --- a/packages/react-core/src/components/Page/PageSection.tsx +++ b/packages/react-core/src/components/Page/PageSection.tsx @@ -12,7 +12,6 @@ export enum PageSectionVariants { export enum PageSectionTypes { default = 'default', - nav = 'nav', subNav = 'subnav', breadcrumb = 'breadcrumb', tabs = 'tabs', @@ -27,7 +26,7 @@ export interface PageSectionProps extends React.HTMLProps { /** Section background color variant. This will only apply when the type prop has the "default" value. */ variant?: 'default' | 'secondary'; /** Section type variant */ - type?: 'default' | 'nav' | 'subnav' | 'breadcrumb' | 'tabs' | 'wizard'; + type?: 'default' | 'subnav' | 'breadcrumb' | 'tabs' | 'wizard'; /** Enables the page section to fill the available vertical space */ isFilled?: boolean; /** Limits the width of the section */ @@ -72,7 +71,6 @@ export interface PageSectionProps extends React.HTMLProps { const variantType = { [PageSectionTypes.default]: styles.pageMainSection, - [PageSectionTypes.nav]: styles.pageMainNav, [PageSectionTypes.subNav]: styles.pageMainSubnav, [PageSectionTypes.breadcrumb]: styles.pageMainBreadcrumb, [PageSectionTypes.tabs]: styles.pageMainTabs, diff --git a/packages/react-core/src/components/Page/__tests__/Page.test.tsx b/packages/react-core/src/components/Page/__tests__/Page.test.tsx index a72f3400ee1..e2a3e20cbfa 100644 --- a/packages/react-core/src/components/Page/__tests__/Page.test.tsx +++ b/packages/react-core/src/components/Page/__tests__/Page.test.tsx @@ -10,7 +10,6 @@ import { Breadcrumb, BreadcrumbItem } from '../../Breadcrumb'; import { Nav, NavList, NavItem } from '../../Nav'; import { SkipToContent } from '../../SkipToContent'; import { PageBreadcrumb } from '../PageBreadcrumb'; -import { PageNavigation } from '../PageNavigation'; import { PageGroup } from '../PageGroup'; import { Masthead } from '../../Masthead'; @@ -228,33 +227,6 @@ describe('Page', () => { expect(asFragment()).toMatchSnapshot(); }); - test('Check page to verify nav is created - PageNavigation syntax', () => { - const masthead = ; - const Sidebar = ; - - const { asFragment } = render( - - - - - Section with default background - - ); - - expect(screen.getByRole('main')).not.toHaveAttribute('id'); - expect(asFragment()).toMatchSnapshot(); - }); - test('Check page to verify grouped nav and breadcrumb - new components syntax', () => { const masthead = ; const Sidebar = ; @@ -272,19 +244,17 @@ describe('Page', () => { - - - + Section with default background diff --git a/packages/react-core/src/components/Page/__tests__/PageMainBody.test.tsx b/packages/react-core/src/components/Page/__tests__/PageMainBody.test.tsx index 3a55fe479de..53b39ac60fb 100644 --- a/packages/react-core/src/components/Page/__tests__/PageMainBody.test.tsx +++ b/packages/react-core/src/components/Page/__tests__/PageMainBody.test.tsx @@ -31,4 +31,4 @@ test(`Renders with spread props`, () => { test('Matches snapshot', () => { const { asFragment } = render(Test); expect(asFragment()).toMatchSnapshot(); -}) +}); diff --git a/packages/react-core/src/components/Page/__tests__/PageNavigation.test.tsx b/packages/react-core/src/components/Page/__tests__/PageNavigation.test.tsx deleted file mode 100644 index 4aa470acccd..00000000000 --- a/packages/react-core/src/components/Page/__tests__/PageNavigation.test.tsx +++ /dev/null @@ -1,91 +0,0 @@ -import * as React from 'react'; -import { render, screen } from '@testing-library/react'; -import { PageNavigation } from '../PageNavigation'; -import styles from '@patternfly/react-styles/css/components/Page/page'; - -describe('page navigation', () => { - test('Verify basic render', () => { - const { asFragment } = render(test); - expect(asFragment()).toMatchSnapshot(); - }); - test('Verify limited width', () => { - const { asFragment } = render(test); - expect(asFragment()).toMatchSnapshot(); - }); - test('Verify top sticky', () => { - const { asFragment } = render(test); - expect(asFragment()).toMatchSnapshot(); - }); - test('Verify bottom sticky', () => { - const { asFragment } = render(test); - expect(asFragment()).toMatchSnapshot(); - }); - test('Verify top shadow', () => { - const { asFragment } = render(test); - expect(asFragment()).toMatchSnapshot(); - }); - test('Verify bottom shadow', () => { - const { asFragment } = render(test); - expect(asFragment()).toMatchSnapshot(); - }); - test('Verify overflow scroll', () => { - const { asFragment } = render(test); - expect(asFragment()).toMatchSnapshot(); - }); - - // Old snapshot tests end here. The following tests can be kept if Page test suites need a revamp - test('Renders without an aria-label by default', () => { - render(test); - - expect(screen.getByText('test')).not.toHaveAccessibleName('Test label'); - }); - - test('Renders with the passed aria-label applied', () => { - render( - - test - - ); - - expect(screen.getByText('test').parentElement).toHaveAccessibleName('Test label'); - }); - - test('Does not log a warning in the console by default', () => { - const consoleWarning = jest.spyOn(console, 'warn').mockImplementation(); - - render(test); - - expect(consoleWarning).not.toHaveBeenCalled(); - }); - - test('Does not log a warning in the console when an aria-label is included with hasOverflowScroll', () => { - const consoleWarning = jest.spyOn(console, 'warn').mockImplementation(); - - render( - - test - - ); - - expect(consoleWarning).not.toHaveBeenCalled(); - }); - - test('Logs a warning in the console when an aria-label is not included with hasOverflowScroll', () => { - const consoleWarning = jest.spyOn(console, 'warn').mockImplementation(); - - render(test); - - expect(consoleWarning).toHaveBeenCalled(); - }); - - test('Renders with PageMainBody wrapper by default', () => { - render(test); - - expect(screen.getByText('test')).toHaveClass(styles.pageMainBody); - }); - test('Does not render with PageMainBody wrapper when hasMainBodyWrapper is false', () => { - render(test); - - expect(screen.getByText('test')).not.toHaveClass(styles.pageMainBody); - }); -}); diff --git a/packages/react-core/src/components/Page/__tests__/__snapshots__/Page.test.tsx.snap b/packages/react-core/src/components/Page/__tests__/__snapshots__/Page.test.tsx.snap index 55caf19c94a..b3d5b4ebe6e 100644 --- a/packages/react-core/src/components/Page/__tests__/__snapshots__/Page.test.tsx.snap +++ b/packages/react-core/src/components/Page/__tests__/__snapshots__/Page.test.tsx.snap @@ -499,108 +499,100 @@ exports[`Page Check page to verify grouped nav and breadcrumb - new components s
-
-
- -
-
+ Server + + + + +
+
+
-
- + + + + + Section Title + + +
  • + + + + + Section Landing + +
    • + + +
    +
    -
    +
    - -
    -
    -
    - Section with default background -
    -
    - - - - -`; - -exports[`Page Check page vertical layout example against snapshot 1`] = ` - -
    -
    -
    -
    - Navigation -
    -
    -
    -
    -
    + + + + Section Title + + +
  • + + + + + Section Landing + +
    • + + +
    +
    + +
    +
    + Section with default background +
    +
    + + + +     +`; + +exports[`Page Check page vertical layout example against snapshot 1`] = ` + +
    +
    +
    +
    + Navigation +
    +
    +
    +
    +
    - + + + + + Section Title + + +
  • + + + + + Section Landing + +
    • + + +
    +
    - + + + + + Section Title + + +
  • + + + + + Section Landing + +
    • + + +
    +
    Date: Mon, 15 Jul 2024 08:55:52 -0400 Subject: [PATCH 08/10] Renamed test file --- .../Page/__tests__/{PageMainBody.test.tsx => PageBody.test.tsx} | 0 .../{PageMainBody.test.tsx.snap => PageBody.test.tsx.snap} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename packages/react-core/src/components/Page/__tests__/{PageMainBody.test.tsx => PageBody.test.tsx} (100%) rename packages/react-core/src/components/Page/__tests__/__snapshots__/{PageMainBody.test.tsx.snap => PageBody.test.tsx.snap} (100%) diff --git a/packages/react-core/src/components/Page/__tests__/PageMainBody.test.tsx b/packages/react-core/src/components/Page/__tests__/PageBody.test.tsx similarity index 100% rename from packages/react-core/src/components/Page/__tests__/PageMainBody.test.tsx rename to packages/react-core/src/components/Page/__tests__/PageBody.test.tsx diff --git a/packages/react-core/src/components/Page/__tests__/__snapshots__/PageMainBody.test.tsx.snap b/packages/react-core/src/components/Page/__tests__/__snapshots__/PageBody.test.tsx.snap similarity index 100% rename from packages/react-core/src/components/Page/__tests__/__snapshots__/PageMainBody.test.tsx.snap rename to packages/react-core/src/components/Page/__tests__/__snapshots__/PageBody.test.tsx.snap From 12d2327c9809011a8f9123b0a25421184c1114d0 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Mon, 15 Jul 2024 13:32:26 -0400 Subject: [PATCH 09/10] Updated logic to always render PageBody for page nav --- packages/react-core/src/components/Page/Page.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/react-core/src/components/Page/Page.tsx b/packages/react-core/src/components/Page/Page.tsx index 017f674c113..735d50550ba 100644 --- a/packages/react-core/src/components/Page/Page.tsx +++ b/packages/react-core/src/components/Page/Page.tsx @@ -264,14 +264,12 @@ class Page extends React.Component { }; let nav = null; - if (horizontalSubnav && isHorizontalSubnavWidthLimited) { + if (horizontalSubnav) { nav = ( -
    +
    {horizontalSubnav}
    ); - } else if (horizontalSubnav) { - nav =
    {horizontalSubnav}
    ; } const crumb = breadcrumb ? ( From 68b1c3244b213ea100cbf32099e56fcbc51ed8e3 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Mon, 15 Jul 2024 14:02:11 -0400 Subject: [PATCH 10/10] Updated tests --- .../__snapshots__/Page.test.tsx.snap | 166 +++++++++--------- .../integration/wizarddeprecated.spec.ts | 6 +- .../WizardDeprecatedDemo.tsx | 2 +- 3 files changed, 91 insertions(+), 83 deletions(-) diff --git a/packages/react-core/src/components/Page/__tests__/__snapshots__/Page.test.tsx.snap b/packages/react-core/src/components/Page/__tests__/__snapshots__/Page.test.tsx.snap index 6cf5bf59d25..1581ee45ee7 100644 --- a/packages/react-core/src/components/Page/__tests__/__snapshots__/Page.test.tsx.snap +++ b/packages/react-core/src/components/Page/__tests__/__snapshots__/Page.test.tsx.snap @@ -648,100 +648,104 @@ exports[`Page Check page to verify grouped nav and breadcrumb - old / props synt
    - + + Server + + + + + +
    { it('Verify wizard in modal launches in a dialog and has a custom width', () => { cy.get('#launchWiz').click(); cy.get('#modalWizId.pf-v6-c-wizard').should('exist'); - cy.get('.pf-v6-c-modal-box').should('have.attr', 'style', '--pf-v6-c-modal-box--Width:710px;'); + cy.get('.pf-v6-c-modal-box').then((modalBox) => { + expect(modalBox) + .to.have.attr('style') + .match(/--pf-v6-c-modal-box--Width:\s*710px;/); + }); cy.focused().click(); }); diff --git a/packages/react-integration/demo-app-ts/src/components/demos/WizardDeprecatedDemo/WizardDeprecatedDemo.tsx b/packages/react-integration/demo-app-ts/src/components/demos/WizardDeprecatedDemo/WizardDeprecatedDemo.tsx index f4034caaa41..91119986abf 100644 --- a/packages/react-integration/demo-app-ts/src/components/demos/WizardDeprecatedDemo/WizardDeprecatedDemo.tsx +++ b/packages/react-integration/demo-app-ts/src/components/demos/WizardDeprecatedDemo/WizardDeprecatedDemo.tsx @@ -210,7 +210,7 @@ export class WizardDeprecatedDemo extends React.Component