From 95048fc681e4df9ce7812a7dc71340bc9f3a6447 Mon Sep 17 00:00:00 2001
From: Gareth Dawson <gareth.dawson@grafana.com>
Date: Wed, 15 Mar 2023 18:28:26 +0000
Subject: [PATCH] Prometheus: Metric encyclopedia modal redesign (#64816)

* feat: metric encyclopedia modal redesign

* test: update failing tests

* refactor: add suggestions from pr review

* test: fix failing test
---
 .../MetricEncyclopediaModal.test.tsx          |   6 +-
 .../components/MetricEncyclopediaModal.tsx    | 546 ++++++++++--------
 2 files changed, 314 insertions(+), 238 deletions(-)

diff --git a/public/app/plugins/datasource/prometheus/querybuilder/components/MetricEncyclopediaModal.test.tsx b/public/app/plugins/datasource/prometheus/querybuilder/components/MetricEncyclopediaModal.test.tsx
index fc16cf74394..9a3b74b0f52 100644
--- a/public/app/plugins/datasource/prometheus/querybuilder/components/MetricEncyclopediaModal.test.tsx
+++ b/public/app/plugins/datasource/prometheus/querybuilder/components/MetricEncyclopediaModal.test.tsx
@@ -10,7 +10,7 @@ import { EmptyLanguageProviderMock } from '../../language_provider.mock';
 import { PromOptions } from '../../types';
 import { PromVisualQuery } from '../types';
 
-import { MetricEncyclopediaModal, testIds, placeholders } from './MetricEncyclopediaModal';
+import { MetricEncyclopediaModal, testIds } from './MetricEncyclopediaModal';
 
 // don't care about interaction tracking in our unit tests
 jest.mock('@grafana/runtime', () => ({
@@ -96,7 +96,7 @@ describe('MetricEncyclopediaModal', () => {
     setup(defaultQuery, listOfMetrics);
 
     await waitFor(() => {
-      const selectType = screen.getByText(placeholders.type);
+      const selectType = screen.getByText('Filter by type');
       expect(selectType).toBeInTheDocument();
     });
   });
@@ -119,7 +119,7 @@ describe('MetricEncyclopediaModal', () => {
     setup(defaultQuery, listOfMetrics);
 
     await waitFor(() => {
-      const selectType = screen.getByText(placeholders.variables);
+      const selectType = screen.getByText('Select template variables');
       expect(selectType).toBeInTheDocument();
     });
   });
diff --git a/public/app/plugins/datasource/prometheus/querybuilder/components/MetricEncyclopediaModal.tsx b/public/app/plugins/datasource/prometheus/querybuilder/components/MetricEncyclopediaModal.tsx
index 72601ae5679..90ad6fa1464 100644
--- a/public/app/plugins/datasource/prometheus/querybuilder/components/MetricEncyclopediaModal.tsx
+++ b/public/app/plugins/datasource/prometheus/querybuilder/components/MetricEncyclopediaModal.tsx
@@ -1,17 +1,17 @@
-import { css } from '@emotion/css';
+import { css, cx } from '@emotion/css';
 import uFuzzy from '@leeoniya/ufuzzy';
 import debounce from 'debounce-promise';
 import { debounce as debounceLodash } from 'lodash';
 import React, { useCallback, useEffect, useMemo, useState } from 'react';
 
 import { GrafanaTheme2, SelectableValue } from '@grafana/data';
+import { EditorField } from '@grafana/experimental';
 import { reportInteraction } from '@grafana/runtime';
 import {
   Button,
   Card,
   Collapse,
   InlineField,
-  InlineLabel,
   InlineSwitch,
   Input,
   Modal,
@@ -73,12 +73,12 @@ const promTypes: PromFilterOption[] = [
 ];
 
 export const placeholders = {
-  browse: 'Browse metric names by text',
-  metadataSearchSwicth: 'Browse by metadata type and description in addition to metric name',
-  type: 'Counter, gauge, histogram, or summary',
-  variables: 'Select a template variable for your metric',
-  excludeNoMetadata: 'Exclude results with no metadata when filtering',
-  setUseBackend: 'Use the backend to browse metrics and disable fuzzy search metadata browsing',
+  browse: 'Search metrics by name',
+  metadataSearchSwitch: 'Search by metadata type and description in addition to name',
+  type: 'Select...',
+  variables: 'Select...',
+  excludeNoMetadata: 'Exclude results with no metadata',
+  setUseBackend: 'Use the backend to browse metrics',
 };
 
 export const DEFAULT_RESULTS_PER_PAGE = 10;
@@ -397,6 +397,19 @@ export const MetricEncyclopediaModal = (props: Props) => {
     });
   }
 
+  const MAXIMUM_RESULTS_PER_PAGE = 1000;
+  const calculateResultsPerPage = (results: number) => {
+    if (results < 1) {
+      return 1;
+    }
+
+    if (results > MAXIMUM_RESULTS_PER_PAGE) {
+      return MAXIMUM_RESULTS_PER_PAGE;
+    }
+
+    return results ?? 10;
+  };
+
   return (
     <Modal
       data-testid={testIds.metricModal}
@@ -404,98 +417,47 @@ export const MetricEncyclopediaModal = (props: Props) => {
       title="Browse Metrics"
       onDismiss={onClose}
       aria-label="Metric Encyclopedia"
+      className={styles.modal}
     >
-      <FeedbackLink feedbackUrl="https://forms.gle/DEMAJHoAMpe3e54CA" />
-      <div className={styles.spacing}>
-        Browse {totalMetricCount} metric{totalMetricCount > 1 ? 's' : ''} by text, by type, alphabetically or select a
-        variable.
-        {isLoading && (
-          <div className={styles.inlineSpinner}>
-            <Spinner></Spinner>
-          </div>
-        )}
-      </div>
-      {query.labels.length > 0 && (
-        <div className={styles.spacing}>
-          <i>These metrics have been pre-filtered by labels chosen in the label filters.</i>
-        </div>
-      )}
-      <div className="gf-form">
-        <Input
-          data-testid={testIds.searchMetric}
-          placeholder={placeholders.browse}
-          value={fuzzySearchQuery}
-          autoFocus
-          onInput={(e) => {
-            const value = e.currentTarget.value ?? '';
-            setFuzzySearchQuery(value);
-            if (useBackend && value === '') {
-              // get all metrics data if a user erases everything in the input
-              updateMetricsMetadata();
-            } else if (useBackend) {
-              debouncedBackendSearch(value);
-            } else {
-              // search either the names or all metadata
-              // fuzzy search go!
+      <div className={styles.inputWrapper}>
+        <div className={cx(styles.inputItem, styles.inputItemFirst)}>
+          <EditorField label="Search metrics">
+            <Input
+              data-testid={testIds.searchMetric}
+              placeholder={placeholders.browse}
+              value={fuzzySearchQuery}
+              onInput={(e) => {
+                const value = e.currentTarget.value ?? '';
+                setFuzzySearchQuery(value);
+                if (useBackend && value === '') {
+                  // get all metrics data if a user erases everything in the input
+                  updateMetricsMetadata();
+                } else if (useBackend) {
+                  debouncedBackendSearch(value);
+                } else {
+                  // search either the names or all metadata
+                  // fuzzy search go!
 
-              if (fullMetaSearch) {
-                debouncedFuzzySearch(metaHaystack, value, setFuzzyMetaSearchResults);
-              } else {
-                debouncedFuzzySearch(nameHaystack, value, setFuzzyNameSearchResults);
-              }
-            }
+                  if (fullMetaSearch) {
+                    debouncedFuzzySearch(metaHaystack, value, setFuzzyMetaSearchResults);
+                  } else {
+                    debouncedFuzzySearch(nameHaystack, value, setFuzzyNameSearchResults);
+                  }
+                }
 
-            setPageNum(1);
-          }}
-        />
-        {hasMetadata && !useBackend && (
-          <InlineField label="" className={styles.labelColor} tooltip={<div>{placeholders.metadataSearchSwicth}</div>}>
-            <InlineSwitch
-              data-testid={testIds.searchWithMetadata}
-              showLabel={true}
-              value={fullMetaSearch}
-              onChange={() => {
-                setFullMetaSearch(!fullMetaSearch);
                 setPageNum(1);
               }}
             />
-          </InlineField>
-        )}
-        <InlineField label="" className={styles.labelColor} tooltip={<div>{placeholders.setUseBackend}</div>}>
-          <InlineSwitch
-            data-testid={testIds.setUseBackend}
-            showLabel={true}
-            value={useBackend}
-            onChange={() => {
-              const newVal = !useBackend;
-              setUseBackend(newVal);
-              if (newVal === false) {
-                // rebuild the metrics metadata if we turn off useBackend
-                updateMetricsMetadata();
-              } else {
-                // check if there is text in the browse search and update
-                if (fuzzySearchQuery !== '') {
-                  debouncedBackendSearch(fuzzySearchQuery);
-                }
-                // otherwise wait for user typing
-              }
-
-              setPageNum(1);
-            }}
-          />
-        </InlineField>
-      </div>
-      {hasMetadata && !useBackend && (
-        <>
-          <div className="gf-form">
-            <h6>Filter by Type</h6>
-          </div>
-          <div className="gf-form">
+          </EditorField>
+        </div>
+        <div className={styles.inputItem}>
+          <EditorField label="Filter by type">
             <MultiSelect
               data-testid={testIds.selectType}
               inputId="my-select"
               options={typeOptions}
               value={selectedTypes}
+              disabled={!hasMetadata || useBackend}
               placeholder={placeholders.type}
               onChange={(v) => {
                 // *** Filter by type
@@ -505,137 +467,204 @@ export const MetricEncyclopediaModal = (props: Props) => {
                 setPageNum(1);
               }}
             />
-            {hasMetadata && (
-              <InlineField label="" className={styles.labelColor} tooltip={<div>{placeholders.excludeNoMetadata}</div>}>
-                <InlineSwitch
-                  showLabel={true}
-                  value={excludeNullMetadata}
-                  onChange={() => {
-                    setExcludeNullMetadata(!excludeNullMetadata);
-                    setPageNum(1);
-                  }}
-                />
-              </InlineField>
-            )}
-          </div>
-        </>
-      )}
-      <div className="gf-form">
-        <h6>Variables</h6>
+          </EditorField>
+        </div>
+        <div className={styles.inputItem}>
+          <EditorField label="Select template variables">
+            <Select
+              inputId="my-select"
+              options={variables}
+              value={''}
+              placeholder={placeholders.variables}
+              onChange={(v) => {
+                const value: string = v.value ?? '';
+                onChange({ ...query, metric: value });
+                onClose();
+              }}
+            />
+          </EditorField>
+        </div>
       </div>
-      <div className="gf-form">
-        <Select
-          inputId="my-select"
-          options={variables}
-          value={''}
-          placeholder={placeholders.variables}
-          onChange={(v) => {
-            const value: string = v.value ?? '';
-            onChange({ ...query, metric: value });
-            onClose();
-          }}
-        />
+
+      <div className={styles.selectWrapper}>
+        <EditorField label="Search Settings">
+          <>
+            <div className={styles.selectItem}>
+              <InlineSwitch
+                data-testid={testIds.searchWithMetadata}
+                value={fullMetaSearch}
+                disabled={useBackend || !hasMetadata}
+                onChange={() => {
+                  setFullMetaSearch(!fullMetaSearch);
+                  setPageNum(1);
+                }}
+              />
+              <p className={styles.selectItemLabel}>{placeholders.metadataSearchSwitch}</p>
+            </div>
+            {/* <div className={styles.selectItem}>
+                  <InlineSwitch data-testid={'im not sure what this toggle does.'} value={false} onChange={() => {}} />
+                  <p className={styles.selectItemLabel}>Disable fuzzy search metadata browsing (HELP!)</p>
+                </div> */}
+            <div className={styles.selectItem}>
+              <InlineSwitch
+                data-testid={testIds.setUseBackend}
+                value={useBackend}
+                onChange={() => {
+                  const newVal = !useBackend;
+                  setUseBackend(newVal);
+                  if (newVal === false) {
+                    // rebuild the metrics metadata if we turn off useBackend
+                    updateMetricsMetadata();
+                  } else {
+                    // check if there is text in the browse search and update
+                    if (fuzzySearchQuery !== '') {
+                      debouncedBackendSearch(fuzzySearchQuery);
+                    }
+                    // otherwise wait for user typing
+                  }
+
+                  setPageNum(1);
+                }}
+              />
+              <p className={styles.selectItemLabel}>{placeholders.setUseBackend}</p>
+            </div>
+          </>
+        </EditorField>
       </div>
-      <h5 className={`${styles.center} ${styles.topPadding}`}>{filteredMetricCount} Results</h5>
-      <div className={`${styles.center} ${styles.bottomPadding}`}>{letterSearchComponent()}</div>
-      {metrics &&
-        displayedMetrics(metrics).map((metric: MetricData, idx) => {
-          return (
-            <Collapse
-              aria-label={`open and close ${metric.value} query starter card`}
-              data-testid={testIds.metricCard}
-              key={metric.value}
-              label={metric.value}
-              isOpen={openTabs.includes(metric.value)}
-              collapsible={true}
-              onToggle={() =>
-                setOpenTabs((tabs) =>
-                  // close tab if it's already open, otherwise open it
-                  tabs.includes(metric.value) ? tabs.filter((t) => t !== metric.value) : [...tabs, metric.value]
-                )
-              }
-            >
-              <div className={styles.cardsContainer}>
-                <Card className={styles.card}>
-                  <Card.Description>
-                    {metric.description && metric.type ? (
-                      <>
-                        Type: <span className={styles.metadata}>{metric.type}</span>
-                        <br />
-                        Description: <span className={styles.metadata}>{metric.description}</span>
-                      </>
-                    ) : (
-                      <i>No metadata available</i>
-                    )}
-                  </Card.Description>
-                  <Card.Actions>
-                    {/* *** Make selecting a metric easier, consider click on text */}
-                    <Button
-                      size="sm"
-                      aria-label="use this metric button"
-                      data-testid={testIds.useMetric}
-                      onClick={() => {
-                        onChange({ ...query, metric: metric.value });
-                        reportInteraction('grafana_prom_metric_encycopedia_tracking', {
-                          metric: metric.value,
-                          hasVariables: variables.length > 0,
-                          hasMetadata: hasMetadata,
-                          totalMetricCount: metrics.length,
-                          fuzzySearchQuery: fuzzySearchQuery,
-                          fullMetaSearch: fullMetaSearch,
-                          selectedTypes: selectedTypes,
-                          letterSearch: letterSearch,
-                        });
-                        onClose();
-                      }}
-                    >
-                      Use this metric
-                    </Button>
-                  </Card.Actions>
-                </Card>
-              </div>
-            </Collapse>
-          );
-        })}
-      <br />
-      <div className="gf-form">
-        <InlineLabel width={20} className="query-keyword">
-          Select Page
-        </InlineLabel>
-        <Select
-          data-testid={testIds.searchPage}
-          options={calculatePageList(metrics, resultsPerPage).map((p) => {
-            return { value: p, label: '' + p };
+
+      <h4 className={styles.resultsHeading}>Results</h4>
+      <div className={styles.resultsData}>
+        <div className={styles.resultsDataCount}>
+          Showing {filteredMetricCount} of {totalMetricCount} total metrics.{' '}
+          {isLoading && <Spinner className={styles.loadingSpinner} />}
+        </div>
+        {query.labels.length > 0 && (
+          <p className={styles.resultsDataFiltered}>
+            These metrics have been pre-filtered by labels chosen in the label filters.
+          </p>
+        )}
+      </div>
+
+      <div className={styles.alphabetRow}>
+        <div>{letterSearchComponent()}</div>
+        <div className={styles.selectItem}>
+          <InlineSwitch
+            value={excludeNullMetadata}
+            disabled={useBackend || !hasMetadata}
+            onChange={() => {
+              setExcludeNullMetadata(!excludeNullMetadata);
+              setPageNum(1);
+            }}
+          />
+          <p className={styles.selectItemLabel}>{placeholders.excludeNoMetadata}</p>
+        </div>
+      </div>
+
+      <div className={styles.results}>
+        {metrics &&
+          displayedMetrics(metrics).map((metric: MetricData, idx) => {
+            return (
+              <Collapse
+                aria-label={`open and close ${metric.value} query starter card`}
+                data-testid={testIds.metricCard}
+                key={metric.value}
+                label={metric.value}
+                isOpen={openTabs.includes(metric.value)}
+                collapsible={true}
+                onToggle={() =>
+                  setOpenTabs((tabs) =>
+                    // close tab if it's already open, otherwise open it
+                    tabs.includes(metric.value) ? tabs.filter((t) => t !== metric.value) : [...tabs, metric.value]
+                  )
+                }
+              >
+                <div className={styles.cardsContainer}>
+                  <Card className={styles.card}>
+                    <Card.Description>
+                      {metric.description && metric.type ? (
+                        <>
+                          Type: <span className={styles.metadata}>{metric.type}</span>
+                          <br />
+                          Description: <span className={styles.metadata}>{metric.description}</span>
+                        </>
+                      ) : (
+                        <i>No metadata available</i>
+                      )}
+                    </Card.Description>
+                    <Card.Actions>
+                      {/* *** Make selecting a metric easier, consider click on text */}
+                      <Button
+                        size="sm"
+                        aria-label="use this metric button"
+                        data-testid={testIds.useMetric}
+                        onClick={() => {
+                          onChange({ ...query, metric: metric.value });
+                          reportInteraction('grafana_prom_metric_encycopedia_tracking', {
+                            metric: metric.value,
+                            hasVariables: variables.length > 0,
+                            hasMetadata: hasMetadata,
+                            totalMetricCount: metrics.length,
+                            fuzzySearchQuery: fuzzySearchQuery,
+                            fullMetaSearch: fullMetaSearch,
+                            selectedTypes: selectedTypes,
+                            letterSearch: letterSearch,
+                          });
+                          onClose();
+                        }}
+                      >
+                        Use this metric
+                      </Button>
+                    </Card.Actions>
+                  </Card>
+                </div>
+              </Collapse>
+            );
           })}
-          value={pageNum ?? 1}
-          placeholder="select page"
-          onChange={(e) => {
-            const value = e.value ?? 1;
-            setPageNum(value);
-          }}
-        />
-        <InlineLabel width={20} className="query-keyword">
-          # results per page
-        </InlineLabel>
-        <Input
-          data-testid={testIds.resultsPerPage}
-          value={resultsPerPage ?? 10}
-          placeholder="results per page"
-          onInput={(e) => {
-            const value = +e.currentTarget.value;
-
-            if (isNaN(value)) {
-              return;
-            }
-
-            setResultsPerPage(value);
-          }}
-        />
       </div>
-      <br />
-      <Button aria-label="close metric encyclopedia modal" variant="secondary" onClick={onClose}>
-        Close
-      </Button>
+
+      <div className={styles.pageSettingsWrapper}>
+        <div className={styles.pageSettings}>
+          <InlineField label="Select page" labelWidth={20} className="query-keyword">
+            <Select
+              data-testid={testIds.searchPage}
+              options={calculatePageList(metrics, resultsPerPage).map((p) => {
+                return { value: p, label: '' + p };
+              })}
+              value={pageNum ?? 1}
+              placeholder="select page"
+              width={20}
+              onChange={(e) => {
+                const value = e.value ?? 1;
+                setPageNum(value);
+              }}
+            />
+          </InlineField>
+
+          <InlineField
+            label="# results per page"
+            tooltip={'The maximum results per page is ' + MAXIMUM_RESULTS_PER_PAGE}
+            labelWidth={20}
+          >
+            <Input
+              data-testid={testIds.resultsPerPage}
+              value={calculateResultsPerPage(resultsPerPage)}
+              placeholder="results per page"
+              width={20}
+              onInput={(e) => {
+                const value = +e.currentTarget.value;
+
+                if (isNaN(value)) {
+                  return;
+                }
+
+                setResultsPerPage(value);
+              }}
+            />
+          </InlineField>
+        </div>
+
+        <FeedbackLink feedbackUrl="https://forms.gle/DEMAJHoAMpe3e54CA" />
+      </div>
     </Modal>
   );
 };
@@ -671,33 +700,83 @@ function alphabetically(ascending: boolean, metadataFilters: boolean) {
 
 const getStyles = (theme: GrafanaTheme2) => {
   return {
+    modal: css`
+      width: 85vw;
+      ${theme.breakpoints.down('md')} {
+        width: 100%;
+      }
+    `,
+    inputWrapper: css`
+      display: flex;
+      flex-direction: row;
+      gap: ${theme.spacing(2)};
+      margin-bottom: ${theme.spacing(2)};
+    `,
+    inputItemFirst: css`
+      flex-basis: 40%;
+    `,
+    inputItem: css`
+      flex-grow: 1;
+    `,
+    selectWrapper: css`
+      margin-bottom: ${theme.spacing(2)};
+    `,
+    selectItem: css`
+      display: flex;
+      flex-direction: row;
+    `,
+    selectItemLabel: css`
+      margin: 0 0 0 ${theme.spacing(1)};
+      align-self: center;
+      color: ${theme.colors.text.secondary};
+    `,
+    resultsHeading: css`
+      margin: 0 0 0 0;
+    `,
+    resultsData: css`
+      margin: 0 0 ${theme.spacing(1)} 0;
+    `,
+    resultsDataCount: css`
+      margin: 0;
+    `,
+    resultsDataFiltered: css`
+      margin: 0;
+      color: ${theme.colors.warning.main};
+    `,
+    alphabetRow: css`
+      display: flex;
+      flex-direction: row;
+      justify-content: space-between;
+      align-items: center;
+    `,
+    results: css`
+      height: 300px;
+      overflow-y: scroll;
+    `,
+    pageSettingsWrapper: css`
+      padding-top: ${theme.spacing(1.5)};
+      display: flex;
+      flex-direction: row;
+      justify-content: space-between;
+      align-items: center;
+    `,
+    pageSettings: css`
+      display: flex;
+      flex-direction: row;
+      align-items: center;
+    `,
     cardsContainer: css`
       display: flex;
       flex-direction: row;
       flex-wrap: wrap;
       justify-content: space-between;
     `,
-    spacing: css`
-      margin-bottom: ${theme.spacing(1)};
-    `,
-    center: css`
-      text-align: center;
-      padding: 4px;
-      width: 100%;
-    `,
-    topPadding: css`
-      padding: 10px 0 0 0;
-    `,
-    bottomPadding: css`
-      padding: 0 0 4px 0;
-    `,
     card: css`
       width: 100%;
       display: flex;
       flex-direction: column;
     `,
     selAlpha: css`
-      font-style: italic;
       cursor: pointer;
       color: #6e9fff;
     `,
@@ -710,10 +789,7 @@ const getStyles = (theme: GrafanaTheme2) => {
     metadata: css`
       color: rgb(204, 204, 220);
     `,
-    labelColor: css`
-      color: #6e9fff;
-    `,
-    inlineSpinner: css`
+    loadingSpinner: css`
       display: inline-block;
     `,
   };