Merge pull request #6228 from jakemcdermott/6191-fix-unnecessary-panel-reload-02

Fix unnecessary panel reload and refactor top-level host and inventory routing components

Reviewed-by: https://github.com/apps/softwarefactory-project-zuul

awx/ui_next/src/screens/Host/Host.jsx

@@ -9,7 +9,7 @@ import {
   useRouteMatch,
   useLocation,
 } from 'react-router-dom';
-import { Card, CardActions } from '@patternfly/react-core';
+import { Card, CardActions, PageSection } from '@patternfly/react-core';
 
 import { TabbedCardHeader } from '@components/Card';
 import CardCloseButton from '@components/CardCloseButton';
@@ -34,11 +34,8 @@ function Host({ i18n, setBreadcrumb }) {
   useEffect(() => {
     (async () => {
       setContentError(null);
-      setHasContentLoading(true);
-
       try {
         const { data } = await HostsAPI.readDetail(match.params.id);
-
         setHost(data);
         setBreadcrumb(data);
       } catch (error) {
@@ -47,7 +44,7 @@ function Host({ i18n, setBreadcrumb }) {
         setHasContentLoading(false);
       }
     })();
-  }, [location]); // eslint-disable-line react-hooks/exhaustive-deps
+  }, [match.params.id, location, setBreadcrumb]);
 
   const tabsArray = [
     {
@@ -72,75 +69,73 @@ function Host({ i18n, setBreadcrumb }) {
     },
   ];
 
-  let cardHeader = (
-    <TabbedCardHeader>
-      <RoutedTabs tabsArray={tabsArray} />
-      <CardActions>
-        <CardCloseButton linkTo="/hosts" />
-      </CardActions>
-    </TabbedCardHeader>
-  );
-
-  if (location.pathname.endsWith('edit')) {
-    cardHeader = null;
-  }
-
   if (hasContentLoading) {
-    return <ContentLoading />;
+    return (
+      <PageSection>
+        <Card>
+          <ContentLoading />
+        </Card>
+      </PageSection>
+    );
   }
 
-  if (!hasContentLoading && contentError) {
+  if (contentError) {
     return (
-      <Card>
-        <ContentError error={contentError}>
-          {contentError.response && contentError.response.status === 404 && (
-            <span>
-              {i18n._(`Host not found.`)}{' '}
-              <Link to="/hosts">{i18n._(`View all Hosts.`)}</Link>
-            </span>
-          )}
-        </ContentError>
-      </Card>
+      <PageSection>
+        <Card>
+          <ContentError error={contentError}>
+            {contentError?.response?.status === 404 && (
+              <span>
+                {i18n._(`Host not found.`)}{' '}
+                <Link to="/hosts">{i18n._(`View all Hosts.`)}</Link>
+              </span>
+            )}
+          </ContentError>
+        </Card>
+      </PageSection>
     );
   }
 
   return (
-    <Card>
-      {cardHeader}
-      <Switch>
-        <Redirect from="/hosts/:id" to="/hosts/:id/details" exact />
-        {host && [
-          <Route path="/hosts/:id/details" key="details">
-            <HostDetail host={host} />
-          </Route>,
-          <Route path="/hosts/:id/edit" key="edit">
-            <HostEdit host={host} />
-          </Route>,
-          <Route path="/hosts/:id/facts" key="facts">
-            <HostFacts host={host} />
-          </Route>,
-          <Route path="/hosts/:id/groups" key="groups">
-            <HostGroups host={host} />
-          </Route>,
-          <Route path="/hosts/:id/completed_jobs" key="completed-jobs">
-            <JobList defaultParams={{ job__hosts: host.id }} />
-          </Route>,
-        ]}
-        <Route
-          key="not-found"
-          path="*"
-          render={() =>
-            !hasContentLoading && (
-              <ContentError isNotFound>
-                <Link to={`${match.url}/details`}>
-                  {i18n._(`View Host Details`)}
-                </Link>
-              </ContentError>
-            )
-          }
-        />
-      </Switch>
-    </Card>
+    <PageSection>
+      <Card>
+        {location.pathname.endsWith('edit') ? null : (
+          <TabbedCardHeader>
+            <RoutedTabs tabsArray={tabsArray} />
+            <CardActions>
+              <CardCloseButton linkTo="/hosts" />
+            </CardActions>
+          </TabbedCardHeader>
+        )}
+        <Switch>
+          <Redirect from="/hosts/:id" to="/hosts/:id/details" exact />
+          {host && [
+            <Route path="/hosts/:id/details" key="details">
+              <HostDetail host={host} />
+            </Route>,
+            <Route path="/hosts/:id/edit" key="edit">
+              <HostEdit host={host} />
+            </Route>,
+            <Route path="/hosts/:id/facts" key="facts">
+              <HostFacts host={host} />
+            </Route>,
+            <Route path="/hosts/:id/groups" key="groups">
+              <HostGroups host={host} />
+            </Route>,
+            <Route path="/hosts/:id/completed_jobs" key="completed-jobs">
+              <JobList defaultParams={{ job__hosts: host.id }} />
+            </Route>,
+          ]}
+          <Route key="not-found" path="*">
+            <ContentError isNotFound>
+              <Link to={`${match.url}/details`}>
+                {i18n._(`View Host Details`)}
+              </Link>
+            </ContentError>
+          </Route>
+        </Switch>
+      </Card>
+    </PageSection>
   );
 }
 

awx/ui_next/src/screens/Host/HostList/HostList.jsx

@@ -1,8 +1,8 @@
-import React, { Fragment, useState, useEffect, useCallback } from 'react';
+import React, { useState, useEffect, useCallback } from 'react';
 import { useLocation, useRouteMatch } from 'react-router-dom';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
-import { Card } from '@patternfly/react-core';
+import { Card, PageSection } from '@patternfly/react-core';
 
 import { HostsAPI } from '@api';
 import AlertModal from '@components/AlertModal';
@@ -95,7 +95,7 @@ function HostList({ i18n }) {
     actions && Object.prototype.hasOwnProperty.call(actions, 'POST');
 
   return (
-    <Fragment>
+    <PageSection>
       <Card>
         <PaginatedDataList
           contentError={contentError}
@@ -173,7 +173,7 @@ function HostList({ i18n }) {
           <ErrorDetail error={deletionError} />
         </AlertModal>
       )}
-    </Fragment>
+    </PageSection>
   );
 }
 

awx/ui_next/src/screens/Host/Hosts.jsx

@@ -4,7 +4,6 @@ import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 
 import { Config } from '@contexts/Config';
-import { PageSection, Card } from '@patternfly/react-core';
 import Breadcrumbs from '@components/Breadcrumbs/Breadcrumbs';
 
 import HostList from './HostList';
@@ -39,25 +38,21 @@ function Hosts({ i18n }) {
   return (
     <>
       <Breadcrumbs breadcrumbConfig={breadcrumbConfig} />
-      <PageSection>
-        <Card>
-          <Switch>
-            <Route path="/hosts/add">
-              <HostAdd />
-            </Route>
-            <Route path="/hosts/:id">
-              <Config>
-                {({ me }) => (
-                  <Host setBreadcrumb={buildBreadcrumbConfig} me={me || {}} />
-                )}
-              </Config>
-            </Route>
-            <Route path="/hosts">
-              <HostList />
-            </Route>
-          </Switch>
-        </Card>
-      </PageSection>
+      <Switch>
+        <Route path="/hosts/add">
+          <HostAdd />
+        </Route>
+        <Route path="/hosts/:id">
+          <Config>
+            {({ me }) => (
+              <Host setBreadcrumb={buildBreadcrumbConfig} me={me || {}} />
+            )}
+          </Config>
+        </Route>
+        <Route path="/hosts">
+          <HostList />
+        </Route>
+      </Switch>
     </>
   );
 }

awx/ui_next/src/screens/Inventory/Inventories.jsx

@@ -1,4 +1,4 @@
-import React, { Component, Fragment } from 'react';
+import React, { Component } from 'react';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 import { Route, withRouter, Switch } from 'react-router-dom';
@@ -95,7 +95,7 @@ class Inventories extends Component {
     const { match, history, location } = this.props;
     const { breadcrumbConfig } = this.state;
     return (
-      <Fragment>
+      <>
         <Breadcrumbs breadcrumbConfig={breadcrumbConfig} />
         <Switch>
           <Route
@@ -134,7 +134,7 @@ class Inventories extends Component {
           />
           <Route path={`${match.path}`} render={() => <InventoryList />} />
         </Switch>
-      </Fragment>
+      </>
     );
   }
 }

awx/ui_next/src/screens/Inventory/Inventory.jsx

@@ -63,24 +63,6 @@ function Inventory({ i18n, setBreadcrumb }) {
     },
   ];
 
-  let cardHeader = hasContentLoading ? null : (
-    <TabbedCardHeader>
-      <RoutedTabs tabsArray={tabsArray} />
-      <CardActions>
-        <CardCloseButton linkTo="/inventories" />
-      </CardActions>
-    </TabbedCardHeader>
-  );
-
-  if (
-    location.pathname.endsWith('edit') ||
-    location.pathname.endsWith('add') ||
-    location.pathname.includes('groups/') ||
-    location.pathname.includes('hosts/')
-  ) {
-    cardHeader = null;
-  }
-
   if (hasContentLoading) {
     return (
       <PageSection>
@@ -91,12 +73,12 @@ function Inventory({ i18n, setBreadcrumb }) {
     );
   }
 
-  if (!hasContentLoading && contentError) {
+  if (contentError) {
     return (
       <PageSection>
         <Card>
           <ContentError error={contentError}>
-            {contentError.response.status === 404 && (
+            {contentError.response?.status === 404 && (
               <span>
                 {i18n._(`Inventory not found.`)}{' '}
                 <Link to="/inventories">{i18n._(`View all Inventories.`)}</Link>
@@ -111,7 +93,16 @@ function Inventory({ i18n, setBreadcrumb }) {
   return (
     <PageSection>
       <Card>
-        {cardHeader}
+        {['edit', 'add', 'groups/', 'hosts/'].some(name =>
+          location.pathname.includes(name)
+        ) ? null : (
+          <TabbedCardHeader>
+            <RoutedTabs tabsArray={tabsArray} />
+            <CardActions>
+              <CardCloseButton linkTo="/inventories" />
+            </CardActions>
+          </TabbedCardHeader>
+        )}
         <Switch>
           <Redirect
             from="/inventories/inventory/:id"
@@ -119,59 +110,39 @@ function Inventory({ i18n, setBreadcrumb }) {
             exact
           />
           {inventory && [
+            <Route path="/inventories/inventory/:id/details" key="details">
+              <InventoryDetail
+                inventory={inventory}
+                hasInventoryLoading={hasContentLoading}
+              />
+            </Route>,
+            <Route path="/inventories/inventory/:id/edit" key="edit">
+              <InventoryEdit inventory={inventory} />
+            </Route>,
+            <Route path="/inventories/inventory/:id/hosts" key="hosts">
+              <InventoryHosts
+                inventory={inventory}
+                setBreadcrumb={setBreadcrumb}
+              />
+            </Route>,
+            <Route path="/inventories/inventory/:id/access" key="access">
+              <ResourceAccessList
+                resource={inventory}
+                apiModel={InventoriesAPI}
+              />
+            </Route>,
+            <Route path="/inventories/inventory/:id/groups" key="groups">
+              <InventoryGroups
+                inventory={inventory}
+                setBreadcrumb={setBreadcrumb}
+              />
+            </Route>,
+            <Route path="/inventories/inventory/:id/sources" key="sources">
+              <InventorySources inventory={inventory} />
+            </Route>,
             <Route
-              key="details"
-              path="/inventories/inventory/:id/details"
-              render={() => (
-                <InventoryDetail
-                  hasInventoryLoading={hasContentLoading}
-                  inventory={inventory}
-                />
-              )}
-            />,
-            <Route
-              key="edit"
-              path="/inventories/inventory/:id/edit"
-              render={() => <InventoryEdit inventory={inventory} />}
-            />,
-            <Route
-              key="hosts"
-              path="/inventories/inventory/:id/hosts"
-              render={() => (
-                <InventoryHosts
-                  setBreadcrumb={setBreadcrumb}
-                  inventory={inventory}
-                />
-              )}
-            />,
-            <Route
-              key="access"
-              path="/inventories/inventory/:id/access"
-              render={() => (
-                <ResourceAccessList
-                  resource={inventory}
-                  apiModel={InventoriesAPI}
-                />
-              )}
-            />,
-            <Route
-              key="groups"
-              path="/inventories/inventory/:id/groups"
-              render={() => (
-                <InventoryGroups
-                  setBreadcrumb={setBreadcrumb}
-                  inventory={inventory}
-                />
-              )}
-            />,
-            <Route
-              key="sources"
-              path="/inventories/inventory/:id/sources"
-              render={() => <InventorySources inventory={inventory} />}
-            />,
-            <Route
-              key="completed_jobs"
               path="/inventories/inventory/:id/completed_jobs"
+              key="completed_jobs"
             >
               <JobList
                 defaultParams={{
@@ -183,23 +154,17 @@ function Inventory({ i18n, setBreadcrumb }) {
                 }}
               />
             </Route>,
-            <Route
-              key="not-found"
-              path="*"
-              render={() =>
-                !hasContentLoading && (
-                  <ContentError isNotFound>
-                    {match.params.id && (
-                      <Link
-                        to={`/inventories/inventory/${match.params.id}/details`}
-                      >
-                        {i18n._(`View Inventory Details`)}
-                      </Link>
-                    )}
-                  </ContentError>
-                )
-              }
-            />,
+            <Route path="*" key="not-found">
+              <ContentError isNotFound>
+                {match.params.id && (
+                  <Link
+                    to={`/inventories/inventory/${match.params.id}/details`}
+                  >
+                    {i18n._(`View Inventory Details`)}
+                  </Link>
+                )}
+              </ContentError>
+            </Route>,
           ]}
         </Switch>
       </Card>