PR feedback

packages/react-core/src/components/Page/Page.tsx

@@ -4,12 +4,12 @@ import { css } from '@patternfly/react-styles';
 import globalBreakpointXl from '@patternfly/react-tokens/dist/esm/global_breakpoint_xl';
 import { debounce, canUseDOM } from '../../helpers/util';
 import { Drawer, DrawerContent, DrawerContentBody, DrawerPanelContent } from '../Drawer';
-import { PageBreadcrumbProps } from './PageBreadcrumb';
+import { PageBreadcrumb, PageBreadcrumbProps } from './PageBreadcrumb';
 import { PageGroup, PageGroupProps } from './PageGroup';
 import { getResizeObserver } from '../../helpers/resizeObserver';
-import { formatBreakpointMods, getBreakpoint, getVerticalBreakpoint } from '../../helpers/util';
+import { getBreakpoint, getVerticalBreakpoint } from '../../helpers/util';
 import { PageContextProvider } from './PageContext';
-import { PageMainBody } from './PageMainBody';
+import { PageBody } from './PageBody';
 
 export enum PageLayouts {
   vertical = 'vertical',
@@ -267,29 +267,20 @@ class Page extends React.Component<PageProps, PageState> {
     if (horizontalSubnav && isHorizontalSubnavWidthLimited) {
       nav = (
         <div className={css(styles.pageMainSubnav, styles.modifiers.limitWidth)}>
-          <PageMainBody>{horizontalSubnav}</PageMainBody>
+          <PageBody>{horizontalSubnav}</PageBody>
         </div>
       );
     } else if (horizontalSubnav) {
       nav = <div className={css(styles.pageMainSubnav)}>{horizontalSubnav}</div>;
     }
 
     const crumb = breadcrumb ? (
-      <section
-        className={css(
-          styles.pageMainBreadcrumb,
-          isBreadcrumbWidthLimited && styles.modifiers.limitWidth,
-          formatBreakpointMods(
-            breadcrumbProps?.stickyOnBreakpoint,
-            styles,
-            'sticky-',
-            getVerticalBreakpoint(height),
-            true
-          )
-        )}
+      <PageBreadcrumb
+        stickyOnBreakpoint={breadcrumbProps?.stickyOnBreakpoint}
+        isWidthLimited={isBreadcrumbWidthLimited}
       >
-        {isBreadcrumbWidthLimited ? <PageMainBody>{breadcrumb}</PageMainBody> : breadcrumb}
-      </section>
+        <PageBody>{breadcrumb}</PageBody>
+      </PageBreadcrumb>
     ) : null;
 
     const isGrouped = isHorizontalSubnavGrouped || isBreadcrumbGrouped || additionalGroupedContent;

packages/react-core/src/components/Page/PageMainBody.tsx → packages/react-core/src/components/Page/PageBody.tsx

@@ -2,22 +2,17 @@ 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<HTMLDivElement> {
+export interface PageBodyProps extends React.HTMLProps<HTMLDivElement> {
   /** 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<PageMainBodyProps> = ({
-  className,
-  children,
-  ...props
-}: PageMainBodyProps) => (
+export const PageBody: React.FunctionComponent<PageBodyProps> = ({ className, children, ...props }: PageBodyProps) => (
   <div {...props} className={css(styles.pageMainBody, className)}>
     {children}
   </div>
 );
 
-PageMainBody.displayName = 'PageMainBody';
+PageBody.displayName = 'PageBody';

packages/react-core/src/components/Page/PageBreadcrumb.tsx

@@ -3,7 +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';
+import { PageBody } from './PageBody';
 
 export interface PageBreadcrumbProps extends React.HTMLProps<HTMLElement> {
   /** Additional classes to apply to the PageBreadcrumb */
@@ -27,10 +27,10 @@ export interface PageBreadcrumbProps extends React.HTMLProps<HTMLElement> {
   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.
+  /** @beta Flag indicating whether children passed to the component should be wrapped by a PageBody.
+   * Set this to false in order to pass multiple, custom PageBody's as children.
    */
-  hasMainBodyWrapper?: boolean;
+  hasBodyWrapper?: boolean;
   /** Adds an accessible name to the breadcrumb section. Required when the hasOverflowScroll prop is set to true. */
   'aria-label'?: string;
 }
@@ -44,7 +44,7 @@ export const PageBreadcrumb = ({
   hasShadowBottom = false,
   hasOverflowScroll = false,
   'aria-label': ariaLabel,
-  hasMainBodyWrapper = true,
+  hasBodyWrapper = true,
   ...props
 }: PageBreadcrumbProps) => {
   const { height, getVerticalBreakpoint } = React.useContext(PageContext);
@@ -71,7 +71,7 @@ export const PageBreadcrumb = ({
       aria-label={ariaLabel}
       {...props}
     >
-      {hasMainBodyWrapper ? <PageMainBody>{children}</PageMainBody> : children}
+      {hasBodyWrapper ? <PageBody>{children}</PageBody> : children}
     </section>
   );
 };

packages/react-core/src/components/Page/PageSection.tsx

@@ -3,7 +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';
+import { PageBody } from './PageBody';
 
 export enum PageSectionVariants {
   default = 'default',
@@ -57,10 +57,10 @@ export interface PageSectionProps extends React.HTMLProps<HTMLDivElement> {
   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.
+  /** @beta Flag indicating whether children passed to the component should be wrapped by a PageBody.
+   * Set this to false in order to pass multiple, custom PageBody's as children.
    */
-  hasMainBodyWrapper?: boolean;
+  hasBodyWrapper?: 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.
    */
@@ -97,7 +97,7 @@ export const PageSection: React.FunctionComponent<PageSectionProps> = ({
   hasOverflowScroll = false,
   'aria-label': ariaLabel,
   component = 'section',
-  hasMainBodyWrapper = true,
+  hasBodyWrapper = true,
   ...props
 }: PageSectionProps) => {
   const { height, getVerticalBreakpoint } = React.useContext(PageContext);
@@ -131,7 +131,7 @@ export const PageSection: React.FunctionComponent<PageSectionProps> = ({
       {...(hasOverflowScroll && { tabIndex: 0 })}
       aria-label={ariaLabel}
     >
-      {hasMainBodyWrapper ? <PageMainBody>{children}</PageMainBody> : children}
+      {hasBodyWrapper ? <PageBody>{children}</PageBody> : children}
     </Component>
   );
 };

packages/react-core/src/components/Page/__tests__/PageBreadcrumb.test.tsx

@@ -74,13 +74,13 @@ describe('page breadcrumb', () => {
     expect(consoleWarning).toHaveBeenCalled();
   });
 
-  test('Renders with PageMainBody wrapper by default', () => {
+  test('Renders with PageBody wrapper by default', () => {
     render(<PageBreadcrumb>test</PageBreadcrumb>);
 
     expect(screen.getByText('test')).toHaveClass(styles.pageMainBody);
   });
-  test('Does not render with PageMainBody wrapper when hasMainBodyWrapper is false', () => {
-    render(<PageBreadcrumb hasMainBodyWrapper={false}>test</PageBreadcrumb>);
+  test('Does not render with PageBody wrapper when hasBodyWrapper is false', () => {
+    render(<PageBreadcrumb hasBodyWrapper={false}>test</PageBreadcrumb>);
 
     expect(screen.getByText('test')).not.toHaveClass(styles.pageMainBody);
   });

packages/react-core/src/components/Page/__tests__/PageMainBody.test.tsx

@@ -1,34 +1,34 @@
 import * as React from 'react';
 import { render, screen } from '@testing-library/react';
-import { PageMainBody } from '../PageMainBody';
+import { PageBody } from '../PageBody';
 import styles from '@patternfly/react-styles/css/components/Page/page';
 
 test('Renders without children', () => {
-  render(<PageMainBody data-testid="page-main-body" />);
+  render(<PageBody data-testid="page-main-body" />);
 
   expect(screen.getByTestId('page-main-body')).toBeVisible();
 });
 test('Renders children', () => {
-  render(<PageMainBody>Test</PageMainBody>);
+  render(<PageBody>Test</PageBody>);
 
   expect(screen.getByText('Test')).toBeVisible();
 });
 test(`Renders with class ${styles.pageMainBody} by default`, () => {
-  render(<PageMainBody>Test</PageMainBody>);
+  render(<PageBody>Test</PageBody>);
 
   expect(screen.getByText('Test')).toHaveClass(styles.pageMainBody);
 });
 test(`Renders with custom classes when className is passed`, () => {
-  render(<PageMainBody className="custom-class">Test</PageMainBody>);
+  render(<PageBody className="custom-class">Test</PageBody>);
 
   expect(screen.getByText('Test')).toHaveClass('custom-class');
 });
 test(`Renders with spread props`, () => {
-  render(<PageMainBody id="custom-id">Test</PageMainBody>);
+  render(<PageBody id="custom-id">Test</PageBody>);
 
   expect(screen.getByText('Test')).toHaveAttribute('id', 'custom-id');
 });
 test('Matches snapshot', () => {
-  const { asFragment } = render(<PageMainBody>Test</PageMainBody>);
+  const { asFragment } = render(<PageBody>Test</PageBody>);
   expect(asFragment()).toMatchSnapshot();
 });

packages/react-core/src/components/Page/__tests__/PageSection.test.tsx

@@ -143,13 +143,13 @@ 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', () => {
+test('Renders with PageBody wrapper by default', () => {
   render(<PageSection>test</PageSection>);
 
   expect(screen.getByText('test')).toHaveClass(styles.pageMainBody);
 });
-test('Does not render with PageMainBody wrapper when hasMainBodyWrapper is false', () => {
-  render(<PageSection hasMainBodyWrapper={false}>test</PageSection>);
+test('Does not render with PageBody wrapper when hasBodyWrapper is false', () => {
+  render(<PageSection hasBodyWrapper={false}>test</PageSection>);
 
   expect(screen.getByText('test')).not.toHaveClass(styles.pageMainBody);
 });

packages/react-core/src/components/Page/index.ts

@@ -1,5 +1,5 @@
 export * from './Page';
-export * from './PageMainBody';
+export * from './PageBody';
 export * from './PageBreadcrumb';
 export * from './PageGroup';
 export * from './PageSidebar';