openshift · rhamilto · Aug 29, 2019
diff --git a/frontend/__tests__/components/cluster-settings.spec.tsx b/frontend/__tests__/components/cluster-settings.spec.tsx
@@ -78,12 +78,6 @@ describe('Cluster Settings page', () => {
         .at(0)
         .props().pages.length,
     ).toBe(3);
-    expect(
-      wrapper
-        .find(HorizontalNav)
-        .at(0)
-        .props().hideDivider,
-    ).toBe(true);
     expect(
       wrapper
         .find(HorizontalNav)

diff --git a/frontend/packages/dev-console/src/components/topology/TopologyApplicationResources.tsx b/frontend/packages/dev-console/src/components/topology/TopologyApplicationResources.tsx
@@ -33,7 +33,7 @@ const TopologyApplicationResources: React.FC<TopologyApplicationResourcesProps>
           'odc-application-resource-tab',
         )}
       >
-        <ul className="co-m-horizontal-nav__menu-primary">
+        <ul className="co-m-horizontal-nav__menu-list">
           <li className="co-m-horizontal-nav__menu-item">
             <button type="button">Resources</button>
           </li>

diff --git a/frontend/public/components/_horizontal-nav.scss b/frontend/public/components/_horizontal-nav.scss
@@ -1,12 +1,15 @@
-$co-m-horizontal-nav__menu-item-link-padding-lr: ($grid-gutter-width / 2);
-
 .co-m-horizontal-nav__menu {
   border-bottom: 1px solid $color-grey-background-border;
-  display: flex;
-  white-space: nowrap;
   @media (min-width: $grid-float-breakpoint) {
-    padding-left: $co-m-horizontal-nav__menu-item-link-padding-lr;
-    padding-right: ($co-m-horizontal-nav__menu-item-link-padding-lr * 2);
+    padding-left: ($grid-gutter-width / 2);
+    padding-right: $grid-gutter-width;
+  }
+  &--scrollable {
+    padding: 0 ($grid-gutter-width / 2);
+    @media (min-width: $grid-float-breakpoint) {
+      padding-left: $grid-gutter-width;
+      padding-right: $grid-gutter-width;
+    }
   }
   &--within-sidebar {
     border-top: 1px solid $color-grey-background-border;
@@ -16,6 +19,7 @@ $co-m-horizontal-nav__menu-item-link-padding-lr: ($grid-gutter-width / 2);
     @media (min-width: $grid-float-breakpoint) {
       margin-left: -($grid-gutter-width);
       margin-right: -($grid-gutter-width);
+      padding-right: ($grid-gutter-width / 2);
     }
     .co-m-horizontal-nav__menu-item {
       font-size: 16px;
@@ -28,23 +32,35 @@ $co-m-horizontal-nav__menu-item-link-padding-lr: ($grid-gutter-width / 2);
     padding-left: 5px;
     padding-right: 5px;
   }
+  .pf-c-nav__item {
+    margin-right: $grid-gutter-width !important;
+  }
+  // override PF4
+  .pf-c-nav__scroll-button {
+    height: 30px !important;
+    padding: 0 !important;
+    top: 5px !important;
+    width: $grid-gutter-width !important;
+    &:nth-of-type(1) {
+      left: -($grid-gutter-width / 2) !important;
+      @media (min-width: $grid-float-breakpoint) {
+        left: -($grid-gutter-width) !important;
+      }
+    }
+    &:nth-of-type(2) {
+      right: -($grid-gutter-width / 2) !important;
+      @media (min-width: $grid-float-breakpoint) {
+        right: -($grid-gutter-width) !important;
+      }
+    }
+  }
 }
 
-.co-m-horizontal-nav__menu-primary {
-  display: flex;
-  list-style: none;
-  margin: 0;
-  padding: 0;
-}
-
-.co-m-horizontal-nav__menu-secondary {
+.co-m-horizontal-nav__menu-list {
   display: flex;
   list-style: none;
-  margin: 0 0 -1px; // -1px bottom so co-m-horizontal-nav__menu-item *::after are fully visible
-  overflow-x: auto;
-  overflow-y: hidden;
-  padding: 0;
-  -webkit-overflow-scrolling: touch;
+  margin-bottom: 0;
+  padding-left: 0;
 }
 
 .co-m-horizontal-nav__menu-item {
@@ -53,14 +69,13 @@ $co-m-horizontal-nav__menu-item-link-padding-lr: ($grid-gutter-width / 2);
   button {
     background: transparent;
     border: 0;
-    color: $color-pf-black-700;
+    color: var(--pf-c-tabs__button--Color);
     display: block;
-    padding: 6px $co-m-horizontal-nav__menu-item-link-padding-lr 8px;
+    padding: 6px ($grid-gutter-width / 2) 8px;
     position: relative;
     text-decoration: none;
     &:hover,
     &:focus {
-      color: #252525;
       text-decoration: none;
       &::after {
         background: $color-pf-black-400;
@@ -71,9 +86,9 @@ $co-m-horizontal-nav__menu-item-link-padding-lr: ($grid-gutter-width / 2);
       content: "";
       display: block;
       height: 2px;
-      left: $co-m-horizontal-nav__menu-item-link-padding-lr;
+      left: ($grid-gutter-width / 2);
       position: absolute;
-      right: $co-m-horizontal-nav__menu-item-link-padding-lr;
+      right: ($grid-gutter-width / 2);
     }
   }
   &.co-m-horizontal-nav-item--active {
@@ -85,9 +100,33 @@ $co-m-horizontal-nav__menu-item-link-padding-lr: ($grid-gutter-width / 2);
       }
     }
   }
-}
-
-.co-m-horizontal-nav__menu-item--divider {
-  border-left: 1px solid $color-grey-background-border;
-  margin: 0 10px;
+  // override PF4
+  .pf-c-nav__link {
+    font-weight: var(--pf-global--FontWeight--normal) !important;
+    padding-left: 0;
+    padding-right: 0;
+    padding-top: 6px !important;
+    &::after {
+      height: 2px !important;
+    }
+    &:active,
+    &:focus,
+    &:hover {
+      color: var(--pf-c-tabs__button--Color);
+      &::after {
+        background-color: $color-pf-black-400 !important;
+      }
+    }
+    &.pf-m-current {
+      color: $link-color;
+      &:active,
+      &:focus,
+      &:hover {
+        color: $link-color;
+        &::after {
+          background-color: $link-color !important;
+        }
+      }
+    }
+  }
 }
diff --git a/frontend/public/components/broker-management.tsx b/frontend/public/components/broker-management.tsx
@@ -19,7 +19,7 @@ const pages = [
 export const BrokerManagementPage: React.SFC<BrokerManagementPageProps> = ({ match }) => (
   <>
     <PageHeading detail={true} title="Broker Management" />
-    <HorizontalNav pages={pages} match={match} hideDivider noStatusBox={true} />
+    <HorizontalNav pages={pages} match={match} noStatusBox={true} />
   </>
 );
 

diff --git a/frontend/public/components/chargeback.tsx b/frontend/public/components/chargeback.tsx
@@ -53,7 +53,6 @@ const ChargebackNavBar: React.SFC<{ match: { url: string } }> = (props) => (
         .split('/')
         .slice(0, -1)
         .join('/')}
-      hideDivider
     />
   </div>
 );

diff --git a/frontend/public/components/cluster-settings/cluster-settings.tsx b/frontend/public/components/cluster-settings/cluster-settings.tsx
@@ -411,7 +411,7 @@ export const ClusterSettingsPage: React.SFC<ClusterSettingsPageProps> = ({ match
         </h1>
       </div>
       <Firehose resources={resources}>
-        <HorizontalNav pages={pages} match={match} resourceKeys={resourceKeys} hideDivider />
+        <HorizontalNav pages={pages} match={match} resourceKeys={resourceKeys} />
       </Firehose>
     </>
   );

diff --git a/frontend/public/components/monitoring.tsx b/frontend/public/components/monitoring.tsx
@@ -1409,7 +1409,7 @@ const AlertingPage: React.SFC<AlertingPageProps> = ({ match }) => {
         </h1>
       </div>
       <div className="co-m-horizontal-nav__menu">
-        <ul className="co-m-horizontal-nav__menu-primary">
+        <ul className="co-m-horizontal-nav__menu-list">
           <li
             className={classNames('co-m-horizontal-nav__menu-item', {
               'co-m-horizontal-nav-item--active': match.url === alertPath,

diff --git a/frontend/public/components/provisioned-services.tsx b/frontend/public/components/provisioned-services.tsx
@@ -19,7 +19,7 @@ const pages = [
 export const ProvisionedServicesPage: React.SFC<ProvisionedServicesPageProps> = ({ match }) => (
   <>
     <PageHeading detail={true} title="Provisioned Services" />
-    <HorizontalNav pages={pages} match={match} hideDivider noStatusBox={true} />
+    <HorizontalNav pages={pages} match={match} noStatusBox={true} />
   </>
 );
 

diff --git a/frontend/public/components/utils/horizontal-nav.tsx b/frontend/public/components/utils/horizontal-nav.tsx
@@ -1,10 +1,10 @@
 import * as _ from 'lodash-es';
 import * as React from 'react';
-import * as classNames from 'classnames';
 import { History, Location } from 'history';
-import { Route, Switch, Link, withRouter, match, matchPath } from 'react-router-dom';
+import { Route, Switch, withRouter, match, matchPath } from 'react-router-dom';
+import { Nav, NavItem, NavList, NavVariants } from '@patternfly/react-core';
 
-import { EmptyBox, StatusBox } from '.';
+import { EmptyBox, StatusBox, history } from '.';
 import { PodsPage } from '../pod';
 import { AsyncComponent } from './async';
 import { K8sResourceKind } from '../../module/k8s';
@@ -130,66 +130,39 @@ export const navFactory: NavFactory = {
   }),
 };
 
-export const NavBar = withRouter<NavBarProps>(({ pages, basePath, hideDivider }) => {
-  // These tabs go before the divider
-  const before = ['', 'edit', 'yaml'];
-  const divider = (
-    <li
-      className="co-m-horizontal-nav__menu-item co-m-horizontal-nav__menu-item--divider"
-      key="_divider"
-    />
-  );
+export const NavBar = withRouter<NavBarProps>(({ pages, basePath }) => {
   basePath = basePath.replace(/\/$/, '');
 
-  const primaryTabs = (
-    <ul className="co-m-horizontal-nav__menu-primary">
-      {pages
-        .filter(
-          ({ href }, i, all) => before.includes(href) || before.includes(_.get(all[i + 1], 'href')),
-        )
-        .map(({ name, href, path }) => {
-          const matchUrl = matchPath(location.pathname, {
-            path: `${basePath}/${path || href}`,
-            exact: true,
-          });
-          const klass = classNames('co-m-horizontal-nav__menu-item', {
-            'co-m-horizontal-nav-item--active': matchUrl && matchUrl.isExact,
-          });
-          return (
-            <li className={klass} key={name}>
-              <Link to={`${basePath}/${href}`}>{name}</Link>
-            </li>
-          );
-        })}
-      {!hideDivider && divider}
-    </ul>
-  );
-
-  const secondaryTabs = (
-    <ul className="co-m-horizontal-nav__menu-secondary">
-      {pages
-        .slice(React.Children.count(primaryTabs.props.children) - 1)
-        .map(({ name, href, path }) => {
-          const matchUrl = matchPath(location.pathname, {
-            path: `${basePath}/${path || href}`,
-            exact: true,
-          });
-          const klass = classNames('co-m-horizontal-nav__menu-item', {
-            'co-m-horizontal-nav-item--active': matchUrl && matchUrl.isExact,
-          });
-          return (
-            <li className={klass} key={name}>
-              <Link to={`${basePath}/${href}`}>{name}</Link>
-            </li>
-          );
-        })}
-    </ul>
+  const tabs = (
+    <>
+      {pages.map(({ name, href, path }) => {
+        const matchUrl = matchPath(location.pathname, {
+          path: `${basePath}/${path || href}`,
+          exact: true,
+        });
+        return (
+          <NavItem
+            key={name}
+            itemId={name}
+            isActive={matchUrl && matchUrl.isExact}
+            preventDefault
+            onClick={() => history.push(`${basePath}/${href}`)}
+            className="co-m-horizontal-nav__menu-item"
+          >
+            {name}
+          </NavItem>
+        );
+      })}
+    </>
   );
 
   return (
-    <div className="co-m-horizontal-nav__menu">
-      {primaryTabs}
-      {secondaryTabs}
+    <div className="co-m-horizontal-nav__menu co-m-horizontal-nav__menu--scrollable">
+      <Nav>
+        <NavList variant={NavVariants.tertiary} className="co-m-horizontal-nav__menu-list">
+          {tabs}
+        </NavList>
+      </Nav>
     </div>
   );
 });
@@ -259,9 +232,7 @@ export const HorizontalNav: React.FC<HorizontalNavProps> = React.memo((props) =>
   return (
     <div className={props.className}>
       <div className="co-m-horizontal-nav">
-        {!props.hideNav && (
-          <NavBar pages={pages} basePath={props.match.url} hideDivider={props.hideDivider} />
-        )}
+        {!props.hideNav && <NavBar pages={pages} basePath={props.match.url} />}
         {renderContent(routes)}
       </div>
     </div>
@@ -275,7 +246,6 @@ export type PodsComponentProps = {
 export type NavBarProps = {
   pages: Page[];
   basePath: string;
-  hideDivider?: boolean;
   history: History;
   location: Location<any>;
   match: match<any>;
@@ -290,7 +260,6 @@ export type HorizontalNavProps = {
   match: any;
   resourceKeys?: string[];
   hideNav?: boolean;
-  hideDivider?: boolean;
   EmptyMsg?: React.ComponentType<any>;
   noStatusBox?: boolean;
   customData?: any;

diff --git a/frontend/public/components/utils/simple-tab-nav.tsx b/frontend/public/components/utils/simple-tab-nav.tsx
@@ -51,7 +51,7 @@ export class SimpleTabNav extends React.Component<SimpleTabNavProps, SimpleTabNa
     return (
       <>
         <div className={classNames('co-m-horizontal-nav__menu', additionalClassNames)}>
-          <ul className="co-m-horizontal-nav__menu-primary">
+          <ul className="co-m-horizontal-nav__menu-list">
             {_.map(tabs, (tab) => (
               <SimpleTab
                 active={selectedTab.name === tab.name}