From e505babbf4d7a9d20ad5a8597bf71a698388f222 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Torkel=20=C3=96degaard?= <torkel@grafana.com>
Date: Sat, 25 Apr 2020 13:08:23 +0200
Subject: [PATCH] ManageDashboards: Fixes and improvements (#23879)

* ManageDashboards: Fixes and improvements

* Fixed tests

* Fixed issue with item height and margin
---
 .../src/components/Forms/Checkbox.tsx         |  6 +-
 .../IconButton/IconButton.story.tsx           | 11 ++-
 .../src/components/IconButton/IconButton.tsx  |  4 +-
 .../core/components/BackButton/BackButton.tsx |  2 +-
 .../dashboard/components/DashNav/DashNav.tsx  |  2 +-
 .../DashboardSettings/DashboardSettings.tsx   |  2 +-
 .../features/search/components/ActionRow.tsx  | 24 +-----
 .../components/DashboardSearch.test.tsx       |  6 +-
 .../search/components/ManageDashboards.tsx    | 83 ++++---------------
 .../search/components/SearchResults.tsx       | 15 +++-
 .../components/SearchResultsFilter.test.tsx   | 16 ++--
 .../search/components/SearchResultsFilter.tsx |  1 -
 .../features/search/hooks/useSearchQuery.ts   | 21 +----
 13 files changed, 62 insertions(+), 131 deletions(-)

diff --git a/packages/grafana-ui/src/components/Forms/Checkbox.tsx b/packages/grafana-ui/src/components/Forms/Checkbox.tsx
index e648ae01c4f..67d5ebc9292 100644
--- a/packages/grafana-ui/src/components/Forms/Checkbox.tsx
+++ b/packages/grafana-ui/src/components/Forms/Checkbox.tsx
@@ -19,6 +19,7 @@ export const getCheckboxStyles = stylesFactory((theme: GrafanaTheme) => {
       labelStyles.label,
       css`
         padding-left: ${theme.spacing.formSpacingBase}px;
+        white-space: nowrap;
       `
     ),
     description: cx(
@@ -50,11 +51,13 @@ export const getCheckboxStyles = stylesFactory((theme: GrafanaTheme) => {
        * */
       &:checked + span {
         background: blue;
-        background: ${theme.colors.formInputBg};
+        background: ${theme.colors.formCheckboxBgChecked};
         border: none;
+
         &:hover {
           background: ${theme.colors.formCheckboxBgCheckedHover};
         }
+
         &:after {
           content: '';
           position: absolute;
@@ -79,6 +82,7 @@ export const getCheckboxStyles = stylesFactory((theme: GrafanaTheme) => {
       position: absolute;
       top: 1px;
       left: 0;
+
       &:hover {
         cursor: pointer;
         border-color: ${theme.colors.formInputBorderHover};
diff --git a/packages/grafana-ui/src/components/IconButton/IconButton.story.tsx b/packages/grafana-ui/src/components/IconButton/IconButton.story.tsx
index 0b5d28e6ce9..ab7ab8f1088 100644
--- a/packages/grafana-ui/src/components/IconButton/IconButton.story.tsx
+++ b/packages/grafana-ui/src/components/IconButton/IconButton.story.tsx
@@ -20,7 +20,12 @@ export const simple = () => {
 
   return (
     <div>
-      {renderScenario('body', theme, ['sm', 'md', 'lg', 'xl', 'xxl'], ['search', 'trash-alt', 'arrow-left', 'times'])}
+      {renderScenario(
+        'dashboard',
+        theme,
+        ['sm', 'md', 'lg', 'xl', 'xxl'],
+        ['search', 'trash-alt', 'arrow-left', 'times']
+      )}
       {renderScenario('panel', theme, ['sm', 'md', 'lg', 'xl', 'xxl'], ['search', 'trash-alt', 'arrow-left', 'times'])}
       {renderScenario('header', theme, ['sm', 'md', 'lg', 'xl', 'xxl'], ['search', 'trash-alt', 'arrow-left', 'times'])}
     </div>
@@ -31,8 +36,8 @@ function renderScenario(surface: string, theme: GrafanaTheme, sizes: IconSize[],
   let bg: string = 'red';
 
   switch (surface) {
-    case 'body':
-      bg = theme.colors.bodyBg;
+    case 'dashboard':
+      bg = theme.colors.dashboardBg;
       break;
     case 'panel':
       bg = theme.colors.bodyBg;
diff --git a/packages/grafana-ui/src/components/IconButton/IconButton.tsx b/packages/grafana-ui/src/components/IconButton/IconButton.tsx
index 1b9340a0f05..532957b01f9 100644
--- a/packages/grafana-ui/src/components/IconButton/IconButton.tsx
+++ b/packages/grafana-ui/src/components/IconButton/IconButton.tsx
@@ -18,7 +18,7 @@ export interface Props extends React.ButtonHTMLAttributes<HTMLButtonElement> {
   tooltipPlacement?: TooltipPlacement;
 }
 
-type SurfaceType = 'body' | 'panel' | 'header';
+type SurfaceType = 'dashboard' | 'panel' | 'header';
 
 export const IconButton = React.forwardRef<HTMLButtonElement, Props>(
   ({ name, size = 'md', surface = 'panel', iconType, tooltip, tooltipPlacement, className, ...restProps }, ref) => {
@@ -47,7 +47,7 @@ IconButton.displayName = 'IconButton';
 
 function getHoverColor(theme: GrafanaTheme, surface: SurfaceType): string {
   switch (surface) {
-    case 'body':
+    case 'dashboard':
       return theme.isLight ? theme.palette.gray95 : theme.palette.gray15;
     case 'panel':
       return theme.isLight ? theme.palette.gray6 : theme.palette.gray15;
diff --git a/public/app/core/components/BackButton/BackButton.tsx b/public/app/core/components/BackButton/BackButton.tsx
index c951da4fd75..5ad54ed2701 100644
--- a/public/app/core/components/BackButton/BackButton.tsx
+++ b/public/app/core/components/BackButton/BackButton.tsx
@@ -3,7 +3,7 @@ import { IconButton } from '@grafana/ui';
 import { e2e } from '@grafana/e2e';
 
 export interface Props extends ButtonHTMLAttributes<HTMLButtonElement> {
-  surface: 'body' | 'panel';
+  surface: 'dashboard' | 'panel' | 'header';
 }
 
 export const BackButton: React.FC<Props> = ({ surface, onClick }) => {
diff --git a/public/app/features/dashboard/components/DashNav/DashNav.tsx b/public/app/features/dashboard/components/DashNav/DashNav.tsx
index 0d7c686ba8b..d0a0c395132 100644
--- a/public/app/features/dashboard/components/DashNav/DashNav.tsx
+++ b/public/app/features/dashboard/components/DashNav/DashNav.tsx
@@ -143,7 +143,7 @@ class DashNav extends PureComponent<Props> {
   renderBackButton() {
     return (
       <div className="navbar-edit">
-        <BackButton surface="body" onClick={this.onClose} />
+        <BackButton surface="dashboard" onClick={this.onClose} />
       </div>
     );
   }
diff --git a/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx b/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx
index e3f09cab87d..0d663c39b6b 100644
--- a/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx
+++ b/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx
@@ -50,7 +50,7 @@ export class DashboardSettings extends PureComponent<Props> {
       <div className="dashboard-settings">
         <div className="navbar navbar--edit">
           <div className="navbar-edit">
-            <BackButton surface="body" onClick={this.onClose} />
+            <BackButton surface="panel" onClick={this.onClose} />
           </div>
           <div className="navbar-page-btn">
             {haveFolder && <div className="navbar-page-btn__folder">{folderTitle} / </div>}
diff --git a/public/app/features/search/components/ActionRow.tsx b/public/app/features/search/components/ActionRow.tsx
index 568cc237883..62f313dbcae 100644
--- a/public/app/features/search/components/ActionRow.tsx
+++ b/public/app/features/search/components/ActionRow.tsx
@@ -1,6 +1,6 @@
 import React, { Dispatch, FC, SetStateAction } from 'react';
 import { css } from 'emotion';
-import { HorizontalGroup, RadioButtonGroup, Select, stylesFactory, useTheme } from '@grafana/ui';
+import { HorizontalGroup, RadioButtonGroup, stylesFactory, useTheme, Checkbox } from '@grafana/ui';
 import { GrafanaTheme, SelectableValue } from '@grafana/data';
 import { SortPicker } from 'app/core/components/Select/SortPicker';
 import { TagFilter } from 'app/core/components/TagFilter/TagFilter';
@@ -8,11 +8,6 @@ import { SearchSrv } from 'app/core/services/search_srv';
 import { layoutOptions } from '../hooks/useSearchLayout';
 import { DashboardQuery } from '../types';
 
-const starredFilterOptions = [
-  { label: 'Yes', value: true },
-  { label: 'No', value: false },
-];
-
 const searchSrv = new SearchSrv();
 
 type onSelectChange = (value: SelectableValue) => void;
@@ -24,7 +19,6 @@ interface Props {
   onTagFilterChange: onSelectChange;
   query: DashboardQuery;
   showStarredFilter?: boolean;
-  hideSelectedTags?: boolean;
   hideLayout?: boolean;
 }
 
@@ -36,7 +30,6 @@ export const ActionRow: FC<Props> = ({
   onTagFilterChange,
   query,
   showStarredFilter,
-  hideSelectedTags,
   hideLayout,
 }) => {
   const theme = useTheme();
@@ -48,24 +41,13 @@ export const ActionRow: FC<Props> = ({
         {!hideLayout ? <RadioButtonGroup options={layoutOptions} onChange={onLayoutChange} value={layout} /> : null}
         <SortPicker onChange={onSortChange} value={query.sort} />
       </HorizontalGroup>
-      <HorizontalGroup spacing="md" justify="space-between">
-        {showStarredFilter && (
-          <Select
-            width={20}
-            placeholder="Filter by starred"
-            key={starredFilterOptions?.find(f => f.value === query.starred)?.label}
-            options={starredFilterOptions}
-            onChange={onStarredFilterChange}
-          />
-        )}
-
+      <HorizontalGroup spacing="md" width="auto">
+        {showStarredFilter && <Checkbox label="Filter by starred" onChange={onStarredFilterChange} />}
         <TagFilter
           placeholder="Filter by tag"
           tags={query.tag}
           tagOptions={searchSrv.getDashboardTags}
           onChange={onTagFilterChange}
-          hideValues={hideSelectedTags}
-          isClearable={!hideSelectedTags}
         />
       </HorizontalGroup>
     </div>
diff --git a/public/app/features/search/components/DashboardSearch.test.tsx b/public/app/features/search/components/DashboardSearch.test.tsx
index f3c2631bbfe..e24e148d5a3 100644
--- a/public/app/features/search/components/DashboardSearch.test.tsx
+++ b/public/app/features/search/components/DashboardSearch.test.tsx
@@ -76,8 +76,10 @@ describe('DashboardSearch', () => {
 
     wrapper.update();
     expect(
-      wrapper.findWhere((c: any) => c.type() === 'h6' && c.text() === 'No dashboards matching your query were found.')
-    ).toHaveLength(1);
+      wrapper
+        .findWhere((c: any) => c.type() === 'div' && c.text() === 'No dashboards matching your query were found.')
+        .exists()
+    ).toBe(true);
   });
 
   it('should render search results', async () => {
diff --git a/public/app/features/search/components/ManageDashboards.tsx b/public/app/features/search/components/ManageDashboards.tsx
index a0bb05099e8..5ebe8d508f1 100644
--- a/public/app/features/search/components/ManageDashboards.tsx
+++ b/public/app/features/search/components/ManageDashboards.tsx
@@ -1,6 +1,6 @@
 import React, { FC, memo, useState } from 'react';
 import { css } from 'emotion';
-import { HorizontalGroup, Icon, stylesFactory, TagList, useTheme } from '@grafana/ui';
+import { HorizontalGroup, stylesFactory, useTheme } from '@grafana/ui';
 import { GrafanaTheme } from '@grafana/data';
 import { contextSrv } from 'app/core/services/context_srv';
 import EmptyListCTA from 'app/core/components/EmptyListCTA/EmptyListCTA';
@@ -32,9 +32,6 @@ export const ManageDashboards: FC<Props> = memo(({ folderId, folderUid }) => {
     query,
     hasFilters,
     onQueryChange,
-    onRemoveStarred,
-    onTagRemove,
-    onClearFilters,
     onTagFilterChange,
     onStarredFilterChange,
     onTagAdd,
@@ -102,72 +99,24 @@ export const ManageDashboards: FC<Props> = memo(({ folderId, folderUid }) => {
           />
           <DashboardActions isEditor={isEditor} canEdit={hasEditPermissionInFolders || canSave} folderId={folderId} />
         </HorizontalGroup>
-
-        {hasFilters && (
-          <HorizontalGroup>
-            <div className="gf-form-inline">
-              {query.tag.length > 0 && (
-                <div className="gf-form">
-                  <label className="gf-form-label width-4">Tags</label>
-                  <TagList tags={query.tag} onClick={onTagRemove} />
-                </div>
-              )}
-              {query.starred && (
-                <div className="gf-form">
-                  <label className="gf-form-label">
-                    <a className="pointer" onClick={onRemoveStarred}>
-                      <Icon name="check" />
-                      Starred
-                    </a>
-                  </label>
-                </div>
-              )}
-              {query.sort && (
-                <div className="gf-form">
-                  <label className="gf-form-label">
-                    <a className="pointer" onClick={() => onSortChange(null)}>
-                      Sort: {query.sort.label}
-                    </a>
-                  </label>
-                </div>
-              )}
-              <div className="gf-form">
-                <label className="gf-form-label">
-                  <a
-                    className="pointer"
-                    onClick={() => {
-                      onClearFilters();
-                      setLayout(SearchLayout.Folders);
-                    }}
-                  >
-                    <Icon name="times" />
-                    &nbsp;Clear
-                  </a>
-                </label>
-              </div>
-            </div>
-          </HorizontalGroup>
-        )}
       </div>
 
       <div className={styles.results}>
-        {results?.length > 0 && (
-          <SearchResultsFilter
-            allChecked={allChecked}
-            canDelete={canDelete}
-            canMove={canMove}
-            deleteItem={onItemDelete}
-            moveTo={onMoveTo}
-            onToggleAllChecked={onToggleAllChecked}
-            onStarredFilterChange={onStarredFilterChange}
-            onSortChange={onSortChange}
-            onTagFilterChange={onTagFilterChange}
-            query={query}
-            layout={layout}
-            hideLayout={!!folderUid}
-            onLayoutChange={onLayoutChange}
-          />
-        )}
+        <SearchResultsFilter
+          allChecked={allChecked}
+          canDelete={canDelete}
+          canMove={canMove}
+          deleteItem={onItemDelete}
+          moveTo={onMoveTo}
+          onToggleAllChecked={onToggleAllChecked}
+          onStarredFilterChange={onStarredFilterChange}
+          onSortChange={onSortChange}
+          onTagFilterChange={onTagFilterChange}
+          query={query}
+          layout={layout}
+          hideLayout={!!folderUid}
+          onLayoutChange={onLayoutChange}
+        />
         <SearchResults
           loading={loading}
           results={results}
diff --git a/public/app/features/search/components/SearchResults.tsx b/public/app/features/search/components/SearchResults.tsx
index 4b2320ed363..b9441193ea7 100644
--- a/public/app/features/search/components/SearchResults.tsx
+++ b/public/app/features/search/components/SearchResults.tsx
@@ -68,7 +68,13 @@ export const SearchResults: FC<Props> = ({
             >
               {({ index, style }) => {
                 const item = items[index];
-                return <SearchItem key={item.id} {...itemProps} item={item} style={style} />;
+                // The wrapper div is needed as the inner SearchItem has margin-bottom spacing
+                // And without this wrapper there is no room for that margin
+                return (
+                  <div style={style}>
+                    <SearchItem key={item.id} {...itemProps} item={item} />
+                  </div>
+                );
               }}
             </FixedSizeList>
           )}
@@ -80,7 +86,7 @@ export const SearchResults: FC<Props> = ({
   if (loading) {
     return <Spinner className={styles.spinner} />;
   } else if (!results || !results.length) {
-    return <h6>No dashboards matching your query were found.</h6>;
+    return <div className={styles.noResults}>No dashboards matching your query were found.</div>;
   }
 
   return (
@@ -120,6 +126,11 @@ const getSectionStyles = stylesFactory((theme: GrafanaTheme) => {
       border-radius: 3px;
       height: 100%;
     `,
+    noResults: css`
+      padding: ${md};
+      background: ${theme.colors.bg2};
+      text-style: italic;
+    `,
     listModeWrapper: css`
       position: relative;
       height: 100%;
diff --git a/public/app/features/search/components/SearchResultsFilter.test.tsx b/public/app/features/search/components/SearchResultsFilter.test.tsx
index f131ca0edd2..a6db78e17e0 100644
--- a/public/app/features/search/components/SearchResultsFilter.test.tsx
+++ b/public/app/features/search/components/SearchResultsFilter.test.tsx
@@ -38,24 +38,21 @@ describe('SearchResultsFilter', () => {
   it('should render "filter by starred" and "filter by tag" filters by default', () => {
     const { wrapper } = setup();
     const ActionRow = wrapper.find('ActionRow').shallow();
-    expect(ActionRow.find({ placeholder: 'Filter by starred' })).toHaveLength(1);
-    expect(ActionRow.find({ placeholder: 'Filter by tag' })).toHaveLength(1);
+    expect(ActionRow.find('Checkbox')).toHaveLength(1);
     expect(findBtnByText(wrapper, 'Move')).toHaveLength(0);
     expect(findBtnByText(wrapper, 'Delete')).toHaveLength(0);
   });
 
   it('should render Move and Delete buttons when canDelete is true', () => {
     const { wrapper } = setup({ canDelete: true });
-    expect(wrapper.find({ placeholder: 'Filter by starred' })).toHaveLength(0);
-    expect(wrapper.find({ placeholder: 'Filter by tag' })).toHaveLength(0);
+    expect(wrapper.find('Checkbox')).toHaveLength(1);
     expect(findBtnByText(wrapper, 'Move')).toHaveLength(1);
     expect(findBtnByText(wrapper, 'Delete')).toHaveLength(1);
   });
 
   it('should render Move and Delete buttons when canMove is true', () => {
     const { wrapper } = setup({ canMove: true });
-    expect(wrapper.find({ placeholder: 'Filter by starred' })).toHaveLength(0);
-    expect(wrapper.find({ placeholder: 'Filter by tag' })).toHaveLength(0);
+    expect(wrapper.find('Checkbox')).toHaveLength(1);
     expect(findBtnByText(wrapper, 'Move')).toHaveLength(1);
     expect(findBtnByText(wrapper, 'Delete')).toHaveLength(1);
   });
@@ -66,9 +63,10 @@ describe('SearchResultsFilter', () => {
     //@ts-ignore
     const { wrapper } = setup({ onStarredFilterChange: mockFilterStarred }, mount);
     wrapper
-      .find({ placeholder: 'Filter by starred' })
-      .at(0)
-      .prop('onChange')(option);
+      .find('Checkbox')
+      .at(1)
+      .prop('onChange')(option as any);
+
     expect(mockFilterStarred).toHaveBeenCalledTimes(1);
     expect(mockFilterStarred).toHaveBeenCalledWith(option);
   });
diff --git a/public/app/features/search/components/SearchResultsFilter.tsx b/public/app/features/search/components/SearchResultsFilter.tsx
index 7e287172f02..596eb938327 100644
--- a/public/app/features/search/components/SearchResultsFilter.tsx
+++ b/public/app/features/search/components/SearchResultsFilter.tsx
@@ -66,7 +66,6 @@ export const SearchResultsFilter: FC<Props> = ({
             query,
           }}
           showStarredFilter
-          hideSelectedTags
         />
       )}
     </div>
diff --git a/public/app/features/search/hooks/useSearchQuery.ts b/public/app/features/search/hooks/useSearchQuery.ts
index aee9674a501..385e67e1b45 100644
--- a/public/app/features/search/hooks/useSearchQuery.ts
+++ b/public/app/features/search/hooks/useSearchQuery.ts
@@ -1,16 +1,7 @@
 import { useReducer } from 'react';
 import { SelectableValue } from '@grafana/data';
 import { defaultQuery, queryReducer } from '../reducers/searchQueryReducer';
-import {
-  ADD_TAG,
-  CLEAR_FILTERS,
-  QUERY_CHANGE,
-  REMOVE_STARRED,
-  REMOVE_TAG,
-  SET_TAGS,
-  TOGGLE_SORT,
-  TOGGLE_STARRED,
-} from '../reducers/actionTypes';
+import { ADD_TAG, CLEAR_FILTERS, QUERY_CHANGE, SET_TAGS, TOGGLE_SORT, TOGGLE_STARRED } from '../reducers/actionTypes';
 import { DashboardQuery } from '../types';
 import { hasFilters } from '../utils';
 
@@ -22,14 +13,6 @@ export const useSearchQuery = (queryParams: Partial<DashboardQuery>) => {
     dispatch({ type: QUERY_CHANGE, payload: query });
   };
 
-  const onRemoveStarred = () => {
-    dispatch({ type: REMOVE_STARRED });
-  };
-
-  const onTagRemove = (tag: string) => {
-    dispatch({ type: REMOVE_TAG, payload: tag });
-  };
-
   const onTagFilterChange = (tags: string[]) => {
     dispatch({ type: SET_TAGS, payload: tags });
   };
@@ -54,8 +37,6 @@ export const useSearchQuery = (queryParams: Partial<DashboardQuery>) => {
     query,
     hasFilters: hasFilters(query),
     onQueryChange,
-    onRemoveStarred,
-    onTagRemove,
     onClearFilters,
     onTagFilterChange,
     onStarredFilterChange,