feat(HMS-4265): skeleton as loading indicator

Improving the loading indicators UX with skeleton. When loading RBAC or first load of domains, still show spinner as it is not known what will be displayed. When some domains were loaded, use skeleton table instead of spinner when refreshing the domains table. In details page, use skeleton as place holders for the form and title. Also fixes a bug in pagination, by removing offset, it then shows correct Y in showing X - Y out of Y + adding a bit of margin between pagination and the table. https://issues.redhat.com/browse/HMS-4265 Signed-off-by: Petr Vobornik <[email protected]>
podengo-project · Jun 19, 2024 · 83c0b76 · 83c0b76
1 parent b3e2776
commit 83c0b76
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 46 deletions.
diff --git a/src/Routes/DefaultPage/DefaultPage.tsx b/src/Routes/DefaultPage/DefaultPage.tsx
@@ -22,6 +22,9 @@ import {
   StackItem,
   Toolbar,
 } from '@patternfly/react-core';
+
+import { SkeletonTable } from '@patternfly/react-component-groups';
+
 import { PageHeader, PageHeaderTitle } from '@redhat-cloud-services/frontend-components/PageHeader';
 
 import './DefaultPage.scss';
@@ -131,11 +134,18 @@ interface ListContentProps {
   perPage: number;
   setPerPage: (value: number) => void;
   itemCount: number;
+  isLoading: boolean;
 }
 
 const ListContent = (props: ListContentProps) => {
   const { page, setPage, perPage, setPerPage, itemCount } = props;
-  const offset = (page - 1) * perPage;
+
+  let table: React.ReactNode = null;
+  if (props.isLoading) {
+    table = <SkeletonTable rows={itemCount || 4} columns={['Name', 'UUID', 'Type', 'Domain auto-join on launch', 'Actions']} />;
+  } else {
+    table = <DomainList />;
+  }
 
   return (
     <>
@@ -150,10 +160,10 @@ const ListContent = (props: ListContentProps) => {
                   </FlexItem>
                 </Flex>
               </Toolbar>
-              <DomainList />
+              {table}
               <Pagination
+                className="pf-u-mt-md"
                 dropDirection="up"
-                offset={offset}
                 itemCount={itemCount}
                 perPage={perPage}
                 page={page}
@@ -221,37 +231,42 @@ const DefaultPage = () => {
     setPage(1);
   };
 
-  const listContent = (
-    <>
-      <Header />
-      <ListContent page={page} setPage={setPage} perPage={perPage} setPerPage={changePageSize} itemCount={appContext.totalDomains} />
-    </>
-  );
-
-  const emptyContent = (
-    <>
-      <Header />
-      <EmptyContent />
-    </>
-  );
-
-  const loadingContent = (
-    <>
-      <Header />
-      <CenteredSpinner />
-    </>
-  );
-
   if (!rbac.isLoading && !hasPermissions) {
     return <NoPermissions />;
   }
 
-  if (rbac.isLoading || isLoading) {
-    return loadingContent;
+  const firstTimeLoading = isLoading && !appContext.totalDomains;
+  if (rbac.isLoading || firstTimeLoading) {
+    return (
+      <>
+        <Header />
+        <CenteredSpinner />
+      </>
+    );
   }
 
-  const content = appContext.totalDomains <= 0 ? emptyContent : listContent;
-  return content;
+  if (!isLoading && appContext.totalDomains <= 0) {
+    return (
+      <>
+        <Header />
+        <EmptyContent />
+      </>
+    );
+  }
+
+  return (
+    <>
+      <Header />
+      <ListContent
+        page={page}
+        setPage={setPage}
+        perPage={perPage}
+        setPerPage={changePageSize}
+        itemCount={appContext.totalDomains}
+        isLoading={isLoading}
+      />
+    </>
+  );
 };
 
 export default DefaultPage;
diff --git a/src/Routes/DetailPage/DetailPage.tsx b/src/Routes/DetailPage/DetailPage.tsx
@@ -12,6 +12,7 @@ import {
   MenuToggle,
   PageGroup,
   PageSection,
+  Skeleton,
   Tab,
   TabTitleText,
   Tabs,
@@ -63,7 +64,7 @@ const DetailPage = () => {
 
   // Load Domain to display
   useEffect(() => {
-    if (!hasPermissions) {
+    if (!hasPermissions || domain_id === undefined || domain) {
       return;
     }
     if (domain_id) {
@@ -132,14 +133,45 @@ const DetailPage = () => {
     return <NoPermissions />;
   }
 
+  let domainTitle: React.ReactNode = domain?.title;
+  if (!domainTitle) {
+    domainTitle = <Skeleton width="15em" />;
+  }
+
+  let body: React.ReactNode = null;
+  if (!domain) {
+    body = (
+      <div style={{ height: '340px' }}>
+        <Skeleton width="40%" height="100%" screenreaderText="Loading domain" />
+      </div>
+    );
+  } else {
+    if (activeTabKey === 0) {
+      body = (
+        <DetailGeneral
+          domain={domain}
+          onShowServerTab={() => {
+            setActiveTabKey(1);
+          }}
+          onChange={(value: Domain) => {
+            setDomain(value);
+            appContext?.updateDomain(value);
+          }}
+        />
+      );
+    } else {
+      body = <DetailServers domain={domain} />;
+    }
+  }
+
   // Return render
   return (
     <>
       <PageGroup>
         <PageHeader className="pf-v5-u-mb-0">
           <Flex>
             <FlexItem className="pf-v5-u-mr-auto">
-              <PageHeaderTitle title={domain?.title} ouiaId="TextDetailTitle" />
+              <PageHeaderTitle title={domainTitle} ouiaId="TextDetailTitle" />
             </FlexItem>
             <FlexItem>
               <Dropdown
@@ -169,7 +201,7 @@ const DetailPage = () => {
                       domain !== undefined && OnShowConfirmDelete();
                     }}
                     ouiaId="ButtonDetailsDelete"
-                    isDisabled={!rbac.permissions.hasDomainsDelete}
+                    isDisabled={!rbac.permissions.hasDomainsDelete || !domain}
                   >
                     Delete
                   </DropdownItem>
@@ -184,21 +216,7 @@ const DetailPage = () => {
         </PageHeader>
         <PageSection>
           <Card ouiaId="CardDetailPage">
-            <CardBody>
-              {activeTabKey === 0 && (
-                <DetailGeneral
-                  domain={domain}
-                  onShowServerTab={() => {
-                    setActiveTabKey(1);
-                  }}
-                  onChange={(value: Domain) => {
-                    setDomain(value);
-                    appContext?.updateDomain(value);
-                  }}
-                />
-              )}
-              {activeTabKey === 1 && <DetailServers domain={domain} />}
-            </CardBody>
+            <CardBody>{body}</CardBody>
           </Card>
         </PageSection>
       </PageGroup>