From 5355131aed4ab87e310548c6436cf2db52741948 Mon Sep 17 00:00:00 2001
From: Ashley Harrison <ashley.harrison@grafana.com>
Date: Thu, 23 Nov 2023 10:51:07 +0000
Subject: [PATCH] Tab: Only make `Tab` an anchor if a `href` is passed (#78540)

* initial work

* only make the tab an anchor if it has a href

* move things around for smaller diff

* use content()

* eslint-disable the type assertions

* extract props into common object and add missing return statement
---
 .../grafana-ui/src/components/Tabs/Tab.tsx    | 58 +++++++++++++------
 packages/grafana-ui/src/components/index.ts   |  2 +-
 .../alerting/unified/PanelAlertTab.tsx        |  2 +-
 3 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/packages/grafana-ui/src/components/Tabs/Tab.tsx b/packages/grafana-ui/src/components/Tabs/Tab.tsx
index 01ae75bb5c4..86ac3adfb36 100644
--- a/packages/grafana-ui/src/components/Tabs/Tab.tsx
+++ b/packages/grafana-ui/src/components/Tabs/Tab.tsx
@@ -7,26 +7,29 @@ import { selectors } from '@grafana/e2e-selectors';
 import { useStyles2 } from '../../themes';
 import { getFocusStyles } from '../../themes/mixins';
 import { IconName } from '../../types';
+import { clearButtonStyles } from '../Button';
 import { Icon } from '../Icon/Icon';
 
 import { Counter } from './Counter';
 
-export interface TabProps extends HTMLProps<HTMLAnchorElement> {
+export interface TabProps extends HTMLProps<HTMLElement> {
   label: string;
   active?: boolean;
   /** When provided, it is possible to use the tab as a hyperlink. Use in cases where the tabs update location. */
   href?: string;
   icon?: IconName;
-  onChangeTab?: (event?: React.MouseEvent<HTMLAnchorElement>) => void;
+  onChangeTab?: (event: React.MouseEvent<HTMLElement>) => void;
   /** A number rendered next to the text. Usually used to display the number of items in a tab's view. */
   counter?: number | null;
   /** Extra content, displayed after the tab label and counter */
   suffix?: NavModelItem['tabSuffix'];
 }
 
-export const Tab = React.forwardRef<HTMLAnchorElement, TabProps>(
+export const Tab = React.forwardRef<HTMLElement, TabProps>(
   ({ label, active, icon, onChangeTab, counter, suffix: Suffix, className, href, ...otherProps }, ref) => {
     const tabsStyles = useStyles2(getStyles);
+    const clearStyles = useStyles2(clearButtonStyles);
+
     const content = () => (
       <>
         {icon && <Icon name={icon} />}
@@ -36,23 +39,44 @@ export const Tab = React.forwardRef<HTMLAnchorElement, TabProps>(
       </>
     );
 
-    const linkClass = cx(tabsStyles.link, active ? tabsStyles.activeStyle : tabsStyles.notActive);
+    const linkClass = cx(clearStyles, tabsStyles.link, active ? tabsStyles.activeStyle : tabsStyles.notActive);
+
+    const commonProps = {
+      className: linkClass,
+      ...otherProps,
+      onClick: onChangeTab,
+      'aria-label': otherProps['aria-label'] || selectors.components.Tab.title(label),
+      role: 'tab',
+      'aria-selected': active,
+    };
+
+    if (href) {
+      return (
+        <div className={tabsStyles.item}>
+          <a
+            {...commonProps}
+            href={href}
+            // don't think we can avoid the type assertion here :(
+            // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
+            ref={ref as React.ForwardedRef<HTMLAnchorElement>}
+          >
+            {content()}
+          </a>
+        </div>
+      );
+    }
 
     return (
       <div className={tabsStyles.item}>
-        <a
-          // in case there is no href '#' is set in order to maintain a11y
-          href={href ? href : '#'}
-          className={linkClass}
-          {...otherProps}
-          onClick={onChangeTab}
-          aria-label={otherProps['aria-label'] || selectors.components.Tab.title(label)}
-          role="tab"
-          aria-selected={active}
-          ref={ref}
+        <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()}
-        </a>
+        </button>
       </div>
     );
   }
@@ -108,10 +132,6 @@ const getStyles = (theme: GrafanaTheme2) => {
       color: theme.colors.text.primary,
       overflow: 'hidden',
 
-      a: {
-        color: theme.colors.text.primary,
-      },
-
       '&::before': {
         backgroundImage: theme.colors.gradients.brandHorizontal,
       },
diff --git a/packages/grafana-ui/src/components/index.ts b/packages/grafana-ui/src/components/index.ts
index ff0df00871d..f05bc49fd79 100644
--- a/packages/grafana-ui/src/components/index.ts
+++ b/packages/grafana-ui/src/components/index.ts
@@ -100,7 +100,7 @@ export {
 } from './Table/types';
 export { TableInputCSV } from './TableInputCSV/TableInputCSV';
 export { TabsBar } from './Tabs/TabsBar';
-export { Tab } from './Tabs/Tab';
+export { Tab, type TabProps } from './Tabs/Tab';
 export { VerticalTab } from './Tabs/VerticalTab';
 export { TabContent } from './Tabs/TabContent';
 export { Counter } from './Tabs/Counter';
diff --git a/public/app/features/alerting/unified/PanelAlertTab.tsx b/public/app/features/alerting/unified/PanelAlertTab.tsx
index 88e6a763f2c..6289d88e69c 100644
--- a/public/app/features/alerting/unified/PanelAlertTab.tsx
+++ b/public/app/features/alerting/unified/PanelAlertTab.tsx
@@ -1,6 +1,6 @@
 import React from 'react';
 
-import { Tab, TabProps } from '@grafana/ui/src/components/Tabs/Tab';
+import { Tab, TabProps } from '@grafana/ui';
 import { DashboardModel, PanelModel } from 'app/features/dashboard/state';
 
 import { usePanelCombinedRules } from './hooks/usePanelCombinedRules';