From b820e411d36045b888108757fcb97d6126d45d0b Mon Sep 17 00:00:00 2001 From: Marliana Lara Date: Thu, 24 Jan 2019 23:53:10 -0500 Subject: [PATCH] Refactor breadcrumbs, tabs, routing. * Cleanup previous params logic from Notificaitons list * Add shared breadcrumbs and tabs components * Structure Organization screens to render only cardBody content * Add styles * Fetch organization when location changes --- .../OrganizationBreadcrumb.test.jsx | 18 -- .../Organization/OrganizationDetail.test.jsx | 5 +- __tests__/pages/Organizations/utils.test.jsx | 12 -- src/app.scss | 4 +- src/components/Breadcrumbs/Breadcrumbs.jsx | 70 +++++++ src/components/Breadcrumbs/breadcrumbs.scss | 15 ++ src/components/DataListToolbar/styles.scss | 2 +- .../NotificationsList/Notifications.list.jsx | 14 +- src/components/Tabs/Tab.jsx | 48 ++--- src/components/Tabs/Tabs.jsx | 30 ++- src/components/Tabs/tabs.scss | 36 ++-- src/pages/Organizations/Organizations.jsx | 102 +++++++--- .../components/OrganizationListItem.jsx | 10 +- .../screens/Organization/Organization.jsx | 180 +++++++++++------- .../Organization/OrganizationDetail.jsx | 126 ++---------- .../screens/Organization/OrganizationEdit.jsx | 21 +- .../screens/OrganizationsList.jsx | 11 +- src/pages/Organizations/utils.jsx | 15 -- 18 files changed, 363 insertions(+), 356 deletions(-) delete mode 100644 __tests__/pages/Organizations/components/OrganizationBreadcrumb.test.jsx delete mode 100644 __tests__/pages/Organizations/utils.test.jsx create mode 100644 src/components/Breadcrumbs/Breadcrumbs.jsx create mode 100644 src/components/Breadcrumbs/breadcrumbs.scss delete mode 100644 src/pages/Organizations/utils.jsx diff --git a/__tests__/pages/Organizations/components/OrganizationBreadcrumb.test.jsx b/__tests__/pages/Organizations/components/OrganizationBreadcrumb.test.jsx deleted file mode 100644 index bbb222555f..0000000000 --- a/__tests__/pages/Organizations/components/OrganizationBreadcrumb.test.jsx +++ /dev/null @@ -1,18 +0,0 @@ -import React from 'react'; -import { mount } from 'enzyme'; -import { MemoryRouter } from 'react-router-dom'; -import OrganizationBreadcrumb from '../../../../src/pages/Organizations/components/OrganizationBreadcrumb'; - -describe('', () => { - test('initially renders succesfully', () => { - mount( - - - - ); - }); -}); diff --git a/__tests__/pages/Organizations/screens/Organization/OrganizationDetail.test.jsx b/__tests__/pages/Organizations/screens/Organization/OrganizationDetail.test.jsx index 7262ae85a9..11ad3a144f 100644 --- a/__tests__/pages/Organizations/screens/Organization/OrganizationDetail.test.jsx +++ b/__tests__/pages/Organizations/screens/Organization/OrganizationDetail.test.jsx @@ -10,9 +10,8 @@ describe('', () => { diff --git a/__tests__/pages/Organizations/utils.test.jsx b/__tests__/pages/Organizations/utils.test.jsx deleted file mode 100644 index e2274bfeeb..0000000000 --- a/__tests__/pages/Organizations/utils.test.jsx +++ /dev/null @@ -1,12 +0,0 @@ -import getTabName from '../../../src/pages/Organizations/utils'; - -describe('getTabName', () => { - test('returns tab name', () => { - expect(getTabName('details')).toBe('Details'); - expect(getTabName('access')).toBe('Access'); - expect(getTabName('teams')).toBe('Teams'); - expect(getTabName('notifications')).toBe('Notifications'); - expect(getTabName('unknown')).toBe(''); - expect(getTabName()).toBe(''); - }); -}); diff --git a/src/app.scss b/src/app.scss index d9b34ab4ad..8997ddb56c 100644 --- a/src/app.scss +++ b/src/app.scss @@ -57,8 +57,8 @@ // .pf-c-page__main-section.pf-m-condensed { - padding-top: 16px; - padding-bottom: 16px; + padding-top: 10px; + padding-bottom: 10px; } // diff --git a/src/components/Breadcrumbs/Breadcrumbs.jsx b/src/components/Breadcrumbs/Breadcrumbs.jsx new file mode 100644 index 0000000000..ffc4f95346 --- /dev/null +++ b/src/components/Breadcrumbs/Breadcrumbs.jsx @@ -0,0 +1,70 @@ +import React, { Fragment } from 'react'; +import { + PageSection, + PageSectionVariants, + Breadcrumb, + BreadcrumbItem, + BreadcrumbHeading +} from '@patternfly/react-core'; +import { + Link, + Route, + withRouter +} from 'react-router-dom'; +import './breadcrumbs.scss'; + +const Breadcrumbs = ({ breadcrumbConfig }) => { + const { light } = PageSectionVariants; + + return ( + + + } + /> + + + ); +}; + +const Crumb = ({ breadcrumbConfig, match }) => { + const crumb = breadcrumbConfig[match.url]; + + let crumbElement = ( + + + {crumb} + + + ); + + if (match.isExact) { + crumbElement = ( + + {crumb} + + ); + } + + if (!crumb) { + crumbElement = null; + } + + return ( + + {crumbElement} + } + /> + + ); +}; + +export default withRouter(Breadcrumbs); diff --git a/src/components/Breadcrumbs/breadcrumbs.scss b/src/components/Breadcrumbs/breadcrumbs.scss new file mode 100644 index 0000000000..ab654cfaed --- /dev/null +++ b/src/components/Breadcrumbs/breadcrumbs.scss @@ -0,0 +1,15 @@ +.pf-c-breadcrumb__item { + text-transform: capitalize; +} + +.pf-c-breadcrumb__item.heading { + --pf-c-breadcrumb__heading--FontSize: 20px; + line-height: 24px; + flex: 100%; +} + +.pf-c-breadcrumb__item:nth-last-child(2) { + .pf-c-breadcrumb__item-divider { + display: none; + } +} diff --git a/src/components/DataListToolbar/styles.scss b/src/components/DataListToolbar/styles.scss index 722e2f6a67..7c17f50d56 100644 --- a/src/components/DataListToolbar/styles.scss +++ b/src/components/DataListToolbar/styles.scss @@ -3,7 +3,7 @@ --awx-toolbar--BorderColor: var(--pf-global--Color--light-200); --awx-toolbar--BorderWidth: var(--pf-global--BorderWidth--sm); - border: var(--awx-toolbar--BorderWidth) solid var(--awx-toolbar--BorderColor); + border-bottom: var(--awx-toolbar--BorderWidth) solid var(--awx-toolbar--BorderColor); background-color: var(--awx-toolbar--BackgroundColor); height: 70px; padding-top: 5px; diff --git a/src/components/NotificationsList/Notifications.list.jsx b/src/components/NotificationsList/Notifications.list.jsx index 5b3cbb8635..7aca6b0db8 100644 --- a/src/components/NotificationsList/Notifications.list.jsx +++ b/src/components/NotificationsList/Notifications.list.jsx @@ -65,9 +65,7 @@ class Notifications extends Component { componentDidMount () { const queryParams = this.getQueryParams(); - // TODO: remove this hack once tab query param is gone - const { tab, ...queryParamsWithoutTab } = queryParams; - this.fetchNotifications(queryParamsWithoutTab); + this.fetchNotifications(queryParams); } onSearch () { @@ -81,10 +79,8 @@ class Notifications extends Component { const { search } = location; const searchParams = parseQueryString(search.substring(1)); - // TODO: remove this hack once tab query param is gone - const { tab, ...queryParamsWithoutTab } = searchParams; - return Object.assign({}, this.defaultParams, queryParamsWithoutTab, overrides); + return Object.assign({}, this.defaultParams, searchParams, overrides); } onSort = (sortedColumnKey, sortOrder) => { @@ -240,8 +236,6 @@ class Notifications extends Component { } this.setState(stateToUpdate); - // TODO: remove this hack once tab query param is gone - this.updateUrl({ ...queryParams, tab: 'notifications' }); const notificationTemplateIds = results .map(notificationTemplate => notificationTemplate.id) @@ -299,7 +293,7 @@ class Notifications extends Component { - <Trans>No Notifictions Found</Trans> + <Trans>No Notifications Found</Trans> Please add a notification template to populate this list @@ -331,7 +325,7 @@ class Notifications extends Component { itemId={o.id} name={o.name} notificationType={o.notification_type} - detailUrl={`notifications/${o.id}`} + detailUrl={`/notifications/${o.id}`} isSelected={selected.includes(o.id)} onSelect={() => this.onSelect(o.id)} toggleNotification={this.toggleNotification} diff --git a/src/components/Tabs/Tab.jsx b/src/components/Tabs/Tab.jsx index 2b40bc1a3e..162c740312 100644 --- a/src/components/Tabs/Tab.jsx +++ b/src/components/Tabs/Tab.jsx @@ -1,40 +1,18 @@ import React from 'react'; -import { Link } from 'react-router-dom'; - +import { NavLink } from 'react-router-dom'; import './tabs.scss'; -const Tab = ({ location, match, tab, currentTab, children, breadcrumb }) => { - const tabClasses = () => { - let classes = 'pf-c-tabs__item'; - if (tab === currentTab) { - classes += ' pf-m-current'; - } - - return classes; - }; - - const tabParams = () => { - const params = new URLSearchParams(location.search); - if (params.get('tab') !== undefined) { - params.set('tab', tab); - } else { - params.append('tab', tab); - } - - return `?${params.toString()}`; - }; - - return ( -
  • - - {children} - -
    • - ); -}; +const Tab = ({ children, link, replace }) => ( +
  • + + {children} + +
    • +); export default Tab; diff --git a/src/components/Tabs/Tabs.jsx b/src/components/Tabs/Tabs.jsx index 91fd3e2bda..3a308ccdc4 100644 --- a/src/components/Tabs/Tabs.jsx +++ b/src/components/Tabs/Tabs.jsx @@ -1,11 +1,37 @@ import React from 'react'; +import { Link } from 'react-router-dom'; +import { Button } from '@patternfly/react-core'; +import { TimesIcon } from '@patternfly/react-icons'; +import Tooltip from '../Tooltip'; import './tabs.scss'; -const Tabs = ({ children, labelText }) => ( -
    +const Tabs = ({ children, labelText, closeButton }) => ( +
      {children}
    + {closeButton + && ( +
    + + + + + +
    + ) + }
    ); diff --git a/src/components/Tabs/tabs.scss b/src/components/Tabs/tabs.scss index 421e933063..15c557dea8 100644 --- a/src/components/Tabs/tabs.scss +++ b/src/components/Tabs/tabs.scss @@ -1,9 +1,3 @@ -.at-c-orgPane { - a { - display: block; - } -} - .pf-c-card__header { --pf-c-card__header--PaddingBottom: 0; --pf-c-card__header--PaddingX: 0; @@ -29,22 +23,34 @@ .pf-c-tabs__button { --pf-c-tabs__button--PaddingLeft: 20px; --pf-c-tabs__button--PaddingRight: 20px; - } + display: block; - .pf-c-tabs__item.pf-m-current - .pf-c-tabs__button::after { - border-bottom: 3px solid var(--pf-c-tabs__item--m-current--Color); - border-top: none; + &:after { + content: ''; + bottom: 0; + left: 0; + position: absolute; + right: 0; + top: 0; + } } .pf-c-tabs__item:not(.pf-m-current):hover .pf-c-tabs__button::after { + border-top: none; + } + + .pf-c-tabs__item:hover + .pf-c-tabs__button:not(.pf-m-current)::after { border-bottom: 3px solid var(--pf-global--Color--dark-200); border-top: none; } + + .pf-c-tabs__button.pf-m-current::after { + content: ''; + border-bottom: 3px solid var(--pf-c-tabs__item--m-current--Color); + border-top: none; + margin-left: 1px; + } } -.pf-c-breadcrumb__item.heading { - flex: 100%; - font-size: 20px; -} \ No newline at end of file diff --git a/src/pages/Organizations/Organizations.jsx b/src/pages/Organizations/Organizations.jsx index 27f1830cc6..7b5d41377e 100644 --- a/src/pages/Organizations/Organizations.jsx +++ b/src/pages/Organizations/Organizations.jsx @@ -1,36 +1,80 @@ -import React from 'react'; +import React, { Component, Fragment } from 'react'; import { Route, Switch } from 'react-router-dom'; +import { i18nMark } from '@lingui/react'; import OrganizationsList from './screens/OrganizationsList'; import OrganizationAdd from './screens/OrganizationAdd'; import Organization from './screens/Organization/Organization'; +import Breadcrumbs from '../../components/Breadcrumbs/Breadcrumbs'; -export default ({ api, match, history }) => ( - - ( - { + if (!organization) { + return; + } + + const breadcrumbConfig = { + '/organizations': i18nMark('Organizations'), + '/organizations/add': i18nMark('Create New Organization'), + [`/organizations/${organization.id}`]: `${organization.name}`, + [`/organizations/${organization.id}/edit`]: i18nMark('Edit Details'), + [`/organizations/${organization.id}/details`]: i18nMark('Details'), + [`/organizations/${organization.id}/access`]: i18nMark('Access'), + [`/organizations/${organization.id}/teams`]: i18nMark('Teams'), + [`/organizations/${organization.id}/notifications`]: i18nMark('Notifications'), + }; + + this.setState({ breadcrumbConfig }); + } + + render () { + const { match, api, history, location } = this.props; + const { breadcrumbConfig } = this.state; + + return ( + + - )} - /> - ( - - )} - /> - ( - - )} - /> - -); + + ( + + )} + /> + ( + + )} + /> + ( + + )} + /> + + + ); + } +} + +export default Organizations; diff --git a/src/pages/Organizations/components/OrganizationListItem.jsx b/src/pages/Organizations/components/OrganizationListItem.jsx index 161b8274a2..74a3699aa1 100644 --- a/src/pages/Organizations/components/OrganizationListItem.jsx +++ b/src/pages/Organizations/components/OrganizationListItem.jsx @@ -17,7 +17,6 @@ export default ({ isSelected, onSelect, detailUrl, - parentBreadcrumb }) => (
  • @@ -35,17 +34,14 @@ export default ({
    {name}
    - + Users @@ -53,7 +49,7 @@ export default ({ {userCount} {' '} - + Teams diff --git a/src/pages/Organizations/screens/Organization/Organization.jsx b/src/pages/Organizations/screens/Organization/Organization.jsx index acd0dd407a..83bc7bdd59 100644 --- a/src/pages/Organizations/screens/Organization/Organization.jsx +++ b/src/pages/Organizations/screens/Organization/Organization.jsx @@ -1,128 +1,166 @@ -import React, { Component, Fragment } from 'react'; -import { i18nMark } from '@lingui/react'; +import React, { Component } from 'react'; +import { I18n, i18nMark } from '@lingui/react'; +import { Trans, t } from '@lingui/macro'; import { Switch, Route, withRouter, + Redirect } from 'react-router-dom'; import { + Card, + CardBody, + CardHeader, PageSection } from '@patternfly/react-core'; -import OrganizationBreadcrumb from '../../components/OrganizationBreadcrumb'; import OrganizationDetail from './OrganizationDetail'; import OrganizationEdit from './OrganizationEdit'; +import OrganizationNotifications from './OrganizationNotifications'; +import Tabs from '../../../../components/Tabs/Tabs'; +import Tab from '../../../../components/Tabs/Tab'; class Organization extends Component { constructor (props) { super(props); - let { breadcrumb: parentBreadcrumbObj, organization } = props.location.state || {}; - if (!parentBreadcrumbObj) { - parentBreadcrumbObj = 'loading'; - } - if (!organization) { - organization = 'loading'; - } this.state = { - parentBreadcrumbObj, - organization, + organization: null, error: false, - loading: false, - mounted: false + loading: true, }; this.fetchOrganization = this.fetchOrganization.bind(this); } componentDidMount () { - this.setState({ mounted: true }, () => { - const { organization } = this.state; - if (organization === 'loading') { - this.fetchOrganization(); - } - }); + this.fetchOrganization(); } - componentWillUnmount () { - this.setState({ mounted: false }); + async componentDidUpdate (prevProps) { + const { location } = this.props; + if (location !== prevProps.location) { + await this.fetchOrganization(); + } } async fetchOrganization () { - const { mounted } = this.state; - const { api } = this.props; - - if (mounted) { - this.setState({ error: false, loading: true }); - - const { match } = this.props; - const { parentBreadcrumbObj, organization } = this.state; - try { - const { data } = await api.getOrganizationDetails(match.params.id); - if (organization === 'loading') { - this.setState({ organization: data }); - } - const { name } = data; - if (parentBreadcrumbObj === 'loading') { - this.setState({ parentBreadcrumbObj: [{ name: i18nMark('Organizations'), url: '/organizations' }, { name, url: match.url }] }); - } - } catch (err) { - this.setState({ error: true }); - } finally { - this.setState({ loading: false }); - } + const { + api, + match, + setBreadcrumb + } = this.props; + try { + const { data } = await api.getOrganizationDetails(+match.params.id); + this.setState({ organization: data }); + setBreadcrumb(data); + } catch (error) { + this.setState({ error: true }); + } finally { + this.setState({ loading: false }); } } render () { - const { location, match, api, history } = this.props; - const { parentBreadcrumbObj, organization, error, loading } = this.state; - const params = new URLSearchParams(location.search); - const currentTab = params.get('tab') || 'details'; + const { + location, + match, + api, + history + } = this.props; + + const { + organization, + error, + loading + } = this.state; + + const tabElements = [ + { name: i18nMark('Details'), link: `${match.url}/details` }, + { name: i18nMark('Access'), link: `${match.url}/access` }, + { name: i18nMark('Teams'), link: `${match.url}/teams` }, + { name: i18nMark('Notifications'), link: `${match.url}/notifications` }, + ]; + + let cardHeader = ( + + + {({ i18n }) => ( + + {tabElements.map(tabElement => ( + + {tabElement.name} + + ))} + + )} + + + ); + + if (location.pathname.endsWith('edit')) { + cardHeader = null; + } return ( - - - + + + { cardHeader } + ( )} /> ( + )} + /> +

    Access

    } + /> +

    Teams

    } + /> + ( + )} />     {error ? 'error!' : ''} {loading ? 'loading...' : ''} -     -     + + ); } } diff --git a/src/pages/Organizations/screens/Organization/OrganizationDetail.jsx b/src/pages/Organizations/screens/Organization/OrganizationDetail.jsx index 4c9b4727e7..b7683ec5e1 100644 --- a/src/pages/Organizations/screens/Organization/OrganizationDetail.jsx +++ b/src/pages/Organizations/screens/Organization/OrganizationDetail.jsx @@ -1,115 +1,15 @@ -import React, { Fragment } from 'react'; -import { I18n } from '@lingui/react'; -import { Trans, t } from '@lingui/macro'; -import { - Card, - CardHeader, - CardBody, -} from '@patternfly/react-core'; -import { - Switch, - Link, - Route -} from 'react-router-dom'; +import React from 'react'; +import { withRouter, Link } from 'react-router-dom'; +import { Trans } from '@lingui/macro'; +import { CardBody } from '@patternfly/react-core'; -import OrganizationNotifications from './OrganizationNotifications'; +const OrganizationDetail = ({ match, organization }) => ( + +

    {`${organization && organization.name} Detail View`}

    + + Edit Details + +     +); -import Tab from '../../../../components/Tabs/Tab'; -import Tabs from '../../../../components/Tabs/Tabs'; -import getTabName from '../../utils'; - -const OrganizationDetail = ({ - location, - match, - parentBreadcrumbObj, - organization, - params, - currentTab, - api, - history -}) => { - // TODO: set objectName by param or through grabbing org detail get from api - const tabList = ['details', 'access', 'teams', 'notifications']; - - const deleteResourceView = () => ( - - {`deleting ${currentTab} association with orgs `} - - {`confirm removal of ${currentTab}/cancel and go back to ${currentTab} view.`} - - - ); - - const addResourceView = () => ( - - {`adding ${currentTab} `} - - {`save/cancel and go back to ${currentTab} view`} - - - ); - - const resourceView = () => { - let relatedTemplate; - switch (currentTab) { - case 'notifications': - relatedTemplate = ( - - ); - break; - default: - relatedTemplate = ( - - {`${currentTab} detail view `} - - {`add ${currentTab}`} - - {' '} - - {`delete ${currentTab}`} - - - ); - } - return relatedTemplate; - }; - - return ( - - - - {({ i18n }) => ( - - {tabList.map(tab => ( - - {getTabName(tab)} - - ))} - - )} - - - - - deleteResourceView()} /> - addResourceView()} /> - resourceView(props)} /> - - - - ); -}; - -export default OrganizationDetail; +export default withRouter(OrganizationDetail); diff --git a/src/pages/Organizations/screens/Organization/OrganizationEdit.jsx b/src/pages/Organizations/screens/Organization/OrganizationEdit.jsx index e72b1eba39..6ef6a469b2 100644 --- a/src/pages/Organizations/screens/Organization/OrganizationEdit.jsx +++ b/src/pages/Organizations/screens/Organization/OrganizationEdit.jsx @@ -1,22 +1,17 @@ import React from 'react'; import { Trans } from '@lingui/macro'; -import { - Card, - CardBody -} from '@patternfly/react-core'; import { Link } from 'react-router-dom'; +import { CardBody } from '@patternfly/react-core'; -const OrganizationEdit = ({ match, parentBreadcrumbObj, organization }) => ( - - - edit view - - save/cancel and go back to view - - - +const OrganizationEdit = ({ match }) => ( + +

    edit view

    + + save/cancel and go back to view + +     ); export default OrganizationEdit; diff --git a/src/pages/Organizations/screens/OrganizationsList.jsx b/src/pages/Organizations/screens/OrganizationsList.jsx index 5e3f0bd5d9..2ebb50c33b 100644 --- a/src/pages/Organizations/screens/OrganizationsList.jsx +++ b/src/pages/Organizations/screens/OrganizationsList.jsx @@ -6,11 +6,10 @@ import { withRouter } from 'react-router-dom'; import { I18n, i18nMark } from '@lingui/react'; -import { Trans, t } from '@lingui/macro'; +import { t } from '@lingui/macro'; import { PageSection, PageSectionVariants, - Title, } from '@patternfly/react-core'; import DataListToolbar from '../../../components/DataListToolbar'; @@ -177,7 +176,6 @@ class OrganizationsList extends Component { render () { const { - light, medium, } = PageSectionVariants; const { @@ -193,15 +191,9 @@ class OrganizationsList extends Component { selected, } = this.state; const { match } = this.props; - const parentBreadcrumb = { name: i18nMark('Organizations'), url: match.url }; return ( - - - <Trans>Organizations</Trans> - - { - let tabName = ''; - if (tab === 'details') { - tabName = 'Details'; - } else if (tab === 'access') { - tabName = 'Access'; - } else if (tab === 'teams') { - tabName = 'Teams'; - } else if (tab === 'notifications') { - tabName = 'Notifications'; - } - return tabName; -}; - -export default getTabName;