TagsInput: fix tags remove button accessibility issues (#46254)

* TagsInput: fix remove button focusable state

* Add tests

* use IconButton

* reverted style changes & disable iconbutton hover animation

packages/grafana-ui/src/components/TagsInput/TagItem.tsx

@@ -2,12 +2,12 @@ import React, { FC } from 'react';
 import { css } from '@emotion/css';
 import { getTagColorsFromName } from '../../utils';
 import { stylesFactory, useTheme } from '../../themes';
-import { Icon } from '../Icon/Icon';
 import { GrafanaTheme } from '@grafana/data';
+import { IconButton } from '../IconButton/IconButton';
 
 interface Props {
   name: string;
-
+  disabled?: boolean;
   onRemove: (tag: string) => void;
 }
 
@@ -36,6 +36,13 @@ const getStyles = stylesFactory(({ theme, name }: { theme: GrafanaTheme; name: s
     nameStyle: css`
       margin-right: 3px;
     `,
+
+    buttonStyles: css`
+      margin: 0;
+      &:hover::before {
+        display: none;
+      }
+    `,
   };
 });
 
@@ -43,14 +50,22 @@ const getStyles = stylesFactory(({ theme, name }: { theme: GrafanaTheme; name: s
  * @internal
  * Only used internally by TagsInput
  * */
-export const TagItem: FC<Props> = ({ name, onRemove }) => {
+export const TagItem: FC<Props> = ({ name, disabled, onRemove }) => {
   const theme = useTheme();
   const styles = getStyles({ theme, name });
 
   return (
     <div className={styles.itemStyle}>
       <span className={styles.nameStyle}>{name}</span>
-      <Icon className="pointer" name="times" onClick={() => onRemove(name)} />
+      <IconButton
+        name="times"
+        size="lg"
+        disabled={disabled}
+        ariaLabel={`Remove ${name}`}
+        onClick={() => onRemove(name)}
+        type="button"
+        className={styles.buttonStyles}
+      />
     </div>
   );
 };

packages/grafana-ui/src/components/TagsInput/TagsInput.test.tsx

@@ -0,0 +1,23 @@
+import React from 'react';
+import { render, fireEvent, screen } from '@testing-library/react';
+import { TagsInput } from './TagsInput';
+
+describe('TagsInput', () => {
+  it('removes tag when clicking on remove button', async () => {
+    const onChange = jest.fn();
+    render(<TagsInput onChange={onChange} tags={['One', 'Two']} />);
+
+    fireEvent.click(await screen.findByRole('button', { name: /remove one/i }));
+
+    expect(onChange).toHaveBeenCalledWith(['Two']);
+  });
+
+  it('does NOT remove tag when clicking on remove button when disabled', async () => {
+    const onChange = jest.fn();
+    render(<TagsInput onChange={onChange} tags={['One', 'Two']} disabled />);
+
+    fireEvent.click(await screen.findByRole('button', { name: /remove one/i }));
+
+    expect(onChange).not.toHaveBeenCalled();
+  });
+});

packages/grafana-ui/src/components/TagsInput/TagsInput.tsx

@@ -42,10 +42,7 @@ export const TagsInput: FC<Props> = ({
   };
 
   const onRemove = (tagToRemove: string) => {
-    if (disabled) {
-      return;
-    }
-    onChange(tags?.filter((x) => x !== tagToRemove));
+    onChange(tags.filter((x) => x !== tagToRemove));
   };
 
   const onAdd = (event?: React.MouseEvent) => {
@@ -74,7 +71,7 @@ export const TagsInput: FC<Props> = ({
     <div className={cx(styles.wrapper, className, width ? css({ width: theme.spacing(width) }) : '')}>
       <div className={tags?.length ? styles.tags : undefined}>
         {tags?.map((tag: string, index: number) => {
-          return <TagItem key={`${tag}-${index}`} name={tag} onRemove={onRemove} />;
+          return <TagItem key={`${tag}-${index}`} name={tag} onRemove={onRemove} disabled={disabled} />;
         })}
       </div>
       <div>