Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions packages/react-core/src/components/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ export interface ButtonProps extends Omit<React.HTMLProps<HTMLButtonElement>, 'r
hamburgerVariant?: 'expand' | 'collapse';
/** @beta Flag indicating the button is a circle button. Intended for buttons that only contain an icon.. */
isCircle?: boolean;
/** @beta Flag indicating the button is a docked variant button. For use in docked navigation. */
isDocked?: boolean;
/** @beta Flag indicating the dock button should display text. Only applies when isDock is true. */
isTextExpanded?: boolean;
/** @hide Forwarded ref */
innerRef?: React.Ref<any>;
/** Adds count number to button */
Expand All @@ -134,6 +138,8 @@ const ButtonBase: React.FunctionComponent<ButtonProps> = ({
isHamburger,
hamburgerVariant,
isCircle,
isDocked = false,
isTextExpanded = false,
spinnerAriaValueText,
spinnerAriaLabelledBy,
spinnerAriaLabel,
Expand Down Expand Up @@ -265,6 +271,8 @@ const ButtonBase: React.FunctionComponent<ButtonProps> = ({
size === ButtonSize.sm && styles.modifiers.small,
size === ButtonSize.lg && styles.modifiers.displayLg,
isCircle && styles.modifiers.circle,
isDocked && styles.modifiers.dock, // Replace wwith docked class from https://github.com/patternfly/patternfly/pull/8308
isTextExpanded && styles.modifiers.textExpanded,
className
)}
disabled={isButtonElement ? isDisabled : null}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,39 @@ describe('Favorite button', () => {
});
});

describe('Dock variant', () => {
test(`Renders with class ${styles.modifiers.dock} when isDocked = true`, () => {
render(<Button isDocked>Dock Button</Button>);
expect(screen.getByRole('button')).toHaveClass(styles.modifiers.dock);
});

test(`Does not render with class ${styles.modifiers.dock} when isDocked is not passed`, () => {
render(<Button>Button</Button>);
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.dock);
});

test(`Renders with class ${styles.modifiers.textExpanded} when isTextExpanded = true`, () => {
render(<Button isTextExpanded>Text Expanded Button</Button>);
expect(screen.getByRole('button')).toHaveClass(styles.modifiers.textExpanded);
});

test(`Does not render with class ${styles.modifiers.textExpanded} when isTextExpanded is not passed`, () => {
render(<Button>Button</Button>);
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.textExpanded);
});

test(`Renders with both ${styles.modifiers.dock} and ${styles.modifiers.textExpanded} when both props are true`, () => {
render(
<Button isDocked isTextExpanded>
Dock Text Expanded Button
</Button>
);
const button = screen.getByRole('button');
expect(button).toHaveClass(styles.modifiers.dock);
expect(button).toHaveClass(styles.modifiers.textExpanded);
});
});

test(`Renders basic button`, () => {
const { asFragment } = render(<Button aria-label="basic button">Basic Button</Button>);
expect(asFragment()).toMatchSnapshot();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ exports[`Renders basic button 1`] = `
<button
aria-label="basic button"
class="pf-v6-c-button pf-m-primary"
data-ouia-component-id="OUIA-Generated-Button-primary-:r30:"
data-ouia-component-id="OUIA-Generated-Button-primary-:r35:"
data-ouia-component-type="PF6/Button"
data-ouia-safe="true"
type="button"
Expand Down
9 changes: 8 additions & 1 deletion packages/react-core/src/components/Masthead/MastheadLogo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,15 @@ export interface MastheadLogoProps extends React.DetailedHTMLProps<
className?: string;
/** Component type of the masthead logo. */
component?: React.ElementType<any> | React.ComponentType<any>;
/** @beta Flag indicating the logo is a compact variant. Used in docked layouts. */
isCompact?: boolean;
}

export const MastheadLogo: React.FunctionComponent<MastheadLogoProps> = ({
children,
className,
component,
isCompact = false,
...props
}: MastheadLogoProps) => {
let Component = component as any;
Expand All @@ -28,7 +31,11 @@ export const MastheadLogo: React.FunctionComponent<MastheadLogoProps> = ({
}
}
return (
<Component className={css(styles.mastheadLogo, className)} {...(Component === 'a' && { tabIndex: 0 })} {...props}>
<Component
className={css(styles.mastheadLogo, isCompact && styles.modifiers.compact, className)}
{...(Component === 'a' && { tabIndex: 0 })}
{...props}
>
{children}
</Component>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,29 @@ describe('MastheadLogo', () => {

expect(asFragment()).toMatchSnapshot();
});

test(`Renders with ${styles.modifiers.compact} class when isCompact is true`, () => {
render(
<MastheadLogo isCompact data-testid="compact-logo">
test
</MastheadLogo>
);
expect(screen.getByTestId('compact-logo')).toHaveClass(styles.modifiers.compact);
});

test(`Does not render with ${styles.modifiers.compact} class when isCompact is false`, () => {
render(
<MastheadLogo isCompact={false} data-testid="logo">
test
</MastheadLogo>
);
expect(screen.getByTestId('logo')).not.toHaveClass(styles.modifiers.compact);
});

test(`Does not render with ${styles.modifiers.compact} class when isCompact is not passed`, () => {
render(<MastheadLogo data-testid="logo">test</MastheadLogo>);
expect(screen.getByTestId('logo')).not.toHaveClass(styles.modifiers.compact);
});
});

describe('MastheadContent', () => {
Expand Down
10 changes: 10 additions & 0 deletions packages/react-core/src/components/MenuToggle/MenuToggle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ export interface MenuToggleProps
isCircle?: boolean;
/** Flag indicating whether the toggle is a settings toggle. This will override the icon property */
isSettings?: boolean;
/** @beta Flag indicating the menu toggle is a docked variant. For use in docked navigation. */
isDocked?: boolean;
/** @beta Flag indicating the dock toggle should display text. Only applies when isDock is true. */
isTextExpanded?: boolean;
/** Elements to display before the toggle button. When included, renders the menu toggle as a split button. */
splitButtonItems?: React.ReactNode[];
/** Variant styles of the menu toggle */
Expand Down Expand Up @@ -85,6 +89,8 @@ class MenuToggleBase extends Component<MenuToggleProps> {
isFullHeight: false,
isPlaceholder: false,
isCircle: false,
isDocked: false,
isTextExpanded: false,
size: 'default',
ouiaSafe: true
};
Expand All @@ -102,6 +108,8 @@ class MenuToggleBase extends Component<MenuToggleProps> {
isPlaceholder,
isCircle,
isSettings,
isDocked,
isTextExpanded,
splitButtonItems,
variant,
status,
Expand Down Expand Up @@ -185,6 +193,8 @@ class MenuToggleBase extends Component<MenuToggleProps> {
isDisabled && styles.modifiers.disabled,
isPlaceholder && styles.modifiers.placeholder,
isSettings && styles.modifiers.settings,
isDocked && styles.modifiers.dock, // Replace wwith docked class from https://github.com/patternfly/patternfly/pull/8308
isTextExpanded && styles.modifiers.textExpanded,
size === MenuToggleSize.sm && styles.modifiers.small,
className
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,34 @@ test('Does not render custom icon when icon prop and isSettings are passed', ()
);
expect(screen.queryByText('Custom icon')).not.toBeInTheDocument();
});

test(`Renders with class ${styles.modifiers.dock} when isDocked is passed`, () => {
render(<MenuToggle isDocked>Dock Toggle</MenuToggle>);
expect(screen.getByRole('button')).toHaveClass(styles.modifiers.dock);
});

test(`Does not render with class ${styles.modifiers.dock} when isDocked is not passed`, () => {
render(<MenuToggle>Toggle</MenuToggle>);
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.dock);
});

test(`Renders with class ${styles.modifiers.textExpanded} when isTextExpanded is passed`, () => {
render(<MenuToggle isTextExpanded>Text Expanded Toggle</MenuToggle>);
expect(screen.getByRole('button')).toHaveClass(styles.modifiers.textExpanded);
});

test(`Does not render with class ${styles.modifiers.textExpanded} when isTextExpanded is not passed`, () => {
render(<MenuToggle>Toggle</MenuToggle>);
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.textExpanded);
});

test(`Renders with both ${styles.modifiers.dock} and ${styles.modifiers.textExpanded} when both props are passed`, () => {
render(
<MenuToggle isDocked isTextExpanded>
Dock Text Expanded Toggle
</MenuToggle>
);
const button = screen.getByRole('button');
expect(button).toHaveClass(styles.modifiers.dock);
expect(button).toHaveClass(styles.modifiers.textExpanded);
});
4 changes: 4 additions & 0 deletions packages/react-core/src/components/Nav/Nav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ export interface NavProps
'aria-label'?: string;
/** The nav variant to use. Docked is in beta. */
variant?: 'default' | 'horizontal' | 'horizontal-subnav' | 'docked';
/** @beta Flag indicating the docked nav should display text. Only applies when variant is docked. */
isTextExpanded?: boolean;
/** Value to overwrite the randomly generated data-ouia-component-id.*/
ouiaId?: number | string;
/** Set the value of data-ouia-safe. Only set to true when the component is in a static state, i.e. no animations are occurring. At all other times, this value must be false. */
Expand Down Expand Up @@ -119,6 +121,7 @@ class Nav extends Component<NavProps, { isScrollable: boolean; flyoutRef: React.
ouiaId,
ouiaSafe,
variant,
isTextExpanded = false,
...props
} = this.props;
const isHorizontal = ['horizontal', 'horizontal-subnav'].includes(variant);
Expand Down Expand Up @@ -156,6 +159,7 @@ class Nav extends Component<NavProps, { isScrollable: boolean; flyoutRef: React.
isHorizontal && styles.modifiers.horizontal,
variant === 'docked' && styles.modifiers.docked,
variant === 'horizontal-subnav' && styles.modifiers.subnav,
isTextExpanded && styles.modifiers.textExpanded,
this.state.isScrollable && styles.modifiers.scrollable,
className
)}
Expand Down
30 changes: 30 additions & 0 deletions packages/react-core/src/components/Nav/__tests__/Nav.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -274,4 +274,34 @@ describe('Nav', () => {
);
expect(screen.getByTestId('docked-nav')).toHaveClass(styles.modifiers.docked);
});

test(`Renders with ${styles.modifiers.textExpanded} class when isTextExpanded is true`, () => {
renderNav(
<Nav isTextExpanded data-testid="text-expanded-nav">
<NavList>
{props.items.map((item) => (
<NavItem to={item.to} key={item.to}>
{item.label}
</NavItem>
))}
</NavList>
</Nav>
);
expect(screen.getByTestId('text-expanded-nav')).toHaveClass(styles.modifiers.textExpanded);
});

test(`Does not render with ${styles.modifiers.textExpanded} class when isTextExpanded is not passed`, () => {
renderNav(
<Nav data-testid="nav">
<NavList>
{props.items.map((item) => (
<NavItem to={item.to} key={item.to}>
{item.label}
</NavItem>
))}
</NavList>
</Nav>
);
expect(screen.getByTestId('nav')).not.toHaveClass(styles.modifiers.textExpanded);
});
Comment on lines +278 to +306
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

isTextExpanded tests should run in docked variant context.

These tests currently validate isTextExpanded on a non-docked Nav, which conflicts with the docked-only contract and risks cementing unsupported behavior.

Suggested test correction
-  test(`Renders with ${styles.modifiers.textExpanded} class when isTextExpanded is true`, () => {
+  test(`Renders with ${styles.modifiers.textExpanded} class when variant is docked and isTextExpanded is true`, () => {
     renderNav(
-      <Nav isTextExpanded data-testid="text-expanded-nav">
+      <Nav variant="docked" isTextExpanded data-testid="text-expanded-nav">
         <NavList>
           {props.items.map((item) => (
             <NavItem to={item.to} key={item.to}>
               {item.label}
             </NavItem>
           ))}
         </NavList>
       </Nav>
     );
     expect(screen.getByTestId('text-expanded-nav')).toHaveClass(styles.modifiers.textExpanded);
   });
 
-  test(`Does not render with ${styles.modifiers.textExpanded} class when isTextExpanded is not passed`, () => {
+  test(`Does not render with ${styles.modifiers.textExpanded} class when variant is docked and isTextExpanded is not passed`, () => {
     renderNav(
-      <Nav data-testid="nav">
+      <Nav variant="docked" data-testid="nav">
         <NavList>
           {props.items.map((item) => (
             <NavItem to={item.to} key={item.to}>
               {item.label}
             </NavItem>
           ))}
         </NavList>
       </Nav>
     );
     expect(screen.getByTestId('nav')).not.toHaveClass(styles.modifiers.textExpanded);
   });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test(`Renders with ${styles.modifiers.textExpanded} class when isTextExpanded is true`, () => {
renderNav(
<Nav isTextExpanded data-testid="text-expanded-nav">
<NavList>
{props.items.map((item) => (
<NavItem to={item.to} key={item.to}>
{item.label}
</NavItem>
))}
</NavList>
</Nav>
);
expect(screen.getByTestId('text-expanded-nav')).toHaveClass(styles.modifiers.textExpanded);
});
test(`Does not render with ${styles.modifiers.textExpanded} class when isTextExpanded is not passed`, () => {
renderNav(
<Nav data-testid="nav">
<NavList>
{props.items.map((item) => (
<NavItem to={item.to} key={item.to}>
{item.label}
</NavItem>
))}
</NavList>
</Nav>
);
expect(screen.getByTestId('nav')).not.toHaveClass(styles.modifiers.textExpanded);
});
test(`Renders with ${styles.modifiers.textExpanded} class when variant is docked and isTextExpanded is true`, () => {
renderNav(
<Nav variant="docked" isTextExpanded data-testid="text-expanded-nav">
<NavList>
{props.items.map((item) => (
<NavItem to={item.to} key={item.to}>
{item.label}
</NavItem>
))}
</NavList>
</Nav>
);
expect(screen.getByTestId('text-expanded-nav')).toHaveClass(styles.modifiers.textExpanded);
});
test(`Does not render with ${styles.modifiers.textExpanded} class when variant is docked and isTextExpanded is not passed`, () => {
renderNav(
<Nav variant="docked" data-testid="nav">
<NavList>
{props.items.map((item) => (
<NavItem to={item.to} key={item.to}>
{item.label}
</NavItem>
))}
</NavList>
</Nav>
);
expect(screen.getByTestId('nav')).not.toHaveClass(styles.modifiers.textExpanded);
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/react-core/src/components/Nav/__tests__/Nav.test.tsx` around lines
278 - 306, The tests for isTextExpanded are asserting docked-only behavior but
render a non-docked Nav; update both tests to render the Nav in the docked
variant context so they validate the correct contract—e.g. in the two tests that
call renderNav and render a <Nav ...> component, include the docked variant
(pass variant="docked" or the equivalent docked prop your Nav API accepts) when
constructing the Nav (the tests referencing Nav, isTextExpanded, renderNav, and
styles.modifiers.textExpanded) so the assertions run against a docked Nav.

});
26 changes: 22 additions & 4 deletions packages/react-core/src/components/Page/Page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,14 @@ export interface PageProps extends React.HTMLProps<HTMLDivElement> {
className?: string;
/** @beta Indicates the layout variant */
variant?: 'default' | 'docked';
/** Masthead component (e.g. <Masthead />) */
/** @beta Flag indicating the dock nav is expanded on mobile. Only applies when variant is dock. */
isDockExpanded?: boolean;
/** @beta Flag indicating the dock nav should display text. Only applies when variant is dock. */
isDockTextExpanded?: boolean;
/** The horizontal masthead content (e.g. <Masthead />). When using the dock variant, this content will only render at mobile viewports. */
masthead?: React.ReactNode;
/** @beta Content to render in the vertical dock when variant of dock is used. At mobile viewports, this content will be replaced with the content passed to masthead. */
dockContent?: React.ReactNode;
/** Sidebar component for a side nav, recommended to be a PageSidebar. If set to null, the page grid layout
* will render without a sidebar.
*/
Expand Down Expand Up @@ -232,7 +238,10 @@ class Page extends Component<PageProps, PageState> {
className,
children,
variant,
isDockExpanded = false,
isDockTextExpanded = false,
masthead,
dockContent,
sidebar,
notificationDrawer,
isNotificationDrawerExpanded,
Expand Down Expand Up @@ -349,9 +358,18 @@ class Page extends Component<PageProps, PageState> {
>
{skipToContent}
{variant === 'docked' ? (
<div className={css(styles.pageDock)}>
<div className={css(styles.pageDockMain)}>{masthead}</div>
</div>
<>
{masthead}
<div
className={css(
styles.pageDock,
isDockExpanded && styles.modifiers.expanded,
isDockTextExpanded && styles.modifiers.textExpanded
)}
>
<div className={css(styles.pageDockMain)}>{dockContent}</div>
</div>
</>
) : (
masthead
)}
Expand Down
Loading
Loading