From cedcd977ec9b5af0953e3358f79292613cafb775 Mon Sep 17 00:00:00 2001
From: Ashley Harrison <ashley.harrison@grafana.com>
Date: Thu, 7 Dec 2023 14:20:13 +0000
Subject: [PATCH] Service accounts: Add skeleton loader (#79224)

* use emotion object syntax

* move create button to be a page action

* create and use skeleton
---
 .betterer.results                             |  20 ---
 .../ServiceAccountsListPage.tsx               | 149 ++++++++++--------
 .../components/ServiceAccountsListItem.tsx    | 111 +++++++++----
 3 files changed, 161 insertions(+), 119 deletions(-)

diff --git a/.betterer.results b/.betterer.results
index b33c8a5678c..b8eba4d816f 100644
--- a/.betterer.results
+++ b/.betterer.results
@@ -4308,18 +4308,6 @@ exports[`better eslint`] = {
     "public/app/features/search/utils.ts:5381": [
       [0, 0, 0, "Do not use any type assertions.", "0"]
     ],
-    "public/app/features/serviceaccounts/ServiceAccountsListPage.tsx:5381": [
-      [0, 0, 0, "Styles should be written using objects.", "0"],
-      [0, 0, 0, "Styles should be written using objects.", "1"],
-      [0, 0, 0, "Styles should be written using objects.", "2"],
-      [0, 0, 0, "Styles should be written using objects.", "3"],
-      [0, 0, 0, "Styles should be written using objects.", "4"],
-      [0, 0, 0, "Styles should be written using objects.", "5"],
-      [0, 0, 0, "Styles should be written using objects.", "6"],
-      [0, 0, 0, "Styles should be written using objects.", "7"],
-      [0, 0, 0, "Styles should be written using objects.", "8"],
-      [0, 0, 0, "Styles should be written using objects.", "9"]
-    ],
     "public/app/features/serviceaccounts/components/CreateTokenModal.tsx:5381": [
       [0, 0, 0, "Styles should be written using objects.", "0"],
       [0, 0, 0, "Styles should be written using objects.", "1"],
@@ -4341,14 +4329,6 @@ exports[`better eslint`] = {
       [0, 0, 0, "Styles should be written using objects.", "5"],
       [0, 0, 0, "Styles should be written using objects.", "6"]
     ],
-    "public/app/features/serviceaccounts/components/ServiceAccountsListItem.tsx:5381": [
-      [0, 0, 0, "Styles should be written using objects.", "0"],
-      [0, 0, 0, "Styles should be written using objects.", "1"],
-      [0, 0, 0, "Styles should be written using objects.", "2"],
-      [0, 0, 0, "Styles should be written using objects.", "3"],
-      [0, 0, 0, "Styles should be written using objects.", "4"],
-      [0, 0, 0, "Styles should be written using objects.", "5"]
-    ],
     "public/app/features/serviceaccounts/state/reducers.ts:5381": [
       [0, 0, 0, "Do not use any type assertions.", "0"]
     ],
diff --git a/public/app/features/serviceaccounts/ServiceAccountsListPage.tsx b/public/app/features/serviceaccounts/ServiceAccountsListPage.tsx
index 79411995351..46737866d50 100644
--- a/public/app/features/serviceaccounts/ServiceAccountsListPage.tsx
+++ b/public/app/features/serviceaccounts/ServiceAccountsListPage.tsx
@@ -16,7 +16,6 @@ import {
 } from '@grafana/ui';
 import EmptyListCTA from 'app/core/components/EmptyListCTA/EmptyListCTA';
 import { Page } from 'app/core/components/Page/Page';
-import PageLoader from 'app/core/components/PageLoader/PageLoader';
 import config from 'app/core/config';
 import { contextSrv } from 'app/core/core';
 import { StoreState, ServiceAccountDTO, AccessControlAction, ServiceAccountStateFilter } from 'app/types';
@@ -190,7 +189,19 @@ export const ServiceAccountsListPageUnconnected = ({
   );
 
   return (
-    <Page navId="serviceaccounts" subTitle={subTitle}>
+    <Page
+      navId="serviceaccounts"
+      subTitle={subTitle}
+      actions={
+        <>
+          {!noServiceAccountsCreated && contextSrv.hasPermission(AccessControlAction.ServiceAccountsCreate) && (
+            <LinkButton href="org/serviceaccounts/create" variant="primary">
+              Add service account
+            </LinkButton>
+          )}
+        </>
+      }
+    >
       <Page.Contents>
         <div className="page-action-bar">
           <InlineField grow>
@@ -207,13 +218,7 @@ export const ServiceAccountsListPageUnconnected = ({
             value={serviceAccountStateFilter}
             className={styles.filter}
           />
-          {!noServiceAccountsCreated && contextSrv.hasPermission(AccessControlAction.ServiceAccountsCreate) && (
-            <LinkButton href="org/serviceaccounts/create" variant="primary">
-              Add service account
-            </LinkButton>
-          )}
         </div>
-        {isLoading && <PageLoader />}
         {!isLoading && noServiceAccountsCreated && (
           <>
             <EmptyListCTA
@@ -230,7 +235,7 @@ export const ServiceAccountsListPageUnconnected = ({
           </>
         )}
 
-        {!isLoading && serviceAccounts.length !== 0 && (
+        {(isLoading || serviceAccounts.length !== 0) && (
           <>
             <div className={cx(styles.table, 'admin-list-table')}>
               <table className="filter-table filter-table--hover">
@@ -245,18 +250,26 @@ export const ServiceAccountsListPageUnconnected = ({
                   </tr>
                 </thead>
                 <tbody>
-                  {serviceAccounts.map((serviceAccount: ServiceAccountDTO) => (
-                    <ServiceAccountListItem
-                      serviceAccount={serviceAccount}
-                      key={serviceAccount.id}
-                      roleOptions={roleOptions}
-                      onRoleChange={onRoleChange}
-                      onRemoveButtonClick={onRemoveButtonClick}
-                      onDisable={onDisableButtonClick}
-                      onEnable={onEnable}
-                      onAddTokenClick={onTokenAdd}
-                    />
-                  ))}
+                  {isLoading ? (
+                    <>
+                      <ServiceAccountListItem.Skeleton />
+                      <ServiceAccountListItem.Skeleton />
+                      <ServiceAccountListItem.Skeleton />
+                    </>
+                  ) : (
+                    serviceAccounts.map((serviceAccount) => (
+                      <ServiceAccountListItem
+                        serviceAccount={serviceAccount}
+                        key={serviceAccount.id}
+                        roleOptions={roleOptions}
+                        onRoleChange={onRoleChange}
+                        onRemoveButtonClick={onRemoveButtonClick}
+                        onDisable={onDisableButtonClick}
+                        onEnable={onEnable}
+                        onAddTokenClick={onTokenAdd}
+                      />
+                    ))
+                  )}
                 </tbody>
               </table>
 
@@ -307,55 +320,55 @@ export const ServiceAccountsListPageUnconnected = ({
 
 export const getStyles = (theme: GrafanaTheme2) => {
   return {
-    table: css`
-      margin-top: ${theme.spacing(3)};
-    `,
-    filter: css`
-      margin: 0 ${theme.spacing(1)};
-    `,
-    row: css`
-      display: flex;
-      align-items: center;
-      height: 100% !important;
+    table: css({
+      marginTop: theme.spacing(3),
+    }),
+    filter: css({
+      margin: `0 ${theme.spacing(1)}`,
+    }),
+    row: css({
+      display: 'flex',
+      alignItems: 'center',
+      height: '100% !important',
 
-      a {
-        padding: ${theme.spacing(0.5)} 0 !important;
-      }
-    `,
-    unitTooltip: css`
-      display: flex;
-      flex-direction: column;
-    `,
-    unitItem: css`
-      cursor: pointer;
-      padding: ${theme.spacing(0.5)} 0;
-      margin-right: ${theme.spacing(1)};
-    `,
-    disabled: css`
-      color: ${theme.colors.text.disabled};
-    `,
-    link: css`
-      color: inherit;
-      cursor: pointer;
-      text-decoration: underline;
-    `,
-    pageHeader: css`
-      display: flex;
-      margin-bottom: ${theme.spacing(2)};
-    `,
-    apiKeyInfoLabel: css`
-      margin-left: ${theme.spacing(1)};
-      line-height: 2.2;
-      flex-grow: 1;
-      color: ${theme.colors.text.secondary};
+      a: {
+        padding: `${theme.spacing(0.5)} 0 !important`,
+      },
+    }),
+    unitTooltip: css({
+      display: 'flex',
+      flexDirection: 'column',
+    }),
+    unitItem: css({
+      cursor: 'pointer',
+      padding: theme.spacing(0.5, 0),
+      marginRight: theme.spacing(1),
+    }),
+    disabled: css({
+      color: theme.colors.text.disabled,
+    }),
+    link: css({
+      color: 'inherit',
+      cursor: 'pointer',
+      textDecoration: 'underline',
+    }),
+    pageHeader: css({
+      display: 'flex',
+      marginBottom: theme.spacing(2),
+    }),
+    apiKeyInfoLabel: css({
+      marginLeft: theme.spacing(1),
+      lineHeight: 2.2,
+      flexGrow: 1,
+      color: theme.colors.text.secondary,
 
-      span {
-        padding: ${theme.spacing(0.5)};
-      }
-    `,
-    filterDelimiter: css`
-      flex-grow: 1;
-    `,
+      span: {
+        padding: theme.spacing(0.5),
+      },
+    }),
+    filterDelimiter: css({
+      flexGrow: 1,
+    }),
   };
 };
 
diff --git a/public/app/features/serviceaccounts/components/ServiceAccountsListItem.tsx b/public/app/features/serviceaccounts/components/ServiceAccountsListItem.tsx
index 3031d76f2e6..ed78f6da724 100644
--- a/public/app/features/serviceaccounts/components/ServiceAccountsListItem.tsx
+++ b/public/app/features/serviceaccounts/components/ServiceAccountsListItem.tsx
@@ -1,8 +1,9 @@
 import { css, cx } from '@emotion/css';
 import React, { memo } from 'react';
+import Skeleton from 'react-loading-skeleton';
 
 import { GrafanaTheme2, OrgRole } from '@grafana/data';
-import { Button, HorizontalGroup, Icon, IconButton, useStyles2 } from '@grafana/ui';
+import { Button, Icon, IconButton, Stack, useStyles2 } from '@grafana/ui';
 import { UserRolePicker } from 'app/core/components/RolePicker/UserRolePicker';
 import { contextSrv } from 'app/core/core';
 import { OrgRolePicker } from 'app/features/admin/OrgRolePicker';
@@ -22,7 +23,7 @@ const getServiceAccountsAriaLabel = (name: string) => {
   return `Edit service account's ${name} details`;
 };
 
-const ServiceAccountListItem = memo(
+const ServiceAccountListItemComponent = memo(
   ({
     serviceAccount,
     onRoleChange,
@@ -113,7 +114,7 @@ const ServiceAccountListItem = memo(
         </td>
         <td>
           {!serviceAccount.isExternal && (
-            <HorizontalGroup justify="flex-end">
+            <Stack alignItems="center" justifyContent="flex-end">
               {contextSrv.hasPermission(AccessControlAction.ServiceAccountsWrite) && !serviceAccount.tokens && (
                 <Button
                   onClick={() => onAddTokenClick(serviceAccount)}
@@ -142,54 +143,102 @@ const ServiceAccountListItem = memo(
                   tooltip={`Delete service account ${serviceAccount.name}`}
                 />
               )}
-            </HorizontalGroup>
+            </Stack>
           )}
           {serviceAccount.isExternal && (
-            <HorizontalGroup justify="flex-end">
+            <Stack alignItems="center" justifyContent="flex-end">
               <IconButton
                 disabled={true}
                 name="lock"
                 size="md"
                 tooltip={`This is a managed service account and cannot be modified.`}
               />
-            </HorizontalGroup>
+            </Stack>
           )}
         </td>
       </tr>
     );
   }
 );
-ServiceAccountListItem.displayName = 'ServiceAccountListItem';
+ServiceAccountListItemComponent.displayName = 'ServiceAccountListItem';
+
+const ServiceAccountsListItemSkeleton = () => {
+  const styles = useStyles2(getSkeletonStyles);
+
+  return (
+    <tr>
+      <td className="width-4 text-center">
+        <Skeleton containerClassName={styles.blockSkeleton} circle width={25} height={25} />
+      </td>
+      <td className="max-width-10">
+        <Skeleton width={100} />
+      </td>
+      <td className="max-width-10">
+        <Skeleton width={100} />
+      </td>
+      <td>
+        <Skeleton containerClassName={styles.blockSkeleton} width="100%" height={32} />
+      </td>
+      <td className="max-width-10">
+        <Skeleton width={40} />
+      </td>
+      <td>
+        <Stack alignItems="center" justifyContent="flex-end">
+          <Skeleton containerClassName={styles.blockSkeleton} width={102} height={32} />
+          <Skeleton containerClassName={styles.blockSkeleton} width={85} height={32} />
+          <Skeleton containerClassName={cx(styles.blockSkeleton, styles.deleteButton)} width={16} height={16} />
+        </Stack>
+      </td>
+    </tr>
+  );
+};
+
+interface ServiceAccountsListItemWithSkeleton extends React.NamedExoticComponent<ServiceAccountListItemProps> {
+  Skeleton: typeof ServiceAccountsListItemSkeleton;
+}
+const ServiceAccountListItem: ServiceAccountsListItemWithSkeleton = Object.assign(ServiceAccountListItemComponent, {
+  Skeleton: ServiceAccountsListItemSkeleton,
+});
+
+const getSkeletonStyles = (theme: GrafanaTheme2) => ({
+  blockSkeleton: css({
+    display: 'block',
+    lineHeight: 1,
+  }),
+  deleteButton: css({
+    marginRight: theme.spacing(0.5),
+  }),
+});
 
 const getStyles = (theme: GrafanaTheme2) => {
   return {
-    iconRow: css`
-      svg {
-        margin-left: ${theme.spacing(0.5)};
-      }
-    `,
+    iconRow: css({
+      svg: {
+        marginLeft: theme.spacing(0.5),
+      },
+    }),
     accountId: cx(
       'ellipsis',
-      css`
-        color: ${theme.colors.text.secondary};
-      `
+      css({
+        color: theme.colors.text.secondary,
+      })
     ),
-    deleteButton: css`
-      color: ${theme.colors.text.secondary};
-    `,
-    tokensInfo: css`
-      span {
-        margin-right: ${theme.spacing(1)};
-      }
-    `,
-    tokensInfoSecondary: css`
-      color: ${theme.colors.text.secondary};
-    `,
-    disabled: css`
-      td a {
-        color: ${theme.colors.text.secondary};
-      }
-    `,
+    deleteButton: css({
+      color: theme.colors.text.secondary,
+    }),
+    tokensInfo: css({
+      span: {
+        marginRight: theme.spacing(1),
+      },
+    }),
+    tokensInfoSecondary: css({
+      color: theme.colors.text.secondary,
+    }),
+    disabled: css({
+      'td a': {
+        color: theme.colors.text.secondary,
+      },
+    }),
     actionButton: css({
       minWidth: 85,
     }),