From 9a8125ca3b14e148b1afda89e922e440923060af Mon Sep 17 00:00:00 2001
From: Laura Benz <48948963+L-M-K-B@users.noreply.github.com>
Date: Wed, 5 Jul 2023 08:47:38 +0200
Subject: [PATCH] Elasticsearch: Replace duplicate IconButton component
 (#71050)

* refactor: replace IconButtons

* refactor: replace tooltip text in order to make it more universal

* refactor: remove additional IconButton

* refactor: add aria-labels

* refactor: fix test
---
 .../components/AddRemove.test.tsx             |  8 ++---
 .../elasticsearch/components/AddRemove.tsx    | 10 ++++--
 .../elasticsearch/components/IconButton.tsx   | 34 -------------------
 .../BucketAggregationsEditor/index.tsx        | 12 +++++--
 .../MetricAggregationsEditor/index.tsx        | 12 +++++--
 .../components/QueryEditor/QueryEditorRow.tsx |  4 +--
 6 files changed, 32 insertions(+), 48 deletions(-)
 delete mode 100644 public/app/plugins/datasource/elasticsearch/components/IconButton.tsx

diff --git a/public/app/plugins/datasource/elasticsearch/components/AddRemove.test.tsx b/public/app/plugins/datasource/elasticsearch/components/AddRemove.test.tsx
index ac04cec7aca..d968f58eb72 100644
--- a/public/app/plugins/datasource/elasticsearch/components/AddRemove.test.tsx
+++ b/public/app/plugins/datasource/elasticsearch/components/AddRemove.test.tsx
@@ -18,8 +18,8 @@ describe('AddRemove Button', () => {
     it('Should only show the add button', () => {
       render(<TestComponent items={['something']} />);
 
-      expect(screen.getByText('add')).toBeInTheDocument();
-      expect(screen.queryByText('remove')).not.toBeInTheDocument();
+      expect(screen.getByLabelText('Add')).toBeInTheDocument();
+      expect(screen.queryByLabelText('Remove')).not.toBeInTheDocument();
     });
   });
 
@@ -29,7 +29,7 @@ describe('AddRemove Button', () => {
 
       render(<TestComponent items={items} />);
 
-      expect(screen.getAllByText('remove')).toHaveLength(items.length);
+      expect(screen.getAllByLabelText('Remove')).toHaveLength(items.length);
     });
 
     it('Should show the add button only once', () => {
@@ -37,7 +37,7 @@ describe('AddRemove Button', () => {
 
       render(<TestComponent items={items} />);
 
-      expect(screen.getAllByText('add')).toHaveLength(1);
+      expect(screen.getAllByLabelText('Add')).toHaveLength(1);
     });
   });
 });
diff --git a/public/app/plugins/datasource/elasticsearch/components/AddRemove.tsx b/public/app/plugins/datasource/elasticsearch/components/AddRemove.tsx
index 935167f9ae0..f2765277cd2 100644
--- a/public/app/plugins/datasource/elasticsearch/components/AddRemove.tsx
+++ b/public/app/plugins/datasource/elasticsearch/components/AddRemove.tsx
@@ -1,7 +1,7 @@
 import { css } from '@emotion/css';
 import React from 'react';
 
-import { IconButton } from './IconButton';
+import { Button } from '@grafana/ui';
 
 interface Props {
   index: number;
@@ -21,9 +21,13 @@ export const AddRemove = ({ index, onAdd, onRemove, elements }: Props) => {
         display: flex;
       `}
     >
-      {index === 0 && <IconButton iconName="plus" onClick={onAdd} label="add" />}
+      {index === 0 && (
+        <Button variant="secondary" fill="text" icon="plus" onClick={onAdd} tooltip="Add" aria-label="Add" />
+      )}
 
-      {elements.length >= 2 && <IconButton iconName="minus" onClick={onRemove} label="remove" />}
+      {elements.length >= 2 && (
+        <Button variant="secondary" fill="text" icon="minus" onClick={onRemove} tooltip="Remove" aria-label="Remove" />
+      )}
     </div>
   );
 };
diff --git a/public/app/plugins/datasource/elasticsearch/components/IconButton.tsx b/public/app/plugins/datasource/elasticsearch/components/IconButton.tsx
deleted file mode 100644
index 8758f405cd1..00000000000
--- a/public/app/plugins/datasource/elasticsearch/components/IconButton.tsx
+++ /dev/null
@@ -1,34 +0,0 @@
-import { cx, css } from '@emotion/css';
-import React, { ComponentProps, ButtonHTMLAttributes } from 'react';
-
-import { Icon } from '@grafana/ui';
-
-const SROnly = css`
-  clip: rect(0 0 0 0);
-  clip-path: inset(50%);
-  height: 1px;
-  overflow: hidden;
-  position: absolute;
-  white-space: nowrap;
-  width: 1px;
-`;
-
-interface Props {
-  iconName: ComponentProps<typeof Icon>['name'];
-  onClick: () => void;
-  className?: string;
-  label: string;
-}
-
-export const IconButton = ({
-  iconName,
-  onClick,
-  className,
-  label,
-  ...buttonProps
-}: Props & ButtonHTMLAttributes<HTMLButtonElement>) => (
-  <button className={cx('gf-form-label gf-form-label--btn query-part', className)} onClick={onClick} {...buttonProps}>
-    <span className={SROnly}>{label}</span>
-    <Icon name={iconName} aria-hidden="true" />
-  </button>
-);
diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/index.tsx b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/index.tsx
index 01ae496ea67..dfd631f3c59 100644
--- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/index.tsx
+++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/index.tsx
@@ -1,7 +1,8 @@
 import React from 'react';
 
+import { Button } from '@grafana/ui';
+
 import { useDispatch } from '../../../hooks/useStatelessReducer';
-import { IconButton } from '../../IconButton';
 import { useQuery } from '../ElasticsearchQueryContext';
 import { QueryEditorRow } from '../QueryEditorRow';
 
@@ -29,7 +30,14 @@ export const BucketAggregationsEditor = ({ nextId }: Props) => {
           <BucketAggregationEditor value={bucketAgg} />
 
           {index === 0 && (
-            <IconButton iconName="plus" onClick={() => dispatch(addBucketAggregation(nextId))} label="add" />
+            <Button
+              variant="secondary"
+              fill="text"
+              icon="plus"
+              onClick={() => dispatch(addBucketAggregation(nextId))}
+              tooltip="Add grouping condition"
+              aria-label="Add grouping condition"
+            />
           )}
         </QueryEditorRow>
       ))}
diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/index.tsx b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/index.tsx
index 7051d3a3f05..982202be9f1 100644
--- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/index.tsx
+++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/MetricAggregationsEditor/index.tsx
@@ -1,9 +1,8 @@
 import React from 'react';
 
-import { Alert } from '@grafana/ui';
+import { Alert, Button } from '@grafana/ui';
 
 import { useDispatch } from '../../../hooks/useStatelessReducer';
-import { IconButton } from '../../IconButton';
 import { useQuery } from '../ElasticsearchQueryContext';
 import { QueryEditorRow } from '../QueryEditorRow';
 import { QueryEditorSpecialMetricRow } from '../QueryEditorSpecialMetricRow';
@@ -49,7 +48,14 @@ export const MetricAggregationsEditor = ({ nextId }: Props) => {
                 <MetricEditor value={metric} />
 
                 {metricAggregationConfig[metric.type].impliedQueryType === 'metrics' && index === 0 && (
-                  <IconButton iconName="plus" onClick={() => dispatch(addMetric(nextId))} label="add" />
+                  <Button
+                    variant="secondary"
+                    fill="text"
+                    icon="plus"
+                    onClick={() => dispatch(addMetric(nextId))}
+                    tooltip="Add metric"
+                    aria-label="Add metric"
+                  />
                 )}
               </QueryEditorRow>
             );
diff --git a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/QueryEditorRow.tsx b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/QueryEditorRow.tsx
index 1152640cc61..297c8372625 100644
--- a/public/app/plugins/datasource/elasticsearch/components/QueryEditor/QueryEditorRow.tsx
+++ b/public/app/plugins/datasource/elasticsearch/components/QueryEditor/QueryEditorRow.tsx
@@ -34,7 +34,7 @@ export const QueryEditorRow = ({
                 size="sm"
                 aria-pressed={hidden}
                 className={styles.icon}
-                tooltip="Hide metric"
+                tooltip="Hide row"
               />
             )}
             <IconButton
@@ -43,7 +43,7 @@ export const QueryEditorRow = ({
               className={styles.icon}
               onClick={onRemoveClick || noop}
               disabled={!onRemoveClick}
-              tooltip="Remove metric"
+              tooltip="Remove row"
             />
           </span>
         </InlineLabel>