TagsInput: Prevent adding duplicate tags + refactor, restyle (#56485)

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

@@ -1,9 +1,9 @@
 import { css } from '@emotion/css';
-import React, { FC } from 'react';
+import React, { useMemo } from 'react';
 
-import { GrafanaTheme } from '@grafana/data';
+import { GrafanaTheme2 } from '@grafana/data';
 
-import { stylesFactory, useTheme } from '../../themes';
+import { useStyles2 } from '../../themes';
 import { getTagColorsFromName } from '../../utils';
 import { IconButton } from '../IconButton/IconButton';
 
@@ -13,61 +13,60 @@ interface Props {
   onRemove: (tag: string) => void;
 }
 
-const getStyles = stylesFactory(({ theme, name }: { theme: GrafanaTheme; name: string }) => {
-  const { color, borderColor } = getTagColorsFromName(name);
-  const height = theme.spacing.formInputHeight - 8;
-
-  return {
-    itemStyle: css`
-      display: flex;
-      align-items: center;
-      height: ${height}px;
-      line-height: ${height - 2}px;
-      background-color: ${color};
-      color: ${theme.palette.white};
-      border: 1px solid ${borderColor};
-      border-radius: 3px;
-      padding: 0 ${theme.spacing.xs};
-      margin-right: 3px;
-      white-space: nowrap;
-      text-shadow: none;
-      font-weight: 500;
-      font-size: ${theme.typography.size.sm};
-    `,
-
-    nameStyle: css`
-      margin-right: 3px;
-    `,
-
-    buttonStyles: css`
-      margin: 0;
-      &:hover::before {
-        display: none;
-      }
-    `,
-  };
-});
-
 /**
  * @internal
  * Only used internally by TagsInput
  * */
-export const TagItem: FC<Props> = ({ name, disabled, onRemove }) => {
+export const TagItem = ({ name, disabled, onRemove }: Props) => {
-  const theme = useTheme();
+  const { color, borderColor } = useMemo(() => getTagColorsFromName(name), [name]);
-  const styles = getStyles({ theme, name });
+  const styles = useStyles2(getStyles);
 
   return (
-    <div className={styles.itemStyle}>
+    <li className={styles.itemStyle} style={{ backgroundColor: color, borderColor }}>
       <span className={styles.nameStyle}>{name}</span>
       <IconButton
         name="times"
         size="lg"
         disabled={disabled}
-        ariaLabel={`Remove ${name}`}
+        ariaLabel={`Remove "${name}" tag`}
         onClick={() => onRemove(name)}
         type="button"
         className={styles.buttonStyles}
       />
-    </div>
+    </li>
   );
 };
+
+const getStyles = (theme: GrafanaTheme2) => {
+  const height = theme.spacing.gridSize * 3;
+
+  return {
+    itemStyle: css({
+      display: 'flex',
+      gap: '3px',
+      alignItems: 'center',
+      height: `${height}px`,
+      lineHeight: `${height - 2}px`,
+      color: '#fff',
+      borderWidth: '1px',
+      borderStyle: 'solid',
+      borderRadius: '3px',
+      padding: `0 ${theme.spacing(0.5)}`,
+      whiteSpace: 'nowrap',
+      textShadow: 'none',
+      fontWeight: 500,
+      fontSize: theme.typography.size.sm,
+    }),
+    nameStyle: css({
+      maxWidth: '25ch',
+      textOverflow: 'ellipsis',
+      overflow: 'hidden',
+    }),
+    buttonStyles: css({
+      margin: 0,
+      '&:hover::before': {
+        display: 'none',
+      },
+    }),
+  };
+};

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

@@ -8,7 +8,7 @@ describe('TagsInput', () => {
     const onChange = jest.fn();
     render(<TagsInput onChange={onChange} tags={['One', 'Two']} />);
 
-    fireEvent.click(await screen.findByRole('button', { name: /remove one/i }));
+    fireEvent.click(await screen.findByRole('button', { name: /Remove \"One\"/i }));
 
     expect(onChange).toHaveBeenCalledWith(['Two']);
   });
@@ -17,7 +17,7 @@ describe('TagsInput', () => {
     const onChange = jest.fn();
     render(<TagsInput onChange={onChange} tags={['One', 'Two']} disabled />);
 
-    fireEvent.click(await screen.findByRole('button', { name: /remove one/i }));
+    fireEvent.click(await screen.findByRole('button', { name: /Remove \"One\"/i }));
 
     expect(onChange).not.toHaveBeenCalled();
   });

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

@@ -1,9 +1,9 @@
 import { css, cx } from '@emotion/css';
-import React, { ChangeEvent, KeyboardEvent, FC, useState } from 'react';
+import React, { useCallback, useState } from 'react';
 
-import { GrafanaTheme } from '@grafana/data';
+import { GrafanaTheme2 } from '@grafana/data';
 
-import { useStyles, useTheme2 } from '../../themes/ThemeContext';
+import { useStyles2, useTheme2 } from '../../themes/ThemeContext';
 import { Button } from '../Button';
 import { Input } from '../Input/Input';
 
@@ -25,7 +25,7 @@ export interface Props {
   invalid?: boolean;
 }
 
-export const TagsInput: FC<Props> = ({
+export const TagsInput = ({
   placeholder = 'New tag (enter key to add)',
   tags = [],
   onChange,
@@ -35,25 +35,25 @@ export const TagsInput: FC<Props> = ({
   addOnBlur,
   invalid,
   id,
-}) => {
+}: Props) => {
-  const [newTagName, setNewName] = useState('');
+  const [newTagName, setNewTagName] = useState('');
-  const styles = useStyles(getStyles);
+  const styles = useStyles2(getStyles);
   const theme = useTheme2();
 
-  const onNameChange = (event: ChangeEvent<HTMLInputElement>) => {
+  const onNameChange = useCallback((event: React.ChangeEvent<HTMLInputElement>) => {
-    setNewName(event.target.value);
+    setNewTagName(event.target.value);
-  };
+  }, []);
 
   const onRemove = (tagToRemove: string) => {
     onChange(tags.filter((x) => x !== tagToRemove));
   };
 
-  const onAdd = (event?: React.MouseEvent) => {
+  const onAdd = (event?: React.MouseEvent | React.KeyboardEvent) => {
     event?.preventDefault();
     if (!tags.includes(newTagName)) {
       onChange(tags.concat(newTagName));
     }
-    setNewName('');
+    setNewTagName('');
   };
 
   const onBlur = () => {
@@ -62,65 +62,61 @@ export const TagsInput: FC<Props> = ({
     }
   };
 
-  const onKeyboardAdd = (event: KeyboardEvent) => {
+  const onKeyboardAdd = (event: React.KeyboardEvent) => {
-    event.preventDefault();
     if (event.key === 'Enter' && newTagName !== '') {
-      onChange(tags.concat(newTagName));
+      onAdd(event);
-      setNewName('');
     }
   };
 
   return (
     <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} disabled={disabled} />;
-        })}
-      </div>
-      <div>
       <Input
         id={id}
         disabled={disabled}
         placeholder={placeholder}
         onChange={onNameChange}
         value={newTagName}
-          onKeyUp={onKeyboardAdd}
+        onKeyDown={onKeyboardAdd}
-          onKeyDown={(e) => {
-            // onKeyDown is triggered before onKeyUp, triggering submit behaviour on Enter press if this component
-            // is used inside forms. Moving onKeyboardAdd callback here doesn't work since text input is not captured in onKeyDown
-            if (e.key === 'Enter') {
-              e.preventDefault();
-            }
-          }}
         onBlur={onBlur}
         invalid={invalid}
         suffix={
-            newTagName.length > 0 && (
+          <Button
-              <Button fill="text" className={styles.addButtonStyle} onClick={onAdd} size="md">
+            fill="text"
+            className={styles.addButtonStyle}
+            onClick={onAdd}
+            size="md"
+            disabled={newTagName.length <= 0}
+          >
             Add
           </Button>
-            )
         }
       />
-      </div>
+      {tags?.length > 0 && (
+        <ul className={styles.tags}>
+          {tags.map((tag) => (
+            <TagItem key={tag} name={tag} onRemove={onRemove} disabled={disabled} />
+          ))}
+        </ul>
+      )}
     </div>
   );
 };
 
-const getStyles = (theme: GrafanaTheme) => ({
+const getStyles = (theme: GrafanaTheme2) => ({
   wrapper: css`
-    min-height: ${theme.spacing.formInputHeight}px;
+    min-height: ${theme.spacing(4)};
-    align-items: center;
     display: flex;
+    flex-direction: column;
+    gap: ${theme.spacing(1)};
     flex-wrap: wrap;
   `,
   tags: css`
     display: flex;
     justify-content: flex-start;
     flex-wrap: wrap;
-    margin-right: ${theme.spacing.xs};
+    gap: ${theme.spacing(0.5)};
   `,
   addButtonStyle: css`
-    margin: 0 -${theme.spacing.sm};
+    margin: 0 -${theme.spacing(1)};
   `,
 });

public/app/features/dashboard/components/DashboardSettings/GeneralSettings.tsx

@@ -101,7 +101,7 @@ export function GeneralSettingsUnconnected({
             <Input id="description-input" name="description" onBlur={onBlur} defaultValue={dashboard.description} />
           </Field>
           <Field label="Tags">
-            <TagsInput id="tags-input" tags={dashboard.tags} onChange={onTagsChange} />
+            <TagsInput id="tags-input" tags={dashboard.tags} onChange={onTagsChange} width={40} />
           </Field>
           <Field label="Folder">
             <FolderPicker

public/app/features/dashboard/components/LinksSettings/LinkSettingsEdit.tsx

@@ -94,7 +94,7 @@ export const LinkSettingsEdit: React.FC<LinkSettingsEditProps> = ({ editLinkIdx,
       {linkSettings.type === 'dashboards' && (
         <>
           <Field label="With tags">
-            <TagsInput tags={linkSettings.tags} placeholder="add tags" onChange={onTagsChange} />
+            <TagsInput tags={linkSettings.tags} onChange={onTagsChange} />
           </Field>
         </>
       )}

public/app/plugins/datasource/graphite/components/AnnotationsEditor.tsx

@@ -49,7 +49,7 @@ export const AnnotationEditor = (props: QueryEditorProps<GraphiteDatasource, Gra
 
       <div className="gf-form">
         <InlineFormLabel width={12}>Graphite events tags</InlineFormLabel>
-        <TagsInput id="tags-input" tags={tags} onChange={onTagsChange} placeholder="Example: event_tag" />
+        <TagsInput id="tags-input" width={50} tags={tags} onChange={onTagsChange} placeholder="Example: event_tag" />
       </div>
     </div>
   );