From b3540b5f461dfe5f599b54aec806f28aeec28837 Mon Sep 17 00:00:00 2001
From: Erik Sundell <erik.sundell87@gmail.com>
Date: Tue, 3 Jan 2023 09:56:01 +0100
Subject: [PATCH] Cloudwatch: Add feedback labels to log groups selector
 (#60619)

* add max item text

* add tests

* add selected log groups counter label

* cleanup

* fix styling in selector fields

* remove unused imports

* move selected log groups to a new component

* add confirm dialog

* remove not used import

* set max logs groups to 6

* add margin bottom
---
 .../plugins/datasource/cloudwatch/Search.tsx  |  7 +-
 .../CrossAccountLogsQueryField.test.tsx       | 40 ++++++++
 .../components/CrossAccountLogsQueryField.tsx | 51 +++++-----
 .../components/SelectedLogsGroups.test.tsx    | 93 +++++++++++++++++++
 .../components/SelectedLogsGroups.tsx         | 84 +++++++++++++++++
 .../cloudwatch/components/styles.ts           | 37 +++++---
 6 files changed, 270 insertions(+), 42 deletions(-)
 create mode 100644 public/app/plugins/datasource/cloudwatch/components/SelectedLogsGroups.test.tsx
 create mode 100644 public/app/plugins/datasource/cloudwatch/components/SelectedLogsGroups.tsx

diff --git a/public/app/plugins/datasource/cloudwatch/Search.tsx b/public/app/plugins/datasource/cloudwatch/Search.tsx
index e7af681ffe1..a7af3070bf3 100644
--- a/public/app/plugins/datasource/cloudwatch/Search.tsx
+++ b/public/app/plugins/datasource/cloudwatch/Search.tsx
@@ -1,14 +1,11 @@
 import { debounce } from 'lodash';
 import React, { useEffect, useMemo, useState } from 'react';
 
-import { Icon, Input, useStyles2 } from '@grafana/ui';
-
-import getStyles from './components/styles';
+import { Icon, Input } from '@grafana/ui';
 
 // TODO: consider moving search into grafana/ui, this is mostly the same as that in azure monitor
 const Search = ({ searchFn, searchPhrase }: { searchPhrase: string; searchFn: (searchPhrase: string) => void }) => {
   const [searchFilter, setSearchFilter] = useState(searchPhrase);
-  const styles = useStyles2(getStyles);
 
   const debouncedSearch = useMemo(() => debounce(searchFn, 600), [searchFn]);
 
@@ -21,8 +18,6 @@ const Search = ({ searchFn, searchPhrase }: { searchPhrase: string; searchFn: (s
 
   return (
     <Input
-      className={styles.search}
-      width={64}
       aria-label="log group search"
       prefix={<Icon name="search" />}
       value={searchFilter}
diff --git a/public/app/plugins/datasource/cloudwatch/components/CrossAccountLogsQueryField.test.tsx b/public/app/plugins/datasource/cloudwatch/components/CrossAccountLogsQueryField.test.tsx
index 5dc04fc59fb..f3eda22db87 100644
--- a/public/app/plugins/datasource/cloudwatch/components/CrossAccountLogsQueryField.test.tsx
+++ b/public/app/plugins/datasource/cloudwatch/components/CrossAccountLogsQueryField.test.tsx
@@ -177,4 +177,44 @@ describe('CrossAccountLogsQueryField', () => {
       },
     ]);
   });
+
+  const labelText =
+    'Only the first 50 results can be shown. If you do not see an expected log group, try narrowing down your search.';
+  it('should not display max result info label in case less than 50 logs groups are being displayed', async () => {
+    const defer = new Deferred();
+    const fetchLogGroups = jest.fn(async () => {
+      await Promise.all([defer.promise]);
+      return [];
+    });
+    render(<CrossAccountLogsQueryField {...defaultProps} fetchLogGroups={fetchLogGroups} />);
+    await userEvent.click(screen.getByText('Select Log Groups'));
+    expect(screen.queryByText(labelText)).not.toBeInTheDocument();
+    defer.resolve();
+    await waitFor(() => expect(screen.queryByText(labelText)).not.toBeInTheDocument());
+  });
+
+  it('should display max result info label in case 50 or more logs groups are being displayed', async () => {
+    const defer = new Deferred();
+    const fetchLogGroups = jest.fn(async () => {
+      await Promise.all([defer.promise]);
+      return Array(50).map((i) => ({
+        value: `logGroup${i}`,
+        text: `logGroup${i}`,
+        label: `logGroup${i}`,
+      }));
+    });
+    render(<CrossAccountLogsQueryField {...defaultProps} fetchLogGroups={fetchLogGroups} />);
+    await userEvent.click(screen.getByText('Select Log Groups'));
+    expect(screen.queryByText(labelText)).not.toBeInTheDocument();
+    defer.resolve();
+    await waitFor(() => expect(screen.getByText(labelText)).toBeInTheDocument());
+  });
+
+  it('should display log groups counter label', async () => {
+    render(<CrossAccountLogsQueryField {...defaultProps} selectedLogGroups={[]} />);
+    await userEvent.click(screen.getByText('Select Log Groups'));
+    await waitFor(() => expect(screen.getByText('0 log groups selected')).toBeInTheDocument());
+    await userEvent.click(screen.getByLabelText('logGroup2'));
+    await waitFor(() => expect(screen.getByText('1 log group selected')).toBeInTheDocument());
+  });
 });
diff --git a/public/app/plugins/datasource/cloudwatch/components/CrossAccountLogsQueryField.tsx b/public/app/plugins/datasource/cloudwatch/components/CrossAccountLogsQueryField.tsx
index b328eeca6ee..59e156b3a01 100644
--- a/public/app/plugins/datasource/cloudwatch/components/CrossAccountLogsQueryField.tsx
+++ b/public/app/plugins/datasource/cloudwatch/components/CrossAccountLogsQueryField.tsx
@@ -2,13 +2,14 @@ import React, { useMemo, useState } from 'react';
 
 import { SelectableValue } from '@grafana/data';
 import { EditorField, Space } from '@grafana/experimental';
-import { Button, Checkbox, IconButton, LoadingPlaceholder, Modal, useStyles2 } from '@grafana/ui';
+import { Button, Checkbox, Icon, Label, LoadingPlaceholder, Modal, useStyles2 } from '@grafana/ui';
 
 import Search from '../Search';
 import { SelectableResourceValue } from '../api';
 import { DescribeLogGroupsRequest } from '../types';
 
 import { Account, ALL_ACCOUNTS_OPTION } from './Account';
+import { SelectedLogsGroups } from './SelectedLogsGroups';
 import getStyles from './styles';
 
 type CrossAccountLogsQueryProps = {
@@ -81,15 +82,17 @@ export const CrossAccountLogsQueryField = (props: CrossAccountLogsQueryProps) =>
     <>
       <Modal className={styles.modal} title="Select Log Groups" isOpen={isModalOpen} onDismiss={toggleModal}>
         <div className={styles.logGroupSelectionArea}>
-          <EditorField label="Log Group Name">
-            <Search
-              searchFn={(phrase) => {
-                searchFn(phrase, searchAccountId);
-                setSearchPhrase(phrase);
-              }}
-              searchPhrase={searchPhrase}
-            />
-          </EditorField>
+          <div className={styles.searchField}>
+            <EditorField label="Log Group Name">
+              <Search
+                searchFn={(phrase) => {
+                  searchFn(phrase, searchAccountId);
+                  setSearchPhrase(phrase);
+                }}
+                searchPhrase={searchPhrase}
+              />
+            </EditorField>
+          </div>
 
           <Account
             onChange={(accountId?: string) => {
@@ -102,6 +105,16 @@ export const CrossAccountLogsQueryField = (props: CrossAccountLogsQueryProps) =>
         </div>
         <Space layout="block" v={2} />
         <div>
+          {!isLoading && selectableLogGroups.length >= 25 && (
+            <>
+              <Label className={styles.limitLabel}>
+                <Icon name="info-circle"></Icon>
+                Only the first 50 results can be shown. If you do not see an expected log group, try narrowing down your
+                search.
+              </Label>
+              <Space layout="block" v={1} />
+            </>
+          )}
           <div className={styles.tableScroller}>
             <table className={styles.table}>
               <thead>
@@ -149,6 +162,10 @@ export const CrossAccountLogsQueryField = (props: CrossAccountLogsQueryProps) =>
           </div>
         </div>
         <Space layout="block" v={2} />
+        <Label className={styles.logGroupCountLabel}>
+          {selectedLogGroups.length} log group{selectedLogGroups.length !== 1 && 's'} selected
+        </Label>
+        <Space layout="block" v={1.5} />
         <div>
           <Button onClick={handleApply} type="button" className={styles.addBtn}>
             Add log groups
@@ -165,19 +182,7 @@ export const CrossAccountLogsQueryField = (props: CrossAccountLogsQueryProps) =>
         </Button>
       </div>
 
-      <div className={styles.selectedLogGroupsContainer}>
-        {props.selectedLogGroups.map((lg) => (
-          <div key={lg.value} className={styles.selectedLogGroup}>
-            {lg.label}
-            <IconButton
-              size="sm"
-              name="times"
-              className={styles.removeButton}
-              onClick={() => props.onChange(props.selectedLogGroups.filter((slg) => slg.value !== lg.value))}
-            />
-          </div>
-        ))}
-      </div>
+      <SelectedLogsGroups selectedLogGroups={props.selectedLogGroups} onChange={props.onChange}></SelectedLogsGroups>
     </>
   );
 };
diff --git a/public/app/plugins/datasource/cloudwatch/components/SelectedLogsGroups.test.tsx b/public/app/plugins/datasource/cloudwatch/components/SelectedLogsGroups.test.tsx
new file mode 100644
index 00000000000..b514d386903
--- /dev/null
+++ b/public/app/plugins/datasource/cloudwatch/components/SelectedLogsGroups.test.tsx
@@ -0,0 +1,93 @@
+import { render, screen, waitFor } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import React from 'react';
+
+import { SelectedLogsGroups } from './SelectedLogsGroups';
+
+const defaultProps = {
+  selectedLogGroups: [
+    {
+      value: 'aws/lambda/lambda-name1',
+      label: 'aws/lambda/lambda-name1',
+      text: 'aws/lambda/lambda-name1',
+    },
+    {
+      value: 'aws/lambda/lambda-name2',
+      label: 'aws/lambda/lambda-name2',
+      text: 'aws/lambda/lambda-name2',
+    },
+  ],
+  onChange: jest.fn(),
+};
+
+describe('SelectedLogsGroups', () => {
+  afterEach(() => {
+    jest.resetAllMocks();
+  });
+  describe("'Show more' button", () => {
+    it('should not be displayed in case 0 logs have been selected', async () => {
+      render(<SelectedLogsGroups {...defaultProps} selectedLogGroups={[]} />);
+      await waitFor(() => expect(screen.queryByText('Show all')).not.toBeInTheDocument());
+    });
+    it('should not be displayed in case logs group have been selected but theyre less than 10', async () => {
+      render(<SelectedLogsGroups {...defaultProps} />);
+      await waitFor(() => expect(screen.queryByText('Show all')).not.toBeInTheDocument());
+    });
+    it('should be displayed in case more than 10 log groups have been selected', async () => {
+      const selectedLogGroups = Array(12).map((i) => ({
+        value: `logGroup${i}`,
+        text: `logGroup${i}`,
+        label: `logGroup${i}`,
+      }));
+      render(<SelectedLogsGroups {...defaultProps} selectedLogGroups={selectedLogGroups} />);
+      await waitFor(() => expect(screen.getByText('Show all')).toBeInTheDocument());
+    });
+  });
+
+  describe("'Clear selection' button", () => {
+    it('should not be displayed in case 0 logs have been selected', async () => {
+      render(<SelectedLogsGroups {...defaultProps} selectedLogGroups={[]} />);
+      await waitFor(() => expect(screen.queryByText('Clear selection')).not.toBeInTheDocument());
+    });
+    it('should be displayed in case at least one log group have been selected', async () => {
+      const selectedLogGroups = Array(11).map((i) => ({
+        value: `logGroup${i}`,
+        text: `logGroup${i}`,
+        label: `logGroup${i}`,
+      }));
+      render(<SelectedLogsGroups {...defaultProps} selectedLogGroups={selectedLogGroups} />);
+      await waitFor(() => expect(screen.getByText('Clear selection')).toBeInTheDocument());
+    });
+
+    it('should display confirm dialog before clearing all selections', async () => {
+      const selectedLogGroups = Array(11).map((i) => ({
+        value: `logGroup${i}`,
+        text: `logGroup${i}`,
+        label: `logGroup${i}`,
+      }));
+      render(<SelectedLogsGroups {...defaultProps} selectedLogGroups={selectedLogGroups} />);
+      await waitFor(() => userEvent.click(screen.getByText('Clear selection')));
+      await waitFor(() =>
+        expect(screen.getByText('Are you sure you want to clear all log groups?')).toBeInTheDocument()
+      );
+      await waitFor(() => userEvent.click(screen.getByLabelText('Confirm Modal Danger Button')));
+      expect(defaultProps.onChange).toHaveBeenCalledWith([]);
+    });
+  });
+
+  describe("'Clear selection' button", () => {
+    it('should not be displayed in case 0 logs have been selected', async () => {
+      render(<SelectedLogsGroups {...defaultProps} selectedLogGroups={[]} />);
+      await waitFor(() => expect(screen.queryByText('Clear selection')).not.toBeInTheDocument());
+    });
+    it('should be displayed in case at least one log group have been selected', async () => {
+      const selectedLogGroups = Array(11).map((i) => ({
+        value: `logGroup${i}`,
+        text: `logGroup${i}`,
+        label: `logGroup${i}`,
+      }));
+      render(<SelectedLogsGroups {...defaultProps} selectedLogGroups={selectedLogGroups} />);
+      await waitFor(() => expect(screen.getByText('Clear selection')).toBeInTheDocument());
+    });
+  });
+});
diff --git a/public/app/plugins/datasource/cloudwatch/components/SelectedLogsGroups.tsx b/public/app/plugins/datasource/cloudwatch/components/SelectedLogsGroups.tsx
new file mode 100644
index 00000000000..0733da6aabd
--- /dev/null
+++ b/public/app/plugins/datasource/cloudwatch/components/SelectedLogsGroups.tsx
@@ -0,0 +1,84 @@
+import React, { useEffect, useState } from 'react';
+
+import { Button, ConfirmModal, useStyles2 } from '@grafana/ui';
+
+import { SelectableResourceValue } from '../api';
+
+import getStyles from './styles';
+
+type CrossAccountLogsQueryProps = {
+  selectedLogGroups: SelectableResourceValue[];
+  onChange: (selectedLogGroups: SelectableResourceValue[]) => void;
+};
+
+const MAX_VISIBLE_LOG_GROUPS = 6;
+
+export const SelectedLogsGroups = ({ selectedLogGroups, onChange }: CrossAccountLogsQueryProps) => {
+  const styles = useStyles2(getStyles);
+  const [showConfirm, setShowConfirm] = useState(false);
+  const [visibleSelectecLogGroups, setVisibleSelectecLogGroups] = useState(
+    selectedLogGroups.slice(0, MAX_VISIBLE_LOG_GROUPS)
+  );
+
+  useEffect(() => {
+    setVisibleSelectecLogGroups(selectedLogGroups.slice(0, MAX_VISIBLE_LOG_GROUPS));
+  }, [selectedLogGroups]);
+
+  return (
+    <>
+      <div className={styles.selectedLogGroupsContainer}>
+        {visibleSelectecLogGroups.map((lg) => (
+          <Button
+            key={lg.value}
+            size="sm"
+            variant="secondary"
+            icon="times"
+            className={styles.removeButton}
+            onClick={() => {
+              onChange(selectedLogGroups.filter((slg) => slg.value !== lg.value));
+            }}
+          >
+            {lg.label}
+          </Button>
+        ))}
+        {visibleSelectecLogGroups.length !== selectedLogGroups.length && (
+          <Button
+            size="sm"
+            variant="secondary"
+            icon="plus"
+            fill="outline"
+            className={styles.removeButton}
+            onClick={() => setVisibleSelectecLogGroups(selectedLogGroups)}
+          >
+            Show all
+          </Button>
+        )}
+        {selectedLogGroups.length > 0 && (
+          <Button
+            size="sm"
+            variant="secondary"
+            icon="times"
+            fill="outline"
+            className={styles.removeButton}
+            onClick={() => setShowConfirm(true)}
+          >
+            Clear selection
+          </Button>
+        )}
+      </div>
+      <ConfirmModal
+        isOpen={showConfirm}
+        title="Clear Log Group Selection"
+        body="Are you sure you want to clear all log groups?"
+        confirmText="Yes"
+        dismissText="No"
+        icon="exclamation-triangle"
+        onConfirm={() => {
+          setShowConfirm(false);
+          onChange([]);
+        }}
+        onDismiss={() => setShowConfirm(false)}
+      />
+    </>
+  );
+};
diff --git a/public/app/plugins/datasource/cloudwatch/components/styles.ts b/public/app/plugins/datasource/cloudwatch/components/styles.ts
index 5371a277ece..1903d570845 100644
--- a/public/app/plugins/datasource/cloudwatch/components/styles.ts
+++ b/public/app/plugins/datasource/cloudwatch/components/styles.ts
@@ -10,8 +10,27 @@ const getStyles = (theme: GrafanaTheme2) => ({
 
   selectedLogGroupsContainer: css({
     marginLeft: theme.spacing(0.5),
+    marginBottom: theme.spacing(0.5),
     display: 'flex',
     flexFlow: 'wrap',
+    gap: theme.spacing(1),
+    button: {
+      margin: 'unset',
+    },
+  }),
+
+  limitLabel: css({
+    color: theme.colors.text.secondary,
+    textAlign: 'center',
+    maxWidth: 'none',
+    svg: {
+      marginRight: theme.spacing(0.5),
+    },
+  }),
+
+  logGroupCountLabel: css({
+    color: theme.colors.text.secondary,
+    maxWidth: 'none',
   }),
 
   tableScroller: css({
@@ -61,24 +80,16 @@ const getStyles = (theme: GrafanaTheme2) => ({
     display: 'flex',
   }),
 
+  searchField: css({
+    width: '100%',
+    marginRight: theme.spacing(1),
+  }),
+
   resultLimit: css({
     margin: '4px 0',
     fontStyle: 'italic',
   }),
 
-  selectedLogGroup: css({
-    background: theme.colors.background.secondary,
-    borderRadius: theme.shape.borderRadius(),
-    margin: theme.spacing(0, 1, 1, 0),
-    padding: theme.spacing(0.5, 0, 0.5, 1),
-    color: theme.colors.text.primary,
-    fontSize: theme.typography.size.sm,
-  }),
-
-  search: css({
-    marginRight: '10px',
-  }),
-
   removeButton: css({
     verticalAlign: 'middle',
     marginLeft: theme.spacing(0.5),