From 9114c16a97e7808c648494db7bf284adee617d6f Mon Sep 17 00:00:00 2001 From: Jake McDermott Date: Fri, 14 Dec 2018 03:39:33 -0500 Subject: [PATCH 1/4] refactor navitem factory function to wrapped router-hoc nav components --- src/App.jsx | 327 ++++++++++++++++--------------- src/components/NavExpandable.jsx | 44 +++++ src/components/NavItem.jsx | 7 + 3 files changed, 217 insertions(+), 161 deletions(-) create mode 100644 src/components/NavExpandable.jsx create mode 100644 src/components/NavItem.jsx diff --git a/src/App.jsx b/src/App.jsx index 69e300dd75..53cc835047 100644 --- a/src/App.jsx +++ b/src/App.jsx @@ -11,9 +11,7 @@ import { BackgroundImage, BackgroundImageSrc, Nav, - NavExpandable, NavList, - NavItem, Page, PageHeader, PageSidebar, @@ -30,6 +28,8 @@ import HelpDropdown from './components/HelpDropdown'; import LogoutButton from './components/LogoutButton'; import TowerLogo from './components/TowerLogo'; import ConditionalRedirect from './components/ConditionalRedirect'; +import NavExpandable from './components/NavExpandable'; +import NavItem from './components/NavItem'; import Applications from './pages/Applications'; import Credentials from './pages/Credentials'; @@ -67,41 +67,6 @@ const language = (navigator.languages && navigator.languages[0]) const languageWithoutRegionCode = language.toLowerCase().split(/[_-]+/)[0]; -const SideNavItems = ({ items, history }) => { - const currentPath = history.location.pathname.split('/')[1]; - let activeGroup; - if (currentPath !== '') { - [{ groupName: activeGroup }] = items - .map(({ groupName, routes }) => ({ - groupName, - paths: routes.map(({ path }) => path) - })) - .filter(({ paths }) => paths.indexOf(currentPath) > -1); - } else { - activeGroup = 'views'; - } - - return (items.map(({ title, groupName, routes }) => ( - - {routes.map(({ path, title: itemTitle }) => ( - - {itemTitle} - - ))} - - ))); -}; class App extends React.Component { constructor (props) { @@ -160,7 +125,12 @@ class App extends React.Component { }} /> - api.isAuthenticated()} redirectPath="/" path="/login" component={() => } /> + api.isAuthenticated()} + redirectPath="/" + path="/login" + component={() => } + /> ( )} diff --git a/src/components/NavExpandable.jsx b/src/components/NavExpandable.jsx new file mode 100644 index 0000000000..b6cf0c7f0e --- /dev/null +++ b/src/components/NavExpandable.jsx @@ -0,0 +1,44 @@ +import React, { Component } from 'react'; +import { + withRouter +} from 'react-router-dom'; +import { + NavExpandable, + NavItem, +} from '@patternfly/react-core'; + +class NavExpandableWrapper extends Component { + constructor (props) { + super(props); + // introspect to get any child 'NavItem' components + const { children } = this.props; + const navItems = children.filter(({ type }) => type.componentType === NavItem.displayName); + + // Extract a list of 'to' params from the nav items and store it for later. This will create + // an array of the url paths associated with any child NavItem components. + this.navItemPaths = navItems.map(item => item.props.to.replace('/#', '')); + } + + isActiveGroup = () => { + const { history } = this.props; + + return this.navItemPaths.some(path => history.location.pathname.includes(path)); + }; + + render () { + const { children, staticContext, ...rest } = this.props; + const isActive = this.isActiveGroup(); + + return ( + + { children } + + ); + } +} + +export default withRouter(NavExpandableWrapper); diff --git a/src/components/NavItem.jsx b/src/components/NavItem.jsx new file mode 100644 index 0000000000..5610b09b4c --- /dev/null +++ b/src/components/NavItem.jsx @@ -0,0 +1,7 @@ +import React from 'react'; +import { withRouter } from 'react-router-dom'; +import { NavItem } from '@patternfly/react-core'; + +export default withRouter(({ history, to, staticContext, ...props }) => ( + +)); From 5d4aa56f4ab6d79ed0e36c9ab69336a5a291ba85 Mon Sep 17 00:00:00 2001 From: Jake McDermott Date: Fri, 14 Dec 2018 04:02:44 -0500 Subject: [PATCH 2/4] refactor wrapped nav components to expandable nav group component --- src/App.jsx | 203 ++++++-------------------- src/components/NavExpandable.jsx | 44 ------ src/components/NavExpandableGroup.jsx | 53 +++++++ src/components/NavItem.jsx | 7 - 4 files changed, 101 insertions(+), 206 deletions(-) delete mode 100644 src/components/NavExpandable.jsx create mode 100644 src/components/NavExpandableGroup.jsx delete mode 100644 src/components/NavItem.jsx diff --git a/src/App.jsx b/src/App.jsx index 53cc835047..38465599d8 100644 --- a/src/App.jsx +++ b/src/App.jsx @@ -28,8 +28,7 @@ import HelpDropdown from './components/HelpDropdown'; import LogoutButton from './components/LogoutButton'; import TowerLogo from './components/TowerLogo'; import ConditionalRedirect from './components/ConditionalRedirect'; -import NavExpandable from './components/NavExpandable'; -import NavItem from './components/NavItem'; +import NavExpandableGroup from './components/NavExpandableGroup'; import Applications from './pages/Applications'; import Credentials from './pages/Credentials'; @@ -149,164 +148,58 @@ class App extends React.Component { {({ i18n }) => ( )} diff --git a/src/components/NavExpandable.jsx b/src/components/NavExpandable.jsx deleted file mode 100644 index b6cf0c7f0e..0000000000 --- a/src/components/NavExpandable.jsx +++ /dev/null @@ -1,44 +0,0 @@ -import React, { Component } from 'react'; -import { - withRouter -} from 'react-router-dom'; -import { - NavExpandable, - NavItem, -} from '@patternfly/react-core'; - -class NavExpandableWrapper extends Component { - constructor (props) { - super(props); - // introspect to get any child 'NavItem' components - const { children } = this.props; - const navItems = children.filter(({ type }) => type.componentType === NavItem.displayName); - - // Extract a list of 'to' params from the nav items and store it for later. This will create - // an array of the url paths associated with any child NavItem components. - this.navItemPaths = navItems.map(item => item.props.to.replace('/#', '')); - } - - isActiveGroup = () => { - const { history } = this.props; - - return this.navItemPaths.some(path => history.location.pathname.includes(path)); - }; - - render () { - const { children, staticContext, ...rest } = this.props; - const isActive = this.isActiveGroup(); - - return ( - - { children } - - ); - } -} - -export default withRouter(NavExpandableWrapper); diff --git a/src/components/NavExpandableGroup.jsx b/src/components/NavExpandableGroup.jsx new file mode 100644 index 0000000000..70cdbf5adb --- /dev/null +++ b/src/components/NavExpandableGroup.jsx @@ -0,0 +1,53 @@ +import React, { Component } from 'react'; +import { + withRouter +} from 'react-router-dom'; +import { + NavExpandable, + NavItem, +} from '@patternfly/react-core'; + +class NavExpandableGroup extends Component { + constructor (props) { + super(props); + const { routes } = this.props; + // Extract a list of paths from the route params and store them for later. This creates + // an array of url paths associated with any NavItem component rendered by this component. + this.navItemPaths = routes.map(({ path }) => path); + } + + isActiveGroup = () => this.navItemPaths.some(this.isActivePath); + + isActivePath = (path) => { + const { history } = this.props; + + return history.location.pathname.includes(path); + }; + + render () { + const { routes, groupId, staticContext, ...rest } = this.props; + const isActive = this.isActiveGroup(); + + return ( + + {routes.map(({ path, title }) => ( + + {title} + + ))} + + ); + } +} + +export default withRouter(NavExpandableGroup); diff --git a/src/components/NavItem.jsx b/src/components/NavItem.jsx deleted file mode 100644 index 5610b09b4c..0000000000 --- a/src/components/NavItem.jsx +++ /dev/null @@ -1,7 +0,0 @@ -import React from 'react'; -import { withRouter } from 'react-router-dom'; -import { NavItem } from '@patternfly/react-core'; - -export default withRouter(({ history, to, staticContext, ...props }) => ( - -)); From 3d730ef8d26f90a5f277bfd81f0137614bb75cd9 Mon Sep 17 00:00:00 2001 From: Jake McDermott Date: Sun, 16 Dec 2018 23:42:36 -0500 Subject: [PATCH 3/4] add basic unit test for expandable navgroup component --- .../components/NavExpandableGroup.test.jsx | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 __tests__/components/NavExpandableGroup.test.jsx diff --git a/__tests__/components/NavExpandableGroup.test.jsx b/__tests__/components/NavExpandableGroup.test.jsx new file mode 100644 index 0000000000..56b0f499fd --- /dev/null +++ b/__tests__/components/NavExpandableGroup.test.jsx @@ -0,0 +1,29 @@ +import React from 'react'; +import { MemoryRouter } from 'react-router-dom'; +import { mount } from 'enzyme'; + +import { Nav } from '@patternfly/react-core'; +import NavExpandableGroup from '../../src/components/NavExpandableGroup'; + +describe('NavExpandableGroup', () => { + test('initialization and render', () => { + const component = mount( + + + + ).find('NavExpandableGroup').instance(); + + expect(component.navItemPaths).toEqual(['/foo', '/bar', '/fiz']); + expect(component.isActiveGroup()).toEqual(true); + }); +}); From 1bb86dbdf07b96305fb726a652a6e283d137324d Mon Sep 17 00:00:00 2001 From: Jake McDermott Date: Mon, 17 Dec 2018 00:03:27 -0500 Subject: [PATCH 4/4] use beginning of location path name when checking for active items --- .../components/NavExpandableGroup.test.jsx | 34 +++++++++++++++++++ src/components/NavExpandableGroup.jsx | 2 +- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/__tests__/components/NavExpandableGroup.test.jsx b/__tests__/components/NavExpandableGroup.test.jsx index 56b0f499fd..68cf571f7d 100644 --- a/__tests__/components/NavExpandableGroup.test.jsx +++ b/__tests__/components/NavExpandableGroup.test.jsx @@ -26,4 +26,38 @@ describe('NavExpandableGroup', () => { expect(component.navItemPaths).toEqual(['/foo', '/bar', '/fiz']); expect(component.isActiveGroup()).toEqual(true); }); + + describe('isActivePath', () => { + const params = [ + ['/fo', '/foo', false], + ['/foo', '/foo', true], + ['/foo/1/bar/fiz', '/foo', true], + ['/foo/1/bar/fiz', 'foo', false], + ['/foo/1/bar/fiz', 'foo/', false], + ['/foo/1/bar/fiz', '/bar', false], + ['/foo/1/bar/fiz', '/fiz', false], + ]; + + params.forEach(([location, path, expected]) => { + test(`when location is ${location}', isActivePath('${path}') returns ${expected} `, () => { + const component = mount( + + + + ).find('NavExpandableGroup').instance(); + + expect(component.isActivePath(path)).toEqual(expected); + }); + }); + }); }); diff --git a/src/components/NavExpandableGroup.jsx b/src/components/NavExpandableGroup.jsx index 70cdbf5adb..ba3058b3df 100644 --- a/src/components/NavExpandableGroup.jsx +++ b/src/components/NavExpandableGroup.jsx @@ -21,7 +21,7 @@ class NavExpandableGroup extends Component { isActivePath = (path) => { const { history } = this.props; - return history.location.pathname.includes(path); + return history.location.pathname.startsWith(path); }; render () {