Nested folders: Unify visual styles between tree + search view (#70814)

* unify search table styles with browse * add a skeleton state when switching to search view * show all column headers * use isItemLoaded * extract number of placeholder rows into variable + add comment * fix all selection toggle behaviour * tidy up * fix unit test
2025-02-25 18:55:37 -06:00 · 2023-06-29 10:31:22 +01:00 · 2023-06-29 10:31:22 +01:00 · 703bf4afcc
commit 703bf4afcc
parent 0bbf011ca8
5 changed files with 172 additions and 147 deletions
--- a/public/app/features/browse-dashboards/components/DashboardsTree.tsx
+++ b/public/app/features/browse-dashboards/components/DashboardsTree.tsx
@ -38,8 +38,8 @@ interface DashboardsTreeProps {
  requestLoadMore: (folderUid: string | undefined) => void;
 }

-const HEADER_HEIGHT = 35;
-const ROW_HEIGHT = 35;
+const HEADER_HEIGHT = 36;
+const ROW_HEIGHT = 36;

 export function DashboardsTree({
  items,
--- a/public/app/features/browse-dashboards/components/SearchView.tsx
+++ b/public/app/features/browse-dashboards/components/SearchView.tsx
@ -1,6 +1,7 @@
 import React, { useCallback } from 'react';

-import { Button, Card, Spinner } from '@grafana/ui';
+import { DataFrameView, toDataFrame } from '@grafana/data';
+import { Button, Card } from '@grafana/ui';
 import { Trans } from 'app/core/internationalization';
 import { useKeyNavigationListener } from 'app/features/search/hooks/useSearchKeyboardSelection';
 import { SearchResultsProps, SearchResultsTable } from 'app/features/search/page/components/SearchResultsTable';
@ -16,6 +17,30 @@ interface SearchViewProps {
  canSelect: boolean;
 }

+const NUM_PLACEHOLDER_ROWS = 50;
+const initialLoadingView = {
+  view: new DataFrameView(
+    toDataFrame({
+      fields: [
+        { name: 'uid', display: true, values: Array(NUM_PLACEHOLDER_ROWS).fill(null) },
+        { name: 'kind', display: true, values: Array(NUM_PLACEHOLDER_ROWS).fill('dashboard') },
+        { name: 'name', display: true, values: Array(NUM_PLACEHOLDER_ROWS).fill('') },
+        { name: 'location', display: true, values: Array(NUM_PLACEHOLDER_ROWS).fill('') },
+        { name: 'tags', display: true, values: Array(NUM_PLACEHOLDER_ROWS).fill([]) },
+      ],
+      meta: {
+        custom: {
+          locationInfo: [],
+        },
+      },
+    })
+  ),
+  loadMoreItems: () => Promise.resolve(),
+  // this is key and controls whether to show the skeleton in generateColumns
+  isItemLoaded: () => false,
+  totalRows: NUM_PLACEHOLDER_ROWS,
+};
+
 export function SearchView({ width, height, canSelect }: SearchViewProps) {
  const dispatch = useDispatch();
  const selectedItems = useSelector((wholeState) => wholeState.browseDashboards.selectedItems);
@ -24,7 +49,7 @@ export function SearchView({ width, height, canSelect }: SearchViewProps) {
  const { keyboardEvents } = useKeyNavigationListener();
  const [searchState, stateManager] = useSearchStateManager();

-  const value = searchState.result;
+  const value = searchState.result ?? initialLoadingView;

  const selectionChecker = useCallback(
    (kind: string | undefined, uid: string): boolean => {
@ -61,14 +86,6 @@ export function SearchView({ width, height, canSelect }: SearchViewProps) {
    [selectionChecker, dispatch]
  );

-  if (!value) {
-    return (
-      <div style={{ width }}>
-        <Spinner />
-      </div>
-    );
-  }
-
  if (value.totalRows === 0) {
    return (
      <div style={{ width }}>
--- a/public/app/features/search/page/components/SearchResultsTable.test.tsx
+++ b/public/app/features/search/page/components/SearchResultsTable.test.tsx
@ -43,7 +43,7 @@ describe('SearchResultsTable', () => {
    });

    const mockSearchResult: QueryResponse = {
-      isItemLoaded: jest.fn(),
+      isItemLoaded: jest.fn().mockReturnValue(true),
      loadMoreItems: jest.fn(),
      totalRows: searchData.length,
      view: new DataFrameView<DashboardQueryResult>(dataFrames[0]),
--- a/public/app/features/search/page/components/SearchResultsTable.tsx
+++ b/public/app/features/search/page/components/SearchResultsTable.tsx
@ -1,7 +1,6 @@
-/* eslint-disable react/jsx-no-undef */
 import { css } from '@emotion/css';
 import React, { useEffect, useMemo, useRef, useCallback, useState, CSSProperties } from 'react';
-import { useTable, Column, TableOptions, Cell, useAbsoluteLayout } from 'react-table';
+import { useTable, Column, TableOptions, Cell } from 'react-table';
 import { FixedSizeList } from 'react-window';
 import InfiniteLoader from 'react-window-infinite-loader';
 import { Observable } from 'rxjs';
@ -11,6 +10,7 @@ import { TableCellHeight } from '@grafana/schema';
 import { useStyles2, useTheme2 } from '@grafana/ui';
 import { TableCell } from '@grafana/ui/src/components/Table/TableCell';
 import { useTableStyles } from '@grafana/ui/src/components/Table/styles';
+import { useCustomFlexLayout } from 'app/features/browse-dashboards/components/customFlexTableLayout';

 import { useSearchKeyboardNavigation } from '../../hooks/useSearchKeyboardSelection';
 import { QueryResponse } from '../../service';
@ -35,7 +35,7 @@ export type TableColumn = Column & {
  field?: Field;
 };

-const HEADER_HEIGHT = 36; // pixels
+const ROW_HEIGHT = 36; // pixels

 export const SearchResultsTable = React.memo(
  ({
@ -101,7 +101,7 @@ export const SearchResultsTable = React.memo(
      [memoizedColumns, memoizedData]
    );

-    const { getTableProps, getTableBodyProps, headerGroups, rows, prepareRow } = useTable(options, useAbsoluteLayout);
+    const { getTableProps, getTableBodyProps, headerGroups, rows, prepareRow } = useTable(options, useCustomFlexLayout);

    const handleLoadMore = useCallback(
      async (startIndex: number, endIndex: number) => {
@ -162,24 +162,24 @@ export const SearchResultsTable = React.memo(

    return (
      <div {...getTableProps()} aria-label="Search results table" role="table">
-        <div>
-          {headerGroups.map((headerGroup) => {
-            const { key, ...headerGroupProps } = headerGroup.getHeaderGroupProps();
+        {headerGroups.map((headerGroup) => {
+          const { key, ...headerGroupProps } = headerGroup.getHeaderGroupProps({
+            style: { width },
+          });

-            return (
-              <div key={key} {...headerGroupProps} className={styles.headerRow}>
-                {headerGroup.headers.map((column) => {
-                  const { key, ...headerProps } = column.getHeaderProps();
-                  return (
-                    <div key={key} {...headerProps} role="columnheader" className={styles.headerCell}>
-                      {column.render('Header')}
-                    </div>
-                  );
-                })}
-              </div>
-            );
-          })}
-        </div>
+          return (
+            <div key={key} {...headerGroupProps} className={styles.headerRow}>
+              {headerGroup.headers.map((column) => {
+                const { key, ...headerProps } = column.getHeaderProps();
+                return (
+                  <div key={key} {...headerProps} role="columnheader" className={styles.headerCell}>
+                    {column.render('Header')}
+                  </div>
+                );
+              })}
+            </div>
+          );
+        })}

        <div {...getTableBodyProps()}>
          <InfiniteLoader
@ -195,10 +195,10 @@ export const SearchResultsTable = React.memo(
                  setListEl(innerRef);
                }}
                onItemsRendered={onItemsRendered}
-                height={height - HEADER_HEIGHT}
+                height={height - ROW_HEIGHT}
                itemCount={rows.length}
                itemSize={tableStyles.rowHeight}
-                width="100%"
+                width={width}
                style={{ overflow: 'hidden auto' }}
              >
                {RenderRow}
@ -224,18 +224,25 @@ const getStyles = (theme: GrafanaTheme2) => {
      height: 100%;
    `,
    headerCell: css`
+      align-items: center;
+      display: flex;
+      overflo: hidden;
      padding: ${theme.spacing(1)};
    `,
    headerRow: css`
      background-color: ${theme.colors.background.secondary};
-      height: ${HEADER_HEIGHT}px;
-      align-items: center;
+      display: flex;
+      gap: ${theme.spacing(1)};
+      height: ${ROW_HEIGHT}px;
    `,
    selectedRow: css`
      background-color: ${rowHoverBg};
      box-shadow: inset 3px 0px ${theme.colors.primary.border};
    `,
    rowContainer: css`
+      display: flex;
+      gap: ${theme.spacing(1)};
+      height: ${ROW_HEIGHT}px;
      label: row;
      &:hover {
        background-color: ${rowHoverBg};
@ -253,27 +260,22 @@ const getStyles = (theme: GrafanaTheme2) => {
 // CSS for columns from react table
 const getColumnStyles = (theme: GrafanaTheme2) => {
  return {
+    cell: css({
+      padding: theme.spacing(1),
+      overflow: 'hidden', // Required so flex children can do text-overflow: ellipsis
+      display: 'flex',
+      alignItems: 'center',
+    }),
    nameCellStyle: css`
-      border-right: none;
-      padding: ${theme.spacing(1)} ${theme.spacing(1)} ${theme.spacing(1)} ${theme.spacing(2)};
      overflow: hidden;
      text-overflow: ellipsis;
      user-select: text;
      white-space: nowrap;
-      &:hover {
-        box-shadow: none;
-      }
    `,
-    headerNameStyle: css`
-      padding-left: ${theme.spacing(1)};
-    `,
-
+    typeCell: css({
+      gap: theme.spacing(0.5),
+    }),
    typeIcon: css`
-      margin-left: 5px;
-      margin-right: 9.5px;
-      vertical-align: middle;
-      display: inline-block;
-      margin-bottom: ${theme.v1.spacing.xxs};
      fill: ${theme.colors.text.secondary};
    `,
    datasourceItem: css`
@ -291,41 +293,24 @@ const getColumnStyles = (theme: GrafanaTheme2) => {
      color: ${theme.colors.error.main};
      text-decoration: line-through;
    `,
-    typeText: css`
-      color: ${theme.colors.text.secondary};
-      padding-top: ${theme.spacing(1)};
-    `,
+    locationContainer: css({
+      display: 'flex',
+      flexWrap: 'nowrap',
+      gap: theme.spacing(1),
+      overflow: 'hidden',
+    }),
    locationItem: css`
+      align-items: center;
      color: ${theme.colors.text.secondary};
-      margin-right: 12px;
-    `,
-    sortedHeader: css`
-      text-align: right;
-      padding-right: ${theme.spacing(2)};
-    `,
-    sortedItems: css`
-      text-align: right;
-      padding: ${theme.spacing(1)} ${theme.spacing(3)} ${theme.spacing(1)} ${theme.spacing(1)};
+      display: flex;
+      flex-wrap: nowrap;
+      gap: 4px;
+      overflow: hidden;
    `,
    explainItem: css`
-      text-align: right;
-      padding: ${theme.spacing(1)} ${theme.spacing(3)} ${theme.spacing(1)} ${theme.spacing(1)};
      cursor: pointer;
    `,
-    locationCellStyle: css`
-      padding-top: ${theme.spacing(1)};
-      padding-right: ${theme.spacing(1)};
-    `,
-    checkboxHeader: css`
-      margin-left: 2px;
-    `,
-    checkbox: css`
-      margin-left: 10px;
-      margin-right: 10px;
-      margin-top: 5px;
-    `,
    tagList: css`
-      padding-top: ${theme.spacing(0.5)};
      justify-content: flex-start;
      flex-wrap: nowrap;
    `,
--- a/public/app/features/search/page/components/columns.tsx
+++ b/public/app/features/search/page/components/columns.tsx
@ -1,5 +1,6 @@
 import { cx } from '@emotion/css';
 import React from 'react';
+import Skeleton from 'react-loading-skeleton';

 import {
  DisplayProcessor,
@ -10,7 +11,8 @@ import {
  getFieldDisplayName,
 } from '@grafana/data';
 import { config, getDataSourceSrv } from '@grafana/runtime';
-import { Checkbox, Icon, IconButton, IconName, TagList } from '@grafana/ui';
+import { Checkbox, Icon, IconName, TagList } from '@grafana/ui';
+import { Span } from '@grafana/ui/src/unstable';
 import appEvents from 'app/core/app_events';
 import { t } from 'app/core/internationalization';
 import { PluginIconName } from 'app/features/plugins/admin/types';
@ -57,38 +59,30 @@ export const generateColumns = (

  let width = 50;
  if (selection && selectionToggle) {
-    width = 30;
+    width = 0;
    columns.push({
      id: `column-checkbox`,
      width,
      Header: () => {
-        if (selection('*', '*')) {
-          return (
-            <div className={styles.checkboxHeader}>
-              <IconButton name="check-square" onClick={clearSelection} tooltip="Clear selection" />
-            </div>
-          );
-        }
        return (
-          <div className={styles.checkboxHeader}>
-            <Checkbox
-              checked={false}
-              onChange={(e) => {
-                e.stopPropagation();
-                e.preventDefault();
-                const { view } = response;
-                const count = Math.min(view.length, 50);
-                for (let i = 0; i < count; i++) {
-                  const item = view.get(i);
-                  if (item.uid && item.kind) {
-                    if (!selection(item.kind, item.uid)) {
-                      selectionToggle(item.kind, item.uid);
-                    }
+          <Checkbox
+            indeterminate={selection('*', '*')}
+            checked={false}
+            disabled={!response}
+            onChange={(e) => {
+              const { view } = response;
+              const count = Math.min(view.length, 50);
+              const hasSelection = selection('*', '*');
+              for (let i = 0; i < count; i++) {
+                const item = view.get(i);
+                if (item.uid && item.kind) {
+                  if (hasSelection === selection(item.kind, item.uid)) {
+                    selectionToggle(item.kind, item.uid);
                  }
                }
-              }}
-            />
-          </div>
+              }
+            }}
+          />
        );
      },
      Cell: (p) => {
@ -97,16 +91,14 @@ export const generateColumns = (
        const selected = selection(kind, uid);
        const hasUID = uid != null; // Panels don't have UID! Likely should not be shown on pages with manage options
        return (
-          <div {...p.cellProps}>
-            <div className={styles.checkbox}>
-              <Checkbox
-                disabled={!hasUID}
-                value={selected && hasUID}
-                onChange={(e) => {
-                  selectionToggle(kind, uid);
-                }}
-              />
-            </div>
+          <div {...p.cellProps} className={styles.cell}>
+            <Checkbox
+              disabled={!hasUID}
+              value={selected && hasUID}
+              onChange={(e) => {
+                selectionToggle(kind, uid);
+              }}
+            />
          </div>
        );
      },
@ -127,20 +119,26 @@ export const generateColumns = (
        classNames += ' ' + styles.missingTitleText;
      }
      return (
-        <a {...p.cellProps} href={p.userProps.href} onClick={p.userProps.onClick} className={classNames} title={name}>
-          {name}
-        </a>
+        <div className={styles.cell} {...p.cellProps}>
+          {!response.isItemLoaded(p.row.index) ? (
+            <Skeleton width={200} />
+          ) : (
+            <a href={p.userProps.href} onClick={p.userProps.onClick} className={classNames} title={name}>
+              {name}
+            </a>
+          )}
+        </div>
      );
    },
    id: `column-name`,
    field: access.name!,
-    Header: () => <div className={styles.headerNameStyle}>{t('search.results-table.name-header', 'Name')}</div>,
+    Header: () => <div>{t('search.results-table.name-header', 'Name')}</div>,
    width,
  });
  availableWidth -= width;

  width = TYPE_COLUMN_WIDTH;
-  columns.push(makeTypeColumn(access.kind, access.panel_type, width, styles));
+  columns.push(makeTypeColumn(response, access.kind, access.panel_type, width, styles));
  availableWidth -= width;

  // Show datasources if we have any
@ -168,20 +166,29 @@ export const generateColumns = (
      Cell: (p) => {
        const parts = (access.location?.values[p.row.index] ?? '').split('/');
        return (
-          <div {...p.cellProps} className={cx(styles.locationCellStyle)}>
-            {parts.map((p) => {
-              let info = meta.locationInfo[p];
-              if (!info && p === 'general') {
-                info = { kind: 'folder', url: '/dashboards', name: 'General' };
-              }
-              return info ? (
-                <a key={p} href={info.url} className={styles.locationItem}>
-                  <Icon name={getIconForKind(info.kind)} /> {info.name}
-                </a>
-              ) : (
-                <span key={p}>{p}</span>
-              );
-            })}
+          <div {...p.cellProps} className={styles.cell}>
+            {!response.isItemLoaded(p.row.index) ? (
+              <Skeleton width={150} />
+            ) : (
+              <div className={styles.locationContainer}>
+                {parts.map((p) => {
+                  let info = meta.locationInfo[p];
+                  if (!info && p === 'general') {
+                    info = { kind: 'folder', url: '/dashboards', name: 'General' };
+                  }
+                  return info ? (
+                    <a key={p} href={info.url} className={styles.locationItem}>
+                      <Icon name={getIconForKind(info.kind)} />
+                      <Span variant="body" truncate>
+                        {info.name}
+                      </Span>
+                    </a>
+                  ) : (
+                    <span key={p}>{p}</span>
+                  );
+                })}
+              </div>
+            )}
          </div>
        );
      },
@ -193,17 +200,17 @@ export const generateColumns = (
  }

  if (availableWidth > 0 && showTags) {
-    columns.push(makeTagsColumn(access.tags, availableWidth, styles.tagList, onTagSelected));
+    columns.push(makeTagsColumn(response, access.tags, availableWidth, styles, onTagSelected));
  }

  if (sortField && sortFieldWith) {
    const disp = sortField.display ?? getDisplayProcessor({ field: sortField, theme: config.theme2 });

    columns.push({
-      Header: () => <div className={styles.sortedHeader}>{getFieldDisplayName(sortField)}</div>,
+      Header: getFieldDisplayName(sortField),
      Cell: (p) => {
        return (
-          <div {...p.cellProps} className={styles.sortedItems}>
+          <div {...p.cellProps} className={styles.cell}>
            {getDisplayValue({
              sortField,
              getDisplay: disp,
@ -239,7 +246,11 @@ export const generateColumns = (
      Header: () => <div className={styles.sortedHeader}>Score</div>,
      Cell: (p) => {
        return (
-          <div {...p.cellProps} className={styles.explainItem} onClick={() => showExplainPopup(p.row.index)}>
+          <div
+            {...p.cellProps}
+            className={cx(styles.cell, styles.explainItem)}
+            onClick={() => showExplainPopup(p.row.index)}
+          >
            {vals[p.row.index]}
          </div>
        );
@ -314,6 +325,7 @@ function makeDataSourceColumn(
 }

 function makeTypeColumn(
+  response: QueryResponse,
  kindField: Field<string>,
  typeField: Field<string>,
  width: number,
@ -366,9 +378,15 @@ function makeTypeColumn(
        }
      }
      return (
-        <div {...p.cellProps} className={styles.typeText}>
-          <Icon name={icon} size="sm" title={txt} className={styles.typeIcon} />
-          {txt}
+        <div {...p.cellProps} className={cx(styles.cell, styles.typeCell)}>
+          {!response.isItemLoaded(p.row.index) ? (
+            <Skeleton width={100} />
+          ) : (
+            <>
+              <Icon name={icon} size="sm" title={txt} className={styles.typeIcon} />
+              {txt}
+            </>
+          )}
        </div>
      );
    },
@ -377,19 +395,24 @@ function makeTypeColumn(
 }

 function makeTagsColumn(
+  response: QueryResponse,
  field: Field<string[]>,
  width: number,
-  tagListClass: string,
+  styles: Record<string, string>,
  onTagSelected: (tag: string) => void
 ): TableColumn {
  return {
    Cell: (p) => {
      const tags = field.values[p.row.index];
-      return tags ? (
-        <div {...p.cellProps}>
-          <TagList className={tagListClass} tags={tags} onClick={onTagSelected} />
+      return (
+        <div {...p.cellProps} className={styles.cell}>
+          {!response.isItemLoaded(p.row.index) ? (
+            <TagList.Skeleton />
+          ) : (
+            <>{tags ? <TagList className={styles.tagList} tags={tags} onClick={onTagSelected} /> : null}</>
+          )}
        </div>
-      ) : null;
+      );
    },
    id: `column-tags`,
    field: field,