PlaylistPage: Skeleton loading state 💀 (#77992)

* playlist skeleton poc * refactor into PlaylistCard * make card actions more responsive, update skeleton color to work on secondary background * don't loop over array * fix unit tests
2025-02-25 18:55:37 -06:00 · 2023-11-13 10:32:42 +00:00 · 2023-11-13 10:32:42 +00:00 · bd85d3e25e
commit bd85d3e25e
parent eca45f6492
6 changed files with 201 additions and 120 deletions
--- a/packages/grafana-ui/src/components/Card/Card.tsx
+++ b/packages/grafana-ui/src/components/Card/Card.tsx
@ -283,22 +283,22 @@ const BaseActions = ({ children, disabled, variant, className }: ActionsProps) =

 const getActionStyles = (theme: GrafanaTheme2) => ({
  actions: css({
+    display: 'flex',
+    flexDirection: 'row',
+    flexWrap: 'wrap',
+    gap: theme.spacing(1),
    gridArea: 'Actions',
    marginTop: theme.spacing(2),
-    '& > *': {
-      marginRight: theme.spacing(1),
-    },
  }),
  secondaryActions: css({
-    display: 'flex',
-    gridArea: 'Secondary',
    alignSelf: 'center',
    color: theme.colors.text.secondary,
-    marginTtop: theme.spacing(2),
-
-    '& > *': {
-      marginRight: `${theme.spacing(1)} !important`,
-    },
+    display: 'flex',
+    flexDirection: 'row',
+    flexWrap: 'wrap',
+    gap: theme.spacing(1),
+    gridArea: 'Secondary',
+    marginTop: theme.spacing(2),
  }),
 });

--- a/public/app/core/utils/ConfigProvider.tsx
+++ b/public/app/core/utils/ConfigProvider.tsx
@ -23,8 +23,8 @@ export const ThemeProvider = ({ children, value }: { children: React.ReactNode;
  return (
    <ThemeContext.Provider value={theme}>
      <SkeletonTheme
-        baseColor={theme.colors.background.secondary}
-        highlightColor={theme.colors.emphasize(theme.colors.background.secondary)}
+        baseColor={theme.colors.emphasize(theme.colors.background.secondary)}
+        highlightColor={theme.colors.emphasize(theme.colors.background.secondary, 0.1)}
        borderRadius={theme.shape.radius.default}
      >
        {children}
--- a/public/app/features/playlist/PlaylistCard.tsx
+++ b/public/app/features/playlist/PlaylistCard.tsx
@ -0,0 +1,95 @@
+import { css } from '@emotion/css';
+import React from 'react';
+import Skeleton from 'react-loading-skeleton';
+
+import { GrafanaTheme2 } from '@grafana/data';
+import { Button, Card, LinkButton, ModalsController, Stack, useStyles2 } from '@grafana/ui';
+import { t, Trans } from 'app/core/internationalization';
+import { contextSrv } from 'app/core/services/context_srv';
+import { DashNavButton } from 'app/features/dashboard/components/DashNav/DashNavButton';
+
+import { ShareModal } from './ShareModal';
+import { Playlist } from './types';
+
+interface Props {
+  setStartPlaylist: (playlistItem: Playlist) => void;
+  setPlaylistToDelete: (playlistItem: Playlist) => void;
+  playlist: Playlist;
+}
+
+export const PlaylistCard = ({ playlist, setStartPlaylist, setPlaylistToDelete }: Props) => {
+  return (
+    <Card>
+      <Card.Heading>
+        {playlist.name}
+        <ModalsController key="button-share">
+          {({ showModal, hideModal }) => (
+            <DashNavButton
+              tooltip={t('playlist-page.card.tooltip', 'Share playlist')}
+              icon="share-alt"
+              iconSize="lg"
+              onClick={() => {
+                showModal(ShareModal, {
+                  playlistUid: playlist.uid,
+                  onDismiss: hideModal,
+                });
+              }}
+            />
+          )}
+        </ModalsController>
+      </Card.Heading>
+      <Card.Actions>
+        <Button variant="secondary" icon="play" onClick={() => setStartPlaylist(playlist)}>
+          <Trans i18nKey="playlist-page.card.start">Start playlist</Trans>
+        </Button>
+        {contextSrv.isEditor && (
+          <>
+            <LinkButton key="edit" variant="secondary" href={`/playlists/edit/${playlist.uid}`} icon="cog">
+              <Trans i18nKey="playlist-page.card.edit">Edit playlist</Trans>
+            </LinkButton>
+            <Button
+              disabled={false}
+              onClick={() => setPlaylistToDelete(playlist)}
+              icon="trash-alt"
+              variant="destructive"
+            >
+              <Trans i18nKey="playlist-page.card.delete">Delete playlist</Trans>
+            </Button>
+          </>
+        )}
+      </Card.Actions>
+    </Card>
+  );
+};
+
+const PlaylistCardSkeleton = () => {
+  const skeletonStyles = useStyles2(getSkeletonStyles);
+  return (
+    <Card>
+      <Card.Heading>
+        <Skeleton width={140} />
+      </Card.Heading>
+      <Card.Actions>
+        <Stack direction="row">
+          <Skeleton containerClassName={skeletonStyles.button} width={142} height={32} />
+          {contextSrv.isEditor && (
+            <>
+              <Skeleton containerClassName={skeletonStyles.button} width={135} height={32} />
+              <Skeleton containerClassName={skeletonStyles.button} width={153} height={32} />
+            </>
+          )}
+        </Stack>
+      </Card.Actions>
+    </Card>
+  );
+};
+
+PlaylistCard.Skeleton = PlaylistCardSkeleton;
+
+function getSkeletonStyles(theme: GrafanaTheme2) {
+  return {
+    button: css({
+      lineHeight: 1,
+    }),
+  };
+}
--- a/public/app/features/playlist/PlaylistPage.test.tsx
+++ b/public/app/features/playlist/PlaylistPage.test.tsx
@ -1,4 +1,4 @@
-import { render, waitFor } from '@testing-library/react';
+import { render, screen } from '@testing-library/react';
 import React from 'react';
 import { TestProvider } from 'test/helpers/TestProvider';

@ -21,7 +21,7 @@ jest.mock('app/core/services/context_srv', () => ({
  },
 }));

-function getTestContext() {
+function setup() {
  return render(
    <TestProvider>
      <PlaylistPage />
@ -37,30 +37,34 @@ describe('PlaylistPage', () => {
  describe('when mounted without a playlist', () => {
    it('page should load', () => {
      fnMock.mockResolvedValue([]);
-      const { getByText } = getTestContext();
-      expect(getByText(/loading/i)).toBeInTheDocument();
+      setup();
+      expect(screen.getByTestId('playlist-page-list-skeleton')).toBeInTheDocument();
    });
+
    it('then show empty list', async () => {
-      const { getByText } = getTestContext();
-      await waitFor(() => getByText('There are no playlists created yet'));
+      setup();
+      expect(await screen.findByText('There are no playlists created yet')).toBeInTheDocument();
    });
+
    describe('and signed in user is not a viewer', () => {
      it('then create playlist button should not be disabled', async () => {
        contextSrv.isEditor = true;
-        const { getByRole } = getTestContext();
-        const createPlaylistButton = await waitFor(() => getByRole('link', { name: /create playlist/i }));
+        setup();
+        const createPlaylistButton = await screen.findByRole('link', { name: /create playlist/i });
        expect(createPlaylistButton).not.toHaveStyle('pointer-events: none');
      });
    });
+
    describe('and signed in user is a viewer', () => {
      it('then create playlist button should be disabled', async () => {
        contextSrv.isEditor = false;
-        const { getByRole } = getTestContext();
-        const createPlaylistButton = await waitFor(() => getByRole('link', { name: /create playlist/i }));
+        setup();
+        const createPlaylistButton = await screen.findByRole('link', { name: /create playlist/i });
        expect(createPlaylistButton).toHaveStyle('pointer-events: none');
      });
    });
  });
+
  describe('when mounted with a playlist', () => {
    it('page should load', () => {
      fnMock.mockResolvedValue([
@ -76,29 +80,31 @@ describe('PlaylistPage', () => {
          uid: 'playlist-0',
        },
      ]);
-      const { getByText } = getTestContext();
-      expect(getByText(/loading/i)).toBeInTheDocument();
+      setup();
+      expect(screen.getByTestId('playlist-page-list-skeleton')).toBeInTheDocument();
    });
+
    describe('and signed in user is not a viewer', () => {
      it('then playlist title and all playlist buttons should appear on the page', async () => {
        contextSrv.isEditor = true;
-        const { getByRole, getByText } = getTestContext();
-        await waitFor(() => getByText('A test playlist'));
-        expect(getByRole('link', { name: /New playlist/i })).toBeInTheDocument();
-        expect(getByRole('button', { name: /Start playlist/i })).toBeInTheDocument();
-        expect(getByRole('link', { name: /Edit playlist/i })).toBeInTheDocument();
-        expect(getByRole('button', { name: /Delete playlist/i })).toBeInTheDocument();
+        setup();
+        expect(await screen.findByText('A test playlist'));
+        expect(await screen.findByRole('link', { name: /New playlist/i })).toBeInTheDocument();
+        expect(await screen.findByRole('button', { name: /Start playlist/i })).toBeInTheDocument();
+        expect(await screen.findByRole('link', { name: /Edit playlist/i })).toBeInTheDocument();
+        expect(await screen.findByRole('button', { name: /Delete playlist/i })).toBeInTheDocument();
      });
    });
+
    describe('and signed in user is a viewer', () => {
      it('then playlist title and only start playlist button should appear on the page', async () => {
        contextSrv.isEditor = false;
-        const { getByRole, getByText, queryByRole } = getTestContext();
-        await waitFor(() => getByText('A test playlist'));
-        expect(queryByRole('link', { name: /New playlist/i })).not.toBeInTheDocument();
-        expect(getByRole('button', { name: /Start playlist/i })).toBeInTheDocument();
-        expect(queryByRole('link', { name: /Edit playlist/i })).not.toBeInTheDocument();
-        expect(queryByRole('button', { name: /Delete playlist/i })).not.toBeInTheDocument();
+        setup();
+        expect(await screen.findByText('A test playlist')).toBeInTheDocument();
+        expect(screen.queryByRole('link', { name: /New playlist/i })).not.toBeInTheDocument();
+        expect(await screen.findByRole('button', { name: /Start playlist/i })).toBeInTheDocument();
+        expect(screen.queryByRole('link', { name: /Edit playlist/i })).not.toBeInTheDocument();
+        expect(screen.queryByRole('button', { name: /Delete playlist/i })).not.toBeInTheDocument();
      });
    });
  });
--- a/public/app/features/playlist/PlaylistPage.tsx
+++ b/public/app/features/playlist/PlaylistPage.tsx
@ -1,11 +1,11 @@
 import React, { useMemo, useState } from 'react';
 import { useAsync } from 'react-use';

-import { ConfirmModal } from '@grafana/ui';
+import { ConfirmModal, LinkButton } from '@grafana/ui';
 import EmptyListCTA from 'app/core/components/EmptyListCTA/EmptyListCTA';
 import { Page } from 'app/core/components/Page/Page';
 import PageActionBar from 'app/core/components/PageActionBar/PageActionBar';
-import { t } from 'app/core/internationalization';
+import { Trans, t } from 'app/core/internationalization';
 import { contextSrv } from 'app/core/services/context_srv';

 import { EmptyQueryListBanner } from './EmptyQueryListBanner';
@ -50,23 +50,26 @@ export const PlaylistPage = () => {
    />
  );

-  const showSearch = playlists.length > 0 || searchQuery.length > 0;
+  const showSearch = allPlaylists.loading || playlists.length > 0 || searchQuery.length > 0;

  return (
-    <Page navId="dashboards/playlists">
-      <Page.Contents isLoading={allPlaylists.loading}>
-        {showSearch && (
-          <PageActionBar
-            searchQuery={searchQuery}
-            linkButton={
-              contextSrv.isEditor
-                ? { title: t('playlist-page.create-button.title', 'New playlist'), href: '/playlists/new' }
-                : undefined
+    <Page
+      actions={
+        contextSrv.isEditor ? (
+          <LinkButton href="/playlists/new">
+            <Trans i18nKey="playlist-page.create-button.title">New playlist</Trans>
+          </LinkButton>
+        ) : undefined
      }
-            setSearchQuery={setSearchQuery}
-          />
-        )}
+      navId="dashboards/playlists"
+    >
+      <Page.Contents>
+        {showSearch && <PageActionBar searchQuery={searchQuery} setSearchQuery={setSearchQuery} />}

+        {allPlaylists.loading ? (
+          <PlaylistPageList.Skeleton />
+        ) : (
+          <>
            {!hasPlaylists && searchQuery ? (
              <EmptyQueryListBanner />
            ) : (
@ -90,6 +93,8 @@ export const PlaylistPage = () => {
              />
            )}
            {startPlaylist && <StartModal playlist={startPlaylist} onDismiss={() => setStartPlaylist(undefined)} />}
+          </>
+        )}
      </Page.Contents>
    </Page>
  );
--- a/public/app/features/playlist/PlaylistPageList.tsx
+++ b/public/app/features/playlist/PlaylistPageList.tsx
@ -2,72 +2,47 @@ import { css } from '@emotion/css';
 import React from 'react';

 import { GrafanaTheme2 } from '@grafana/data';
-import { Button, Card, LinkButton, ModalsController, useStyles2 } from '@grafana/ui';
-import { t, Trans } from 'app/core/internationalization';
-import { contextSrv } from 'app/core/services/context_srv';
-import { DashNavButton } from 'app/features/dashboard/components/DashNav/DashNavButton';
+import { useStyles2 } from '@grafana/ui';

-import { ShareModal } from './ShareModal';
+import { PlaylistCard } from './PlaylistCard';
 import { Playlist } from './types';

 interface Props {
  setStartPlaylist: (playlistItem: Playlist) => void;
  setPlaylistToDelete: (playlistItem: Playlist) => void;
-  playlists: Playlist[] | undefined;
+  playlists: Playlist[];
 }

 export const PlaylistPageList = ({ playlists, setStartPlaylist, setPlaylistToDelete }: Props) => {
  const styles = useStyles2(getStyles);
  return (
    <ul className={styles.list}>
-      {playlists!.map((playlist: Playlist) => (
+      {playlists.map((playlist: Playlist) => (
        <li className={styles.listItem} key={playlist.uid}>
-          <Card>
-            <Card.Heading>
-              {playlist.name}
-              <ModalsController key="button-share">
-                {({ showModal, hideModal }) => (
-                  <DashNavButton
-                    tooltip={t('playlist-page.card.tooltip', 'Share playlist')}
-                    icon="share-alt"
-                    iconSize="lg"
-                    onClick={() => {
-                      showModal(ShareModal, {
-                        playlistUid: playlist.uid,
-                        onDismiss: hideModal,
-                      });
-                    }}
+          <PlaylistCard
+            playlist={playlist}
+            setStartPlaylist={setStartPlaylist}
+            setPlaylistToDelete={setPlaylistToDelete}
          />
-                )}
-              </ModalsController>
-            </Card.Heading>
-            <Card.Actions>
-              <Button variant="secondary" icon="play" onClick={() => setStartPlaylist(playlist)}>
-                <Trans i18nKey="playlist-page.card.start">Start playlist</Trans>
-              </Button>
-              {contextSrv.isEditor && (
-                <>
-                  <LinkButton key="edit" variant="secondary" href={`/playlists/edit/${playlist.uid}`} icon="cog">
-                    <Trans i18nKey="playlist-page.card.edit">Edit playlist</Trans>
-                  </LinkButton>
-                  <Button
-                    disabled={false}
-                    onClick={() => setPlaylistToDelete(playlist)}
-                    icon="trash-alt"
-                    variant="destructive"
-                  >
-                    <Trans i18nKey="playlist-page.card.delete">Delete playlist</Trans>
-                  </Button>
-                </>
-              )}
-            </Card.Actions>
-          </Card>
        </li>
      ))}
    </ul>
  );
 };

+const PlaylistPageListSkeleton = () => {
+  const styles = useStyles2(getStyles);
+  return (
+    <div data-testid="playlist-page-list-skeleton" className={styles.list}>
+      <PlaylistCard.Skeleton />
+      <PlaylistCard.Skeleton />
+      <PlaylistCard.Skeleton />
+    </div>
+  );
+};
+
+PlaylistPageList.Skeleton = PlaylistPageListSkeleton;
+
 function getStyles(theme: GrafanaTheme2) {
  return {
    list: css({