feat(Navigation): Penta updates for nav (#9948)

* feat(Navigation): Penta updates for nav
patternfly · Jan 16, 2024 · dba392d · dba392d
1 parent a90b9d4
commit dba392d
Show file tree

Hide file tree

Showing 29 changed files with 408 additions and 926 deletions.
diff --git a/packages/react-core/package.json b/packages/react-core/package.json
@@ -54,7 +54,7 @@
     "tslib": "^2.5.0"
   },
   "devDependencies": {
-    "@patternfly/patternfly": "6.0.0-alpha.49",
+    "@patternfly/patternfly": "6.0.0-alpha.51",
     "@rollup/plugin-commonjs": "^25.0.0",
     "@rollup/plugin-node-resolve": "^15.0.2",
     "@rollup/plugin-replace": "^5.0.2",

diff --git a/packages/react-core/src/components/Nav/Nav.tsx b/packages/react-core/src/components/Nav/Nav.tsx
@@ -35,10 +35,8 @@ export interface NavProps
   ) => void;
   /** Accessible label for the nav when there are multiple navs on the page */
   'aria-label'?: string;
-  /** Indicates which theme color to use */
-  theme?: 'dark' | 'light';
   /** For horizontal navs */
-  variant?: 'default' | 'horizontal' | 'tertiary' | 'horizontal-subnav';
+  variant?: 'default' | 'horizontal' | 'horizontal-subnav';
   /** 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. */
@@ -72,7 +70,6 @@ class Nav extends React.Component<
   static defaultProps: NavProps = {
     onSelect: () => undefined,
     onToggle: () => undefined,
-    theme: 'dark',
     ouiaSafe: true
   };
 
@@ -121,13 +118,12 @@ class Nav extends React.Component<
       onSelect,
       // eslint-disable-next-line @typescript-eslint/no-unused-vars
       onToggle,
-      theme,
       ouiaId,
       ouiaSafe,
       variant,
       ...props
     } = this.props;
-    const isHorizontal = ['horizontal', 'tertiary'].includes(variant);
+    const isHorizontal = ['horizontal', 'horizontal-subnav'].includes(variant);
 
     return (
       <NavContext.Provider
@@ -148,7 +144,7 @@ class Nav extends React.Component<
           onToggle: (event: React.MouseEvent<HTMLButtonElement>, groupId: number | string, expanded: boolean) =>
             this.onToggle(event, groupId, expanded),
           updateIsScrollable: (isScrollable: boolean) => this.setState({ isScrollable }),
-          isHorizontal: ['horizontal', 'tertiary', 'horizontal-subnav'].includes(variant),
+          isHorizontal: ['horizontal', 'horizontal-subnav'].includes(variant),
           flyoutRef: this.state.flyoutRef,
           setFlyoutRef: (flyoutRef) => this.setState({ flyoutRef }),
           navRef: this.navRef
@@ -157,14 +153,12 @@ class Nav extends React.Component<
         <nav
           className={css(
             styles.nav,
-            theme === 'light' && styles.modifiers.light,
             isHorizontal && styles.modifiers.horizontal,
-            variant === 'tertiary' && styles.modifiers.tertiary,
-            variant === 'horizontal-subnav' && styles.modifiers.horizontalSubnav,
+            variant === 'horizontal-subnav' && styles.modifiers.subnav,
             this.state.isScrollable && styles.modifiers.scrollable,
             className
           )}
-          aria-label={ariaLabel || (['tertiary', 'horizontal-subnav'].includes(variant) ? 'Local' : 'Global')}
+          aria-label={ariaLabel || (variant === 'horizontal-subnav' ? 'Local' : 'Global')}
           ref={this.navRef}
           {...getOUIAProps(Nav.displayName, ouiaId !== undefined ? ouiaId : this.state.ouiaStateId, ouiaSafe)}
           {...props}

diff --git a/packages/react-core/src/components/Nav/NavExpandable.tsx b/packages/react-core/src/components/Nav/NavExpandable.tsx
@@ -115,7 +115,6 @@ class NavExpandable extends React.Component<NavExpandableProps, NavExpandableSta
           <li
             className={css(
               styles.navItem,
-              styles.modifiers.expandable,
               expandedState && styles.modifiers.expanded,
               isActive && styles.modifiers.current,
               className

diff --git a/packages/react-core/src/components/Nav/NavItem.tsx b/packages/react-core/src/components/Nav/NavItem.tsx
@@ -1,5 +1,7 @@
 import * as React from 'react';
 import styles from '@patternfly/react-styles/css/components/Nav/nav';
+import menuStyles from '@patternfly/react-styles/css/components/Menu/menu';
+import dividerStyles from '@patternfly/react-styles/css/components/Divider/divider';
 import { css } from '@patternfly/react-styles';
 import { NavContext, NavSelectClickHandler } from './Nav';
 import { PageSidebarContext } from '../Page/PageSidebar';
@@ -125,7 +127,7 @@ export const NavItem: React.FunctionComponent<NavItemProps> = ({
     // Otherwise, MenuItem should handle closing its flyouts
     if (
       (key === 'Escape' || key === 'ArrowLeft') &&
-      popperRef?.current?.querySelectorAll(`.${styles.menu}`).length === 1
+      popperRef?.current?.querySelectorAll(`.${menuStyles.menu}`).length === 1
     ) {
       if (flyoutVisible) {
         event.stopPropagation();
@@ -151,7 +153,7 @@ export const NavItem: React.FunctionComponent<NavItemProps> = ({
       if (flyoutVisible) {
         const flyoutItems = Array.from(
           (popperRef.current as HTMLElement).getElementsByTagName('UL')[0].children
-        ).filter((el) => !(el.classList.contains('pf-m-disabled') || el.classList.contains(styles.divider)));
+        ).filter((el) => !(el.classList.contains('pf-m-disabled') || el.classList.contains(dividerStyles.divider)));
         (flyoutItems[0].firstChild as HTMLElement).focus();
       } else {
         flyoutTarget.focus();

diff --git a/packages/react-core/src/components/Nav/NavList.tsx b/packages/react-core/src/components/Nav/NavList.tsx
@@ -1,6 +1,7 @@
 import * as React from 'react';
 import styles from '@patternfly/react-styles/css/components/Nav/nav';
 import { css } from '@patternfly/react-styles';
+import { Button } from '../Button';
 import AngleLeftIcon from '@patternfly/react-icons/dist/esm/icons/angle-left-icon';
 import AngleRightIcon from '@patternfly/react-icons/dist/esm/icons/angle-right-icon';
 import { getLanguageDirection, isElementInView } from '../../helpers/util';
@@ -143,15 +144,19 @@ class NavList extends React.Component<NavListProps> {
             {({ isSidebarOpen }) => (
               <React.Fragment>
                 {isHorizontal && (
-                  <button
-                    className={css(styles.navScrollButton)}
-                    aria-label={backScrollAriaLabel || ariaLeftScroll}
-                    onClick={this.scrollBack}
-                    disabled={scrollViewAtStart}
-                    tabIndex={isSidebarOpen ? null : -1}
-                  >
-                    <AngleLeftIcon />
-                  </button>
+                  <div className={css(styles.navScrollButton)}>
+                    <Button
+                      variant="plain"
+                      aria-label={backScrollAriaLabel || ariaLeftScroll}
+                      onClick={this.scrollBack}
+                      disabled={scrollViewAtStart}
+                      tabIndex={isSidebarOpen ? null : -1}
+                    >
+                      <span className={css(styles.buttonIcon)}>
+                        <AngleLeftIcon />
+                      </span>
+                    </Button>
+                  </div>
                 )}
                 <ul
                   ref={this.navList}
@@ -163,15 +168,19 @@ class NavList extends React.Component<NavListProps> {
                   {children}
                 </ul>
                 {isHorizontal && (
-                  <button
-                    className={css(styles.navScrollButton)}
-                    aria-label={forwardScrollAriaLabel || ariaRightScroll}
-                    onClick={this.scrollForward}
-                    disabled={scrollViewAtEnd}
-                    tabIndex={isSidebarOpen ? null : -1}
-                  >
-                    <AngleRightIcon />
-                  </button>
+                  <div className={css(styles.navScrollButton)}>
+                    <Button
+                      variant="plain"
+                      aria-label={forwardScrollAriaLabel || ariaRightScroll}
+                      onClick={this.scrollForward}
+                      disabled={scrollViewAtEnd}
+                      tabIndex={isSidebarOpen ? null : -1}
+                    >
+                      <span className={css(styles.buttonIcon)}>
+                        <AngleRightIcon />
+                      </span>
+                    </Button>
+                  </div>
                 )}
               </React.Fragment>
             )}

diff --git a/...act-core/src/components/Nav/__tests__/Generated/__snapshots__/NavExpandable.test.tsx.snap b/...act-core/src/components/Nav/__tests__/Generated/__snapshots__/NavExpandable.test.tsx.snap
@@ -3,7 +3,7 @@
 exports[`NavExpandable should match snapshot (auto-generated) 1`] = `
 <DocumentFragment>
   <li
-    class="pf-v5-c-nav__item pf-m-expandable ''"
+    class="pf-v5-c-nav__item ''"
     data-ouia-component-id="OUIA-Generated-NavExpandable-1"
     data-ouia-component-type="PF5/NavExpandable"
     data-ouia-safe="true"

diff --git a/packages/react-core/src/components/Nav/__tests__/Nav.test.tsx b/packages/react-core/src/components/Nav/__tests__/Nav.test.tsx
@@ -60,21 +60,6 @@ describe('Nav', () => {
     expect(asFragment()).toMatchSnapshot();
   });
 
-  test('Dark Nav List', () => {
-    const { asFragment } = renderNav(
-      <Nav className="test=nav-class" theme="dark">
-        <NavList className="test-nav-list-class">
-          {props.items.map((item) => (
-            <NavItem to={item.to} key={item.to} className="test-nav-item-class">
-              {item.label}
-            </NavItem>
-          ))}
-        </NavList>
-      </Nav>
-    );
-    expect(asFragment()).toMatchSnapshot();
-  });
-
   test('Default Nav List - Trigger item active update', async () => {
     const user = userEvent.setup();
 
@@ -228,24 +213,9 @@ describe('Nav', () => {
     expect(asFragment()).toMatchSnapshot();
   });
 
-  test('Tertiary Nav List', () => {
-    const { asFragment } = renderNav(
-      <Nav variant="tertiary">
-        <NavList>
-          {props.items.map((item) => (
-            <NavItem to={item.to} key={item.to}>
-              {item.label}
-            </NavItem>
-          ))}
-        </NavList>
-      </Nav>
-    );
-    expect(asFragment()).toMatchSnapshot();
-  });
-
   test('Nav List with custom item nodes', () => {
     const { asFragment } = renderNav(
-      <Nav variant="tertiary">
+      <Nav>
         <NavList>
           <NavItem to="/components/nav#link1" className="test-nav-item-class">
             <div className="my-custom-node">My custom node</div>
@@ -260,7 +230,7 @@ describe('Nav', () => {
     const user = userEvent.setup();
 
     const { asFragment } = renderNav(
-      <Nav variant="tertiary">
+      <Nav>
         <NavList>
           <NavItem className="test-nav-item-class" flyout={<div>Flyout test</div>}>
             <div className="my-custom-node">My custom node</div>