From 0e28d6143b0fe1b729e401096c9d269ef7a53203 Mon Sep 17 00:00:00 2001
From: Ashley Harrison <ashley.harrison@grafana.com>
Date: Wed, 7 Jun 2023 12:21:44 +0200
Subject: [PATCH] Nested folders: Improve loading states (#69556)

* loading states!

* fix uncontrolled checkbox

* cleaner css

* improve flickering title

* make sure @grafana/ui has the same version of react-loading-skeleton

* fix unit test + add tooltip text

* better way of restoring focus

* only restore focus when loading

* missing !

* use aria-label instead of tooltip
---
 packages/grafana-ui/package.json              |  1 +
 .../grafana-ui/src/components/Tags/Tag.tsx    | 25 ++++++-
 .../src/components/Tags/TagList.tsx           | 30 +++++++-
 .../core/components/Page/EditableTitle.tsx    |  2 +-
 .../components/BrowseView.tsx                 | 11 +--
 .../components/CheckboxCell.tsx               | 24 ++++++-
 .../components/DashboardsTree.test.tsx        |  4 +-
 .../components/DashboardsTree.tsx             |  2 +-
 .../browse-dashboards/components/NameCell.tsx | 72 ++++++++++++++-----
 .../browse-dashboards/components/TagsCell.tsx | 12 +++-
 .../browse-dashboards/components/TypeCell.tsx | 48 ++++++++++---
 .../fixtures/dashboardsTreeItem.fixture.ts    |  2 +-
 .../features/browse-dashboards/state/hooks.ts |  2 +-
 yarn.lock                                     |  1 +
 14 files changed, 183 insertions(+), 53 deletions(-)

diff --git a/packages/grafana-ui/package.json b/packages/grafana-ui/package.json
index 1d50e791ac3..2d51b7617cc 100644
--- a/packages/grafana-ui/package.json
+++ b/packages/grafana-ui/package.json
@@ -94,6 +94,7 @@
     "react-hook-form": "7.5.3",
     "react-i18next": "^12.0.0",
     "react-inlinesvg": "3.0.2",
+    "react-loading-skeleton": "3.3.1",
     "react-popper": "2.3.0",
     "react-popper-tooltip": "4.4.2",
     "react-router-dom": "5.3.3",
diff --git a/packages/grafana-ui/src/components/Tags/Tag.tsx b/packages/grafana-ui/src/components/Tags/Tag.tsx
index 067560c0371..11cfa0dffc4 100644
--- a/packages/grafana-ui/src/components/Tags/Tag.tsx
+++ b/packages/grafana-ui/src/components/Tags/Tag.tsx
@@ -1,9 +1,10 @@
 import { cx, css } from '@emotion/css';
 import React, { forwardRef, HTMLAttributes } from 'react';
+import Skeleton from 'react-loading-skeleton';
 
 import { GrafanaTheme2 } from '@grafana/data';
 
-import { useTheme2 } from '../../themes';
+import { useStyles2, useTheme2 } from '../../themes';
 import { IconName } from '../../types/icon';
 import { getTagColor, getTagColorsFromName } from '../../utils';
 import { Icon } from '../Icon/Icon';
@@ -22,7 +23,7 @@ export interface Props extends Omit<HTMLAttributes<HTMLElement>, 'onClick'> {
   onClick?: OnTagClick;
 }
 
-export const Tag = forwardRef<HTMLElement, Props>(({ name, onClick, icon, className, colorIndex, ...rest }, ref) => {
+const TagComponent = forwardRef<HTMLElement, Props>(({ name, onClick, icon, className, colorIndex, ...rest }, ref) => {
   const theme = useTheme2();
   const styles = getTagStyles(theme, name, colorIndex);
 
@@ -47,8 +48,26 @@ export const Tag = forwardRef<HTMLElement, Props>(({ name, onClick, icon, classN
     </span>
   );
 });
+TagComponent.displayName = 'Tag';
 
-Tag.displayName = 'Tag';
+const TagSkeleton = () => {
+  const styles = useStyles2(getSkeletonStyles);
+  return <Skeleton width={60} height={22} containerClassName={styles.container} />;
+};
+
+interface TagWithSkeleton extends React.ForwardRefExoticComponent<Props & React.RefAttributes<HTMLElement>> {
+  Skeleton: typeof TagSkeleton;
+}
+
+export const Tag: TagWithSkeleton = Object.assign(TagComponent, {
+  Skeleton: TagSkeleton,
+});
+
+const getSkeletonStyles = () => ({
+  container: css({
+    lineHeight: 1,
+  }),
+});
 
 const getTagStyles = (theme: GrafanaTheme2, name: string, colorIndex?: number) => {
   let colors;
diff --git a/packages/grafana-ui/src/components/Tags/TagList.tsx b/packages/grafana-ui/src/components/Tags/TagList.tsx
index 20ea4d66fe1..b9c8af15446 100644
--- a/packages/grafana-ui/src/components/Tags/TagList.tsx
+++ b/packages/grafana-ui/src/components/Tags/TagList.tsx
@@ -3,7 +3,7 @@ import React, { forwardRef, memo } from 'react';
 
 import { GrafanaTheme2 } from '@grafana/data';
 
-import { useTheme2 } from '../../themes';
+import { useStyles2, useTheme2 } from '../../themes';
 import { IconName } from '../../types/icon';
 
 import { OnTagClick, Tag } from './Tag';
@@ -23,7 +23,7 @@ export interface Props {
   icon?: IconName;
 }
 
-export const TagList = memo(
+const TagListComponent = memo(
   forwardRef<HTMLUListElement, Props>(({ displayMax, tags, icon, onClick, className, getAriaLabel }, ref) => {
     const theme = useTheme2();
     const styles = getStyles(theme, Boolean(displayMax && displayMax > 0));
@@ -43,8 +43,32 @@ export const TagList = memo(
     );
   })
 );
+TagListComponent.displayName = 'TagList';
 
-TagList.displayName = 'TagList';
+const TagListSkeleton = () => {
+  const styles = useStyles2(getSkeletonStyles);
+  return (
+    <div className={styles.container}>
+      <Tag.Skeleton />
+      <Tag.Skeleton />
+    </div>
+  );
+};
+
+interface TagListWithSkeleton extends React.ForwardRefExoticComponent<Props & React.RefAttributes<HTMLUListElement>> {
+  Skeleton: typeof TagListSkeleton;
+}
+
+export const TagList: TagListWithSkeleton = Object.assign(TagListComponent, {
+  Skeleton: TagListSkeleton,
+});
+
+const getSkeletonStyles = (theme: GrafanaTheme2) => ({
+  container: css({
+    display: 'flex',
+    gap: theme.spacing(1),
+  }),
+});
 
 const getStyles = (theme: GrafanaTheme2, isTruncated: boolean) => {
   return {
diff --git a/public/app/core/components/Page/EditableTitle.tsx b/public/app/core/components/Page/EditableTitle.tsx
index ffe380a4e3b..caf051ddd5a 100644
--- a/public/app/core/components/Page/EditableTitle.tsx
+++ b/public/app/core/components/Page/EditableTitle.tsx
@@ -13,7 +13,7 @@ export interface Props {
 
 export const EditableTitle = ({ value, onEdit }: Props) => {
   const styles = useStyles2(getStyles);
-  const [localValue, setLocalValue] = useState<string>();
+  const [localValue, setLocalValue] = useState(value);
   const [isEditing, setIsEditing] = useState(false);
   const [isLoading, setIsLoading] = useState(false);
   const [errorMessage, setErrorMessage] = useState<string>();
diff --git a/public/app/features/browse-dashboards/components/BrowseView.tsx b/public/app/features/browse-dashboards/components/BrowseView.tsx
index 56f6acad0c2..c1236db1378 100644
--- a/public/app/features/browse-dashboards/components/BrowseView.tsx
+++ b/public/app/features/browse-dashboards/components/BrowseView.tsx
@@ -1,6 +1,5 @@
-import React, { useCallback, useEffect, useRef } from 'react';
+import React, { useCallback, useRef } from 'react';
 
-import { Spinner } from '@grafana/ui';
 import EmptyListCTA from 'app/core/components/EmptyListCTA/EmptyListCTA';
 import { DashboardViewItem } from 'app/features/search/types';
 import { useDispatch } from 'app/types';
@@ -45,10 +44,6 @@ export function BrowseView({ folderUID, width, height, canSelect }: BrowseViewPr
     [dispatch]
   );
 
-  useEffect(() => {
-    dispatch(fetchNextChildrenPage({ parentUID: folderUID, pageSize: ROOT_PAGE_SIZE }));
-  }, [handleFolderClick, dispatch, folderUID]);
-
   const handleItemSelectionChange = useCallback(
     (item: DashboardViewItem, isSelected: boolean) => {
       dispatch(setItemSelectionState({ item, isSelected }));
@@ -116,10 +111,6 @@ export function BrowseView({ folderUID, width, height, canSelect }: BrowseViewPr
 
   const handleLoadMore = useLoadNextChildrenPage(folderUID);
 
-  if (status === 'pending') {
-    return <Spinner />;
-  }
-
   if (status === 'fulfilled' && flatTree.length === 0) {
     return (
       <div style={{ width }}>
diff --git a/public/app/features/browse-dashboards/components/CheckboxCell.tsx b/public/app/features/browse-dashboards/components/CheckboxCell.tsx
index d2de4f46cdc..d00cb921c0e 100644
--- a/public/app/features/browse-dashboards/components/CheckboxCell.tsx
+++ b/public/app/features/browse-dashboards/components/CheckboxCell.tsx
@@ -1,7 +1,9 @@
+import { css } from '@emotion/css';
 import React from 'react';
 
+import { GrafanaTheme2 } from '@grafana/data';
 import { selectors } from '@grafana/e2e-selectors';
-import { Checkbox } from '@grafana/ui';
+import { Checkbox, useStyles2 } from '@grafana/ui';
 
 import { DashboardsTreeCellProps, SelectionState } from '../types';
 
@@ -10,10 +12,19 @@ export default function CheckboxCell({
   isSelected,
   onItemSelectionChange,
 }: DashboardsTreeCellProps) {
+  const styles = useStyles2(getStyles);
   const item = row.item;
 
-  if (item.kind === 'ui' || !isSelected) {
-    return null;
+  if (!isSelected) {
+    return <span className={styles.checkboxSpacer} />;
+  }
+
+  if (item.kind === 'ui') {
+    if (item.uiKind === 'pagination-placeholder') {
+      return <Checkbox disabled value={false} />;
+    } else {
+      return <span className={styles.checkboxSpacer} />;
+    }
   }
 
   const state = isSelected(item);
@@ -27,3 +38,10 @@ export default function CheckboxCell({
     />
   );
 }
+
+const getStyles = (theme: GrafanaTheme2) => ({
+  // Should be the same size as the <IconButton /> so Dashboard name is aligned to Folder name siblings
+  checkboxSpacer: css({
+    paddingLeft: theme.spacing(2),
+  }),
+});
diff --git a/public/app/features/browse-dashboards/components/DashboardsTree.test.tsx b/public/app/features/browse-dashboards/components/DashboardsTree.test.tsx
index e732344a9c2..ff29ea9f9b2 100644
--- a/public/app/features/browse-dashboards/components/DashboardsTree.test.tsx
+++ b/public/app/features/browse-dashboards/components/DashboardsTree.test.tsx
@@ -103,10 +103,10 @@ describe('browse-dashboards DashboardsTree', () => {
         requestLoadMore={requestLoadMore}
       />
     );
-    const folderButton = screen.getByLabelText('Collapse folder');
+    const folderButton = screen.getByLabelText('Expand folder');
     await userEvent.click(folderButton);
 
-    expect(handler).toHaveBeenCalledWith(folder.item.uid, false);
+    expect(handler).toHaveBeenCalledWith(folder.item.uid, true);
   });
 
   it('renders empty folder indicators', () => {
diff --git a/public/app/features/browse-dashboards/components/DashboardsTree.tsx b/public/app/features/browse-dashboards/components/DashboardsTree.tsx
index 4d8e6a29f61..7dbd29514ba 100644
--- a/public/app/features/browse-dashboards/components/DashboardsTree.tsx
+++ b/public/app/features/browse-dashboards/components/DashboardsTree.tsx
@@ -76,7 +76,7 @@ export function DashboardsTree({
     const nameColumn: DashboardsTreeColumn = {
       id: 'name',
       width: 3,
-      Header: <span style={{ paddingLeft: 20 }}>Name</span>,
+      Header: <span style={{ paddingLeft: 24 }}>Name</span>,
       Cell: (props: DashboardsTreeCellProps) => <NameCell {...props} onFolderClick={onFolderClick} />,
     };
 
diff --git a/public/app/features/browse-dashboards/components/NameCell.tsx b/public/app/features/browse-dashboards/components/NameCell.tsx
index 2238d8d430a..86f48ced71e 100644
--- a/public/app/features/browse-dashboards/components/NameCell.tsx
+++ b/public/app/features/browse-dashboards/components/NameCell.tsx
@@ -1,16 +1,20 @@
 import { css } from '@emotion/css';
-import React from 'react';
+import React, { useEffect, useRef, useState } from 'react';
+import Skeleton from 'react-loading-skeleton';
 import { CellProps } from 'react-table';
 
 import { GrafanaTheme2 } from '@grafana/data';
-import { IconButton, Link, useStyles2 } from '@grafana/ui';
+import { IconButton, Link, Spinner, useStyles2 } from '@grafana/ui';
 import { getSvgSize } from '@grafana/ui/src/components/Icon/utils';
-import { Span, TextModifier } from '@grafana/ui/src/unstable';
+import { Span } from '@grafana/ui/src/unstable';
 
+import { useChildrenByParentUIDState } from '../state';
 import { DashboardsTreeItem } from '../types';
 
 import { Indent } from './Indent';
 
+const CHEVRON_SIZE = 'md';
+
 type NameCellProps = CellProps<DashboardsTreeItem, unknown> & {
   onFolderClick: (uid: string, newOpenState: boolean) => void;
 };
@@ -18,36 +22,64 @@ type NameCellProps = CellProps<DashboardsTreeItem, unknown> & {
 export function NameCell({ row: { original: data }, onFolderClick }: NameCellProps) {
   const styles = useStyles2(getStyles);
   const { item, level, isOpen } = data;
+  const childrenByParentUID = useChildrenByParentUIDState();
+  const chevronRef = useRef<HTMLButtonElement>(null);
+  const isLoading = isOpen && !childrenByParentUID[item.uid];
+  const [shouldRestoreFocus, setShouldRestoreFocus] = useState(false);
+
+  // restore focus back to the original button when loading is complete
+  useEffect(() => {
+    if (!isLoading && chevronRef.current && shouldRestoreFocus) {
+      chevronRef.current.focus();
+      setShouldRestoreFocus(false);
+    }
+  }, [isLoading, shouldRestoreFocus]);
 
   if (item.kind === 'ui') {
     return (
       <>
         <Indent level={level} />
         <span className={styles.folderButtonSpacer} />
-        <em>
-          <TextModifier color="secondary">{item.uiKind === 'empty-folder' ? 'No items' : 'Loading...'}</TextModifier>
-        </em>
+        {item.uiKind === 'empty-folder' ? (
+          <em className={styles.emptyText}>
+            <Span variant="body" color="secondary" truncate>
+              No items
+            </Span>
+          </em>
+        ) : (
+          <Skeleton width={200} />
+        )}
       </>
     );
   }
 
-  const chevronIcon = isOpen ? 'angle-down' : 'angle-right';
-
   return (
     <>
       <Indent level={level} />
 
       {item.kind === 'folder' ? (
-        <IconButton
-          size="md"
-          onClick={() => onFolderClick(item.uid, !isOpen)}
-          name={chevronIcon}
-          ariaLabel={isOpen ? 'Collapse folder' : 'Expand folder'}
-        />
+        <>
+          {isLoading ? (
+            <Spinner className={styles.chevron} />
+          ) : (
+            <IconButton
+              size={CHEVRON_SIZE}
+              className={styles.chevron}
+              ref={chevronRef}
+              onClick={() => {
+                if (!isOpen && !childrenByParentUID[item.uid]) {
+                  setShouldRestoreFocus(true);
+                }
+                onFolderClick(item.uid, !isOpen);
+              }}
+              name={isOpen ? 'angle-down' : 'angle-right'}
+              ariaLabel={isOpen ? 'Collapse folder' : 'Expand folder'}
+            />
+          )}
+        </>
       ) : (
         <span className={styles.folderButtonSpacer} />
       )}
-
       <Span variant="body" truncate>
         {item.url ? (
           <Link href={item.url} className={styles.link}>
@@ -63,9 +95,17 @@ export function NameCell({ row: { original: data }, onFolderClick }: NameCellPro
 
 const getStyles = (theme: GrafanaTheme2) => {
   return {
+    chevron: css({
+      marginRight: theme.spacing(1),
+      width: getSvgSize(CHEVRON_SIZE),
+    }),
+    emptyText: css({
+      // needed for text to truncate correctly
+      overflow: 'hidden',
+    }),
     // Should be the same size as the <IconButton /> so Dashboard name is aligned to Folder name siblings
     folderButtonSpacer: css({
-      paddingLeft: `calc(${getSvgSize('md')}px + ${theme.spacing(0.5)})`,
+      paddingLeft: `calc(${getSvgSize(CHEVRON_SIZE)}px + ${theme.spacing(1)})`,
     }),
     link: css({
       '&:hover': {
diff --git a/public/app/features/browse-dashboards/components/TagsCell.tsx b/public/app/features/browse-dashboards/components/TagsCell.tsx
index 82cfc659f53..a97d03f4dbd 100644
--- a/public/app/features/browse-dashboards/components/TagsCell.tsx
+++ b/public/app/features/browse-dashboards/components/TagsCell.tsx
@@ -10,7 +10,16 @@ import { DashboardsTreeItem } from '../types';
 export function TagsCell({ row: { original: data } }: CellProps<DashboardsTreeItem, unknown>) {
   const styles = useStyles2(getStyles);
   const item = data.item;
-  if (item.kind === 'ui' || !item.tags) {
+
+  if (item.kind === 'ui') {
+    if (item.uiKind === 'pagination-placeholder') {
+      return <TagList.Skeleton />;
+    } else {
+      return null;
+    }
+  }
+
+  if (!item.tags) {
     return null;
   }
 
@@ -22,6 +31,7 @@ function getStyles(theme: GrafanaTheme2) {
     // TagList is annoying and has weird default alignment
     tagList: css({
       justifyContent: 'flex-start',
+      flexWrap: 'nowrap',
     }),
   };
 }
diff --git a/public/app/features/browse-dashboards/components/TypeCell.tsx b/public/app/features/browse-dashboards/components/TypeCell.tsx
index d94273c0103..05bb806fda6 100644
--- a/public/app/features/browse-dashboards/components/TypeCell.tsx
+++ b/public/app/features/browse-dashboards/components/TypeCell.tsx
@@ -1,35 +1,61 @@
+import { css } from '@emotion/css';
 import React from 'react';
+import Skeleton from 'react-loading-skeleton';
 import { CellProps } from 'react-table';
 
-import { Icon } from '@grafana/ui';
-import { TextModifier } from '@grafana/ui/src/unstable';
+import { GrafanaTheme2 } from '@grafana/data';
+import { Icon, useStyles2 } from '@grafana/ui';
+import { Span } from '@grafana/ui/src/unstable';
 import { getIconForKind } from 'app/features/search/service/utils';
 
 import { DashboardsTreeItem } from '../types';
 
 export function TypeCell({ row: { original: data } }: CellProps<DashboardsTreeItem, unknown>) {
   const iconName = getIconForKind(data.item.kind);
+  const styles = useStyles2(getStyles);
 
   switch (data.item.kind) {
     case 'dashboard':
       return (
-        <TextModifier color="secondary">
-          <Icon name={iconName} /> Dashboard
-        </TextModifier>
+        <div className={styles.container}>
+          <Icon name={iconName} />
+          <Span variant="body" color="secondary" truncate>
+            Dashboard
+          </Span>
+        </div>
       );
     case 'folder':
       return (
-        <TextModifier color="secondary">
-          <Icon name={iconName} /> Folder
-        </TextModifier>
+        <div className={styles.container}>
+          <Icon name={iconName} />
+          <Span variant="body" color="secondary" truncate>
+            Folder
+          </Span>
+        </div>
       );
     case 'panel':
       return (
-        <TextModifier color="secondary">
-          <Icon name={iconName} /> Panel
-        </TextModifier>
+        <div className={styles.container}>
+          <Icon name={iconName} />
+          <Span variant="body" color="secondary" truncate>
+            Panel
+          </Span>
+        </div>
       );
+    case 'ui':
+      return data.item.uiKind === 'empty-folder' ? null : <Skeleton width={100} />;
     default:
       return null;
   }
 }
+
+const getStyles = (theme: GrafanaTheme2) => ({
+  container: css({
+    alignItems: 'center',
+    display: 'flex',
+    flexWrap: 'nowrap',
+    gap: theme.spacing(0.5),
+    // needed for text to truncate correctly
+    overflow: 'hidden',
+  }),
+});
diff --git a/public/app/features/browse-dashboards/fixtures/dashboardsTreeItem.fixture.ts b/public/app/features/browse-dashboards/fixtures/dashboardsTreeItem.fixture.ts
index 8f04fc73c3c..4cb15098aa3 100644
--- a/public/app/features/browse-dashboards/fixtures/dashboardsTreeItem.fixture.ts
+++ b/public/app/features/browse-dashboards/fixtures/dashboardsTreeItem.fixture.ts
@@ -60,7 +60,7 @@ export function wellFormedFolder(
       ...itemPartial,
     },
     level: 0,
-    isOpen: true,
+    isOpen: false,
     ...partial,
   };
 }
diff --git a/public/app/features/browse-dashboards/state/hooks.ts b/public/app/features/browse-dashboards/state/hooks.ts
index 36011a80689..64436ba8c75 100644
--- a/public/app/features/browse-dashboards/state/hooks.ts
+++ b/public/app/features/browse-dashboards/state/hooks.ts
@@ -143,7 +143,7 @@ function createFlatTree(
 
   let children = (items || []).flatMap((item) => mapItem(item, folderUID, level));
 
-  if (level === 0 && collection && !collection.isFullyLoaded) {
+  if (level === 0 && (!collection || !collection.isFullyLoaded)) {
     children = children.concat(getPaginationPlaceholders(ROOT_PAGE_SIZE, folderUID, level));
   }
 
diff --git a/yarn.lock b/yarn.lock
index c23405bc377..abd58d5b0e0 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -3714,6 +3714,7 @@ __metadata:
     react-hook-form: 7.5.3
     react-i18next: ^12.0.0
     react-inlinesvg: 3.0.2
+    react-loading-skeleton: 3.3.1
     react-popper: 2.3.0
     react-popper-tooltip: 4.4.2
     react-router-dom: 5.3.3