Dynamic dashboards: Warn if row or tab title not unique (#103346)

* warn if row or tab title not unique * Make i18n-extract * minor copy * minor copy update again
2025-04-03 13:31:07 +02:00
parent 3450d243b9
commit 17d075d81d
10 changed files with 131 additions and 24 deletions
@@ -10,6 +10,7 @@ import { getFocusStyles } from '../../themes/mixins';
 import { IconName } from '../../types';
 import { clearButtonStyles } from '../Button';
 import { Icon } from '../Icon/Icon';
+import { Tooltip } from '../Tooltip/Tooltip';

 import { Counter } from './Counter';

@@ -25,10 +26,14 @@ export interface TabProps extends HTMLProps<HTMLElement> {
  /** Extra content, displayed after the tab label and counter */
  suffix?: NavModelItem['tabSuffix'];
  truncate?: boolean;
+  tooltip?: string;
 }

 export const Tab = React.forwardRef<HTMLElement, TabProps>(
-  ({ label, active, icon, onChangeTab, counter, suffix: Suffix, className, href, truncate, ...otherProps }, ref) => {
+  (
+    { label, active, icon, onChangeTab, counter, suffix: Suffix, className, href, truncate, tooltip, ...otherProps },
+    ref
+  ) => {
    const tabsStyles = useStyles2(getStyles);
    const clearStyles = useStyles2(clearButtonStyles);

@@ -55,10 +60,13 @@ export const Tab = React.forwardRef<HTMLElement, TabProps>(
      onClick: onChangeTab,
      role: 'tab',
      'aria-selected': active,
+      title: !!tooltip ? undefined : otherProps.title, // If tooltip is provided, don't set the title on the link or button, it looks weird
    };

+    let tab = null;
+
    if (href) {
-      return (
+      tab = (
        <div className={cx(tabsStyles.item, truncate && tabsStyles.itemTruncate, className)}>
          <a
            {...commonProps}
@@ -71,21 +79,27 @@ export const Tab = React.forwardRef<HTMLElement, TabProps>(
          </a>
        </div>
      );
+    } else {
+      tab = (
+        <div className={cx(tabsStyles.item, truncate && tabsStyles.itemTruncate, className)}>
+          <button
+            {...commonProps}
+            type="button"
+            // don't think we can avoid the type assertion here :(
+            // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
+            ref={ref as React.ForwardedRef<HTMLButtonElement>}
+          >
+            {content()}
+          </button>
+        </div>
+      );
    }

-    return (
-      <div className={cx(tabsStyles.item, truncate && tabsStyles.itemTruncate, className)}>
-        <button
-          {...commonProps}
-          type="button"
-          // don't think we can avoid the type assertion here :(
-          // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
-          ref={ref as React.ForwardedRef<HTMLButtonElement>}
-        >
-          {content()}
-        </button>
-      </div>
-    );
+    if (tooltip) {
+      return <Tooltip content={tooltip}>{tab}</Tooltip>;
+    }
+
+    return tab;
  }
 );

@@ -210,4 +210,10 @@ export class RowItem
  public setCollapsedState(collapse: boolean) {
    this.setState({ collapse });
  }
+
+  public hasUniqueTitle(): boolean {
+    const parentLayout = this.getParentLayout();
+    const duplicateTitles = parentLayout.duplicateTitles();
+    return !duplicateTitles.has(this.state.title);
+  }
 }
@@ -1,7 +1,7 @@
 import { useMemo } from 'react';

 import { selectors } from '@grafana/e2e-selectors';
-import { Alert, Input, Switch, TextLink } from '@grafana/ui';
+import { Alert, Input, Switch, TextLink, Field } from '@grafana/ui';
 import { t, Trans } from 'app/core/internationalization';
 import { OptionsPaneCategoryDescriptor } from 'app/features/dashboard/components/PanelEditor/OptionsPaneCategoryDescriptor';
 import { OptionsPaneItemDescriptor } from 'app/features/dashboard/components/PanelEditor/OptionsPaneItemDescriptor';
@@ -81,14 +81,22 @@ export function getEditOptions(model: RowItem): OptionsPaneCategoryDescriptor[]
 function RowTitleInput({ row }: { row: RowItem }) {
  const { title, isNew } = row.useState();
  const ref = useEditPaneInputAutoFocus({ autoFocus: isNew });
+  const hasUniqueTitle = row.hasUniqueTitle();

  return (
-    <Input
-      ref={ref}
-      title={t('dashboard.rows-layout.row-options.title-option', 'Title')}
-      value={title}
-      onChange={(e) => row.onChangeTitle(e.currentTarget.value)}
-    />
+    <Field
+      invalid={!hasUniqueTitle}
+      error={
+        !hasUniqueTitle ? t('dashboard.rows-layout.row-options.title-not-unique', 'Title should be unique') : undefined
+      }
+    >
+      <Input
+        ref={ref}
+        title={t('dashboard.rows-layout.row-options.title-option', 'Title')}
+        value={title}
+        onChange={(e) => row.onChangeTitle(e.currentTarget.value)}
+      />
+    </Field>
  );
 }

@@ -114,6 +114,13 @@ export function RowItemRenderer({ model }: SceneComponentProps<RowItem>) {
                  )}
                  role="heading"
                >
+                  {!model.hasUniqueTitle() && (
+                    <Tooltip
+                      content={t('dashboard.rows-layout.row-warning.title-not-unique', 'This title is not unique')}
+                    >
+                      <Icon name="exclamation-triangle" />
+                    </Tooltip>
+                  )}
                  {title}
                  {isHeaderHidden && (
                    <Tooltip
@@ -237,4 +237,20 @@ export class RowsLayoutManager extends SceneObjectBase<RowsLayoutManagerState> i

    return new RowsLayoutManager({ rows });
  }
+
+  public duplicateTitles(): Set<string | undefined> {
+    const titleCounts = new Map<string | undefined, number>();
+    const duplicateTitles = new Set<string | undefined>();
+
+    this.state.rows.forEach((row) => {
+      const title = row.state.title;
+      const count = (titleCounts.get(title) ?? 0) + 1;
+      titleCounts.set(title, count);
+      if (count > 1 && title) {
+        duplicateTitles.add(title);
+      }
+    });
+
+    return duplicateTitles;
+  }
 }
@@ -180,4 +180,10 @@ export class TabItem

    scrollCanvasElementIntoView(this, this.containerRef);
  }
+
+  public hasUniqueTitle(): boolean {
+    const parentLayout = this.getParentLayout();
+    const duplicateTitles = parentLayout.duplicateTitles();
+    return !duplicateTitles.has(this.state.title);
+  }
 }
@@ -1,6 +1,6 @@
 import { useMemo } from 'react';

-import { Input } from '@grafana/ui';
+import { Input, Field } from '@grafana/ui';
 import { t } from 'app/core/internationalization';
 import { OptionsPaneCategoryDescriptor } from 'app/features/dashboard/components/PanelEditor/OptionsPaneCategoryDescriptor';
 import { OptionsPaneItemDescriptor } from 'app/features/dashboard/components/PanelEditor/OptionsPaneItemDescriptor';
@@ -44,6 +44,21 @@ export function getEditOptions(model: TabItem): OptionsPaneCategoryDescriptor[]
 function TabTitleInput({ tab }: { tab: TabItem }) {
  const { title, isNew } = tab.useState();
  const ref = useEditPaneInputAutoFocus({ autoFocus: isNew });
+  const hasUniqueTitle = tab.hasUniqueTitle();

-  return <Input ref={ref} value={title} onChange={(e) => tab.onChangeTitle(e.currentTarget.value)} />;
+  return (
+    <Field
+      invalid={!hasUniqueTitle}
+      error={
+        !hasUniqueTitle ? t('dashboard.tabs-layout.tab-options.title-not-unique', 'Title should be unique') : undefined
+      }
+    >
+      <Input
+        ref={ref}
+        title={t('dashboard.tabs-layout.tab-options.title-option', 'Title')}
+        value={title}
+        onChange={(e) => tab.onChangeTitle(e.currentTarget.value)}
+      />
+    </Field>
+  );
 }
@@ -5,6 +5,7 @@ import { useLocation } from 'react-router';
 import { locationUtil, textUtil } from '@grafana/data';
 import { SceneComponentProps, sceneGraph } from '@grafana/scenes';
 import { Tab, useElementSelection, usePointerDistance, useStyles2 } from '@grafana/ui';
+import { t } from 'app/core/internationalization';

 import { useIsConditionallyHidden } from '../../conditional-rendering/useIsConditionallyHidden';
 import { useDashboardState } from '../../utils/utils';
@@ -32,6 +33,15 @@ export function TabItemRenderer({ model }: SceneComponentProps<TabItem>) {
    return null;
  }

+  let titleCollisionProps = {};
+
+  if (!model.hasUniqueTitle()) {
+    titleCollisionProps = {
+      icon: 'exclamation-triangle',
+      tooltip: t('dashboard.tabs-layout.tab-warning.title-not-unique', 'This title is not unique'),
+    };
+  }
+
  return (
    <Draggable key={key!} draggableId={key!} index={myIndex} isDragDisabled={!isEditing}>
      {(dragProvided, dragSnapshot) => (
@@ -70,6 +80,7 @@ export function TabItemRenderer({ model }: SceneComponentProps<TabItem>) {
            }}
            label={titleInterpolated}
            data-dashboard-drop-target-key={model.state.key}
+            {...titleCollisionProps}
          />
        </div>
      )}
@@ -233,4 +233,20 @@ export class TabsLayoutManager extends SceneObjectBase<TabsLayoutManagerState> i

    return key;
  }
+
+  public duplicateTitles() {
+    const titleCounts = new Map<string | undefined, number>();
+    const duplicateTitles = new Set<string | undefined>();
+
+    this.state.tabs.forEach((tab) => {
+      const title = tab.state.title;
+      const count = (titleCounts.get(title) ?? 0) + 1;
+      titleCounts.set(title, count);
+      if (count > 1) {
+        duplicateTitles.add(title);
+      }
+    });
+
+    return duplicateTitles;
+  }
 }
@@ -1821,7 +1821,11 @@
          "hide-header": "Hide row header",
          "title": "Title"
        },
+        "title-not-unique": "Title should be unique",
        "title-option": "Title"
+      },
+      "row-warning": {
+        "title-not-unique": "This title is not unique"
      }
    },
    "save-dashboard-as-button": {
@@ -1872,7 +1876,11 @@
        "new": "New tab"
      },
      "tab-options": {
+        "title-not-unique": "Title should be unique",
        "title-option": "Title"
+      },
+      "tab-warning": {
+        "title-not-unique": "This title is not unique"
      }
    },
    "toolbar": {