From 9114c16a97e7808c648494db7bf284adee617d6f Mon Sep 17 00:00:00 2001 From: Jake McDermott Date: Fri, 14 Dec 2018 03:39:33 -0500 Subject: [PATCH 01/12] 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 4a8791693f2aebf6d7a28c3569bd9d118f4fc278 Mon Sep 17 00:00:00 2001 From: mabashian Date: Fri, 14 Dec 2018 09:55:59 -0500 Subject: [PATCH 02/12] Adds option to hide expand/collapse in toolbar and hides it for the org list --- src/components/DataListToolbar/DataListToolbar.jsx | 5 +++-- src/pages/Organizations/views/Organizations.list.jsx | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/components/DataListToolbar/DataListToolbar.jsx b/src/components/DataListToolbar/DataListToolbar.jsx index eb8b913911..f8ca09c6c7 100644 --- a/src/components/DataListToolbar/DataListToolbar.jsx +++ b/src/components/DataListToolbar/DataListToolbar.jsx @@ -85,7 +85,8 @@ class DataListToolbar extends React.Component { onSort, sortedColumnKey, sortOrder, - addUrl + addUrl, + hideExpandCollapse } = this.props; const { // isActionDropdownOpen, @@ -202,7 +203,7 @@ class DataListToolbar extends React.Component { - + - - - - - - - - + { showExpandCollapse && ( + + + + + + + + + )} diff --git a/src/pages/Organizations/views/Organizations.list.jsx b/src/pages/Organizations/views/Organizations.list.jsx index 2a03a61cf8..5ca2809911 100644 --- a/src/pages/Organizations/views/Organizations.list.jsx +++ b/src/pages/Organizations/views/Organizations.list.jsx @@ -205,7 +205,6 @@ class Organizations extends Component { onSearch={this.onSearch} onSort={this.onSort} onSelectAll={this.onSelectAll} - hideExpandCollapse /> {({ i18n }) => ( From 656e6d4f6a8a0678b30f5058710c9b6e907c9abd Mon Sep 17 00:00:00 2001 From: kialam Date: Mon, 17 Dec 2018 12:39:41 -0500 Subject: [PATCH 07/12] Add back missing style. --- src/app.scss | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/app.scss b/src/app.scss index 5f79cc5505..2d3943a597 100644 --- a/src/app.scss +++ b/src/app.scss @@ -118,3 +118,13 @@ --pf-c-about-modal-box--MaxHeight: 40rem; --pf-c-about-modal-box--MaxWidth: 63rem; } + + +// +// layout styles +// +.at-align-right { + display: flex; + flex-direction: row; + justify-content: flex-end; +} \ No newline at end of file From 71ace1bc0003dd3de995243becdc733bd2e4d274 Mon Sep 17 00:00:00 2001 From: mabashian Date: Wed, 12 Dec 2018 10:40:51 -0500 Subject: [PATCH 08/12] Small ux changes on the org list based on feedback --- .../DataListToolbar/DataListToolbar.jsx | 29 ++++++++----------- src/components/DataListToolbar/styles.scss | 17 +++++++---- .../components/OrganizationListItem.jsx | 2 +- 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/components/DataListToolbar/DataListToolbar.jsx b/src/components/DataListToolbar/DataListToolbar.jsx index c48d855dd7..80b238be6e 100644 --- a/src/components/DataListToolbar/DataListToolbar.jsx +++ b/src/components/DataListToolbar/DataListToolbar.jsx @@ -23,6 +23,7 @@ import { SortNumericDownIcon, SortNumericUpIcon, TrashAltIcon, + PlusIcon } from '@patternfly/react-icons'; import { Link @@ -114,6 +115,12 @@ class DataListToolbar extends React.Component { return icon; }; + const dropdownItems = columns.filter(({ key }) => key !== searchKey).map(({ key, name }) => ( + + { name } + + )); + return ( {({ i18n }) => ( @@ -146,13 +153,8 @@ class DataListToolbar extends React.Component { { searchColumnName } )} - > - {columns.filter(({ key }) => key !== searchKey).map(({ key, name }) => ( - - { name } - - ))} - + dropdownItems={dropdownItems} + /> )} - > - {columns - .filter(({ key, isSortable }) => isSortable && key !== sortedColumnKey) - .map(({ key, name }) => ( - - { name } - - ))} - + dropdownItems={dropdownItems} + /> )} diff --git a/src/components/DataListToolbar/styles.scss b/src/components/DataListToolbar/styles.scss index 66fd6f0902..1258225f31 100644 --- a/src/components/DataListToolbar/styles.scss +++ b/src/components/DataListToolbar/styles.scss @@ -33,7 +33,7 @@ margin-right: 20px; } -.awx-toolbar button { +.awx-toolbar button.pf-c-button { height: 30px; padding: 0px; } @@ -43,7 +43,7 @@ height: 30px; input { - padding: 0px; + padding: 0 10px; width: 300px; } @@ -57,7 +57,7 @@ min-height: 30px; min-width: 70px; height: 30px; - padding: 0px; + padding: 0 10px; margin: 0px; .pf-c-dropdown__toggle-icon { @@ -74,16 +74,23 @@ .awx-toolbar .pf-c-button.pf-m-primary { background-color: #5cb85c; min-width: 0px; - width: 58px; + width: 30px; height: 30px; text-align: center; - padding: 0px; margin: 0px; margin-right: 20px; margin-left: 20px; + border-radius: 50%; } .awx-toolbar .pf-l-toolbar__item .pf-c-button.pf-m-plain { font-size: 18px; } + +.awx-toolbar .pf-l-toolbar__item .pf-c-button.pf-m-plain :nth-of-type(2) { +// .awx-toolbar .pf-l-toolbar__item:nth-of-type(2) { +// .pf-c-button.pf-m-plain { + font-size: 22px; +// } +} diff --git a/src/pages/Organizations/components/OrganizationListItem.jsx b/src/pages/Organizations/components/OrganizationListItem.jsx index da2a2ef2cd..d28f158126 100644 --- a/src/pages/Organizations/components/OrganizationListItem.jsx +++ b/src/pages/Organizations/components/OrganizationListItem.jsx @@ -41,7 +41,7 @@ export default ({ state: { breadcrumb: [parentBreadcrumb, { name, url: detailUrl }] } }} > - {name} + {name} From d43f0cb2fc63f25134b9fef15d3df0f39511ba25 Mon Sep 17 00:00:00 2001 From: mabashian Date: Wed, 12 Dec 2018 10:53:23 -0500 Subject: [PATCH 09/12] Removed extraneous style --- src/components/DataListToolbar/styles.scss | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/components/DataListToolbar/styles.scss b/src/components/DataListToolbar/styles.scss index 1258225f31..178622387e 100644 --- a/src/components/DataListToolbar/styles.scss +++ b/src/components/DataListToolbar/styles.scss @@ -87,10 +87,3 @@ .awx-toolbar .pf-l-toolbar__item .pf-c-button.pf-m-plain { font-size: 18px; } - -.awx-toolbar .pf-l-toolbar__item .pf-c-button.pf-m-plain :nth-of-type(2) { -// .awx-toolbar .pf-l-toolbar__item:nth-of-type(2) { -// .pf-c-button.pf-m-plain { - font-size: 22px; -// } -} From 21cf1d85e3dd245ab4202b93672d4f4dfe49c3e5 Mon Sep 17 00:00:00 2001 From: mabashian Date: Thu, 13 Dec 2018 10:56:11 -0500 Subject: [PATCH 10/12] Remove border-radius on add button --- src/components/DataListToolbar/styles.scss | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/DataListToolbar/styles.scss b/src/components/DataListToolbar/styles.scss index 178622387e..722e2f6a67 100644 --- a/src/components/DataListToolbar/styles.scss +++ b/src/components/DataListToolbar/styles.scss @@ -81,7 +81,6 @@ margin: 0px; margin-right: 20px; margin-left: 20px; - border-radius: 50%; } .awx-toolbar .pf-l-toolbar__item .pf-c-button.pf-m-plain { From 6ce88fdf4db4541979752c722795518c7b45fd0b Mon Sep 17 00:00:00 2001 From: mabashian Date: Mon, 17 Dec 2018 13:16:08 -0500 Subject: [PATCH 11/12] Separates search dropdown items from sort dropdown items --- .../DataListToolbar/DataListToolbar.jsx | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/components/DataListToolbar/DataListToolbar.jsx b/src/components/DataListToolbar/DataListToolbar.jsx index 80b238be6e..e1d2ea6842 100644 --- a/src/components/DataListToolbar/DataListToolbar.jsx +++ b/src/components/DataListToolbar/DataListToolbar.jsx @@ -1,6 +1,6 @@ import React from 'react'; import { I18n } from '@lingui/react'; -import { Trans, t } from '@lingui/macro'; +import { t } from '@lingui/macro'; import { Button, Checkbox, @@ -115,11 +115,21 @@ class DataListToolbar extends React.Component { return icon; }; - const dropdownItems = columns.filter(({ key }) => key !== searchKey).map(({ key, name }) => ( - - { name } - - )); + const searchDropdownItems = columns + .filter(({ key }) => key !== searchKey) + .map(({ key, name }) => ( + + { name } + + )); + + const sortDropdownItems = columns + .filter(({ key, isSortable }) => isSortable && key !== sortedColumnKey) + .map(({ key, name }) => ( + + { name } + + )); return ( @@ -153,7 +163,7 @@ class DataListToolbar extends React.Component { { searchColumnName } )} - dropdownItems={dropdownItems} + dropdownItems={searchDropdownItems} /> )} - dropdownItems={dropdownItems} + dropdownItems={sortDropdownItems} /> From ff0015e21d1ef1907fdca8f4fd870b322ed9c3db Mon Sep 17 00:00:00 2001 From: kialam Date: Mon, 17 Dec 2018 13:44:13 -0500 Subject: [PATCH 12/12] Hook up Cancel button - Update unit tests. - Add basic error handling for API requests in Add Orgs component. --- .../views/Organization.add.test.jsx | 53 +++++++++++++------ .../Organizations/views/Organization.add.jsx | 26 ++++++--- 2 files changed, 56 insertions(+), 23 deletions(-) diff --git a/__tests__/pages/Organizations/views/Organization.add.test.jsx b/__tests__/pages/Organizations/views/Organization.add.test.jsx index 434621985d..8c25eb1eef 100644 --- a/__tests__/pages/Organizations/views/Organization.add.test.jsx +++ b/__tests__/pages/Organizations/views/Organization.add.test.jsx @@ -1,39 +1,60 @@ import React from 'react'; import { mount } from 'enzyme'; import OrganizationAdd from '../../../../src/pages/Organizations/views/Organization.add'; +import { MemoryRouter } from 'react-router-dom'; describe('', () => { test('initially renders succesfully', () => { mount( - + + + ); }); test('calls "handleChange" when input values change', () => { - const spy = jest.spyOn(OrganizationAdd.prototype, 'handleChange'); + const spy = jest.spyOn(OrganizationAdd.WrappedComponent.prototype, 'handleChange'); const wrapper = mount( - + + + ); expect(spy).not.toHaveBeenCalled(); - wrapper.find('input#add-org-form-name').simulate('change', {target: {value: 'foo'}}); - wrapper.find('input#add-org-form-description').simulate('change', {target: {value: 'bar'}}); + wrapper.find('input#add-org-form-name').simulate('change', { target: { value: 'foo' } }); + wrapper.find('input#add-org-form-description').simulate('change', { target: { value: 'bar' } }); expect(spy).toHaveBeenCalledTimes(2); }); test('calls "onSubmit" when Save button is clicked', () => { - const spy = jest.spyOn(OrganizationAdd.prototype, 'onSubmit'); + const spy = jest.spyOn(OrganizationAdd.WrappedComponent.prototype, 'onSubmit'); const wrapper = mount( - + + + ); expect(spy).not.toHaveBeenCalled(); wrapper.find('button.at-C-SubmitButton').prop('onClick')(); expect(spy).toBeCalled(); }); + test('calls "onCancel" when Cancel button is clicked', () => { + const spy = jest.spyOn(OrganizationAdd.WrappedComponent.prototype, 'onCancel'); + const wrapper = mount( + + + + ); + expect(spy).not.toHaveBeenCalled(); + wrapper.find('button.at-C-CancelButton').prop('onClick')(); + expect(spy).toBeCalled(); + }); }); diff --git a/src/pages/Organizations/views/Organization.add.jsx b/src/pages/Organizations/views/Organization.add.jsx index dea2e0973e..023817fe1d 100644 --- a/src/pages/Organizations/views/Organization.add.jsx +++ b/src/pages/Organizations/views/Organization.add.jsx @@ -1,4 +1,5 @@ import React, { Fragment } from 'react'; +import { withRouter } from 'react-router-dom'; import { Trans } from '@lingui/macro'; import { PageSection, @@ -29,6 +30,7 @@ class OrganizationAdd extends React.Component { this.onSelectChange = this.onSelectChange.bind(this); this.onSubmit = this.onSubmit.bind(this); this.resetForm = this.resetForm.bind(this); + this.onCancel = this.onCancel.bind(this); } state = { @@ -38,6 +40,7 @@ class OrganizationAdd extends React.Component { custom_virtualenv: '', custom_virtualenvs: [], hideAnsibleSelect: true, + error:'', }; onSelectChange(value, _) { @@ -62,13 +65,22 @@ class OrganizationAdd extends React.Component { this.resetForm(); } + onCancel() { + this.props.history.push('/organizations'); + } + async componentDidMount() { - const { data } = await api.get(API_CONFIG); - this.setState({ custom_virtualenvs: [...data.custom_virtualenvs] }); - if (this.state.custom_virtualenvs.length > 1) { - // Show dropdown if we have more than one ansible environment - this.setState({ hideAnsibleSelect: !this.state.hideAnsibleSelect }); + try { + const { data } = await api.get(API_CONFIG); + this.setState({ custom_virtualenvs: [...data.custom_virtualenvs] }); + if (this.state.custom_virtualenvs.length > 1) { + // Show dropdown if we have more than one ansible environment + this.setState({ hideAnsibleSelect: !this.state.hideAnsibleSelect }); + } + } catch (error) { + this.setState({ error }) } + } render() { const { name } = this.state; @@ -130,7 +142,7 @@ class OrganizationAdd extends React.Component { - + @@ -143,4 +155,4 @@ class OrganizationAdd extends React.Component { } } -export default OrganizationAdd; +export default withRouter(OrganizationAdd);