Dashboard Controls - Adjust styling for links (#110924)

* feat: adjust styling for the dashboard-controls menu

* refactor: remove unused file DashboardVariableControls.tsx

---------

Co-authored-by: Torkel Ödegaard <torkel@grafana.com>

public/app/features/dashboard-scene/scene/DashboardControlsMenu.tsx

@@ -1,10 +1,10 @@
-import { css } from '@emotion/css';
+import { css, cx } from '@emotion/css';
 
 import { GrafanaTheme2 } from '@grafana/data';
 import { t } from '@grafana/i18n';
 import { sceneGraph, SceneVariable } from '@grafana/scenes';
 import { DashboardLink } from '@grafana/schema';
-import { Box, Dropdown, ToolbarButton, useStyles2 } from '@grafana/ui';
+import { Box, Dropdown, Menu, ToolbarButton, useStyles2 } from '@grafana/ui';
 
 import { DashboardLinkRenderer } from './DashboardLinkRenderer';
 import { DashboardScene } from './DashboardScene';
@@ -15,7 +15,11 @@ export const DASHBOARD_CONTROLS_MENU_TITLE = 'Dashboard controls';
 
 export function DashboardControlsButton({ dashboard }: { dashboard: DashboardScene }) {
   const { links, uid } = dashboard.useState();
-  const filteredLinks = links.filter((link) => link.placement === 'inControlsMenu');
+  // Dashboard links are not supported at the moment.
+  // Reason: nesting <Dropdown> components causes issues since the inner dropdown is rendered in a portal,
+  // so clicking it closes the parent dropdown (the parent sees it as an overlay click, and the event cannot easily be intercepted,
+  // as it is in different HTML subtree).
+  const filteredLinks = links.filter((link) => link.placement === 'inControlsMenu' && link.type !== 'dashboards');
   const variables = sceneGraph
     .getVariables(dashboard)!
     .useState()
@@ -42,13 +46,13 @@ export function DashboardControlsButton({ dashboard }: { dashboard: DashboardSce
   );
 }
 
-interface VariablesMenuProps {
+interface DashboardControlsMenuProps {
   variables: SceneVariable[];
   links: DashboardLink[];
   dashboardUID: string;
 }
 
-function DashboardControlsMenu({ variables, links, dashboardUID }: VariablesMenuProps) {
+function DashboardControlsMenu({ variables, links, dashboardUID }: DashboardControlsMenuProps) {
   const styles = useStyles2(getStyles);
 
   return (
@@ -61,23 +65,31 @@ function DashboardControlsMenu({ variables, links, dashboardUID }: VariablesMenu
       direction={'column'}
       borderRadius={'default'}
       backgroundColor={'primary'}
-      padding={1}
+      padding={1.5}
       gap={0.5}
       onClick={(e) => {
+        // Normally, clicking the overlay closes the dropdown.
+        // We stop event propagation here to keep it open while users interact with variable controls.
         e.stopPropagation();
       }}
     >
       {/* Variables */}
-      {variables.map((variable) => (
-        <div className={styles.menuItem} key={variable.state.key}>
+      {variables.map((variable, index) => (
+        <div className={cx(index > 0 && styles.menuItem)} key={variable.state.key}>
           <VariableValueSelectWrapper variable={variable} inMenu />
         </div>
       ))}
 
+      {variables.length > 0 && links.length > 0 && (
+        <div className={styles.divider}>
+          <Menu.Divider />
+        </div>
+      )}
+
       {/* Links */}
       {links.map((link, index) => (
-        <div className={styles.menuItem} key={`${link.title}-$${index}`}>
-          <DashboardLinkRenderer link={link} dashboardUID={dashboardUID} />
+        <div key={`${link.title}-${index}`}>
+          <DashboardLinkRenderer link={link} dashboardUID={dashboardUID} inMenu />
         </div>
       ))}
     </Box>
@@ -85,7 +97,11 @@ function DashboardControlsMenu({ variables, links, dashboardUID }: VariablesMenu
 }
 
 const getStyles = (theme: GrafanaTheme2) => ({
+  divider: css({
+    marginTop: theme.spacing(1),
+    padding: theme.spacing(0, 0.5),
+  }),
   menuItem: css({
-    padding: theme.spacing(0.5),
+    marginTop: theme.spacing(2),
   }),
 });

public/app/features/dashboard-scene/scene/DashboardLinkRenderer.tsx

@@ -1,7 +1,7 @@
 import { sanitizeUrl } from '@grafana/data/internal';
 import { selectors } from '@grafana/e2e-selectors';
 import { DashboardLink } from '@grafana/schema';
-import { Tooltip } from '@grafana/ui';
+import { MenuItem, Tooltip } from '@grafana/ui';
 import {
   DashboardLinkButton,
   DashboardLinksDashboard,
@@ -13,9 +13,11 @@ import { LINK_ICON_MAP } from '../settings/links/utils';
 export interface Props {
   link: DashboardLink;
   dashboardUID: string;
+  // Set to `true` if displaying a link in a drop-down menu (e.g. dashboard controls)
+  inMenu?: boolean;
 }
 
-export function DashboardLinkRenderer({ link, dashboardUID }: Props) {
+export function DashboardLinkRenderer({ link, dashboardUID, inMenu }: Props) {
   const linkInfo = getLinkSrv().getAnchorInfo(link);
 
   if (link.type === 'dashboards') {
@@ -24,7 +26,15 @@ export function DashboardLinkRenderer({ link, dashboardUID }: Props) {
 
   const icon = LINK_ICON_MAP[link.icon];
 
-  const linkElement = (
+  const linkElement = inMenu ? (
+    <MenuItem
+      icon={icon}
+      url={sanitizeUrl(linkInfo.href)}
+      label={linkInfo.title}
+      target={link.targetBlank ? '_blank' : undefined}
+      data-testid={selectors.components.DashboardLinks.link}
+    />
+  ) : (
     <DashboardLinkButton
       icon={icon}
       href={sanitizeUrl(linkInfo.href)}

public/app/features/dashboard-scene/scene/DropdownVariableControls.tsx

@@ -1,68 +0,0 @@
-import { css } from '@emotion/css';
-
-import { GrafanaTheme2 } from '@grafana/data';
-import { t } from '@grafana/i18n';
-import { sceneGraph } from '@grafana/scenes';
-import { Box, Dropdown, ToolbarButton, useStyles2 } from '@grafana/ui';
-
-import { DashboardScene } from './DashboardScene';
-import { VariableValueSelectWrapper } from './VariableControls';
-
-export const DROPDOWN_CONTROLS_ARIA_LABEL = 'Dashboard controls menu';
-export const DROPDOWN_CONTROLS_TITLE = 'Dashboard controls';
-
-export function DropdownVariableControls({ dashboard }: { dashboard: DashboardScene }) {
-  const styles = useStyles2(getStyles);
-  const variables = sceneGraph
-    .getVariables(dashboard)!
-    .useState()
-    .variables.filter((v) => v.state.showInControlsMenu !== true);
-
-  if (variables.length === 0) {
-    return null;
-  }
-
-  return (
-    <Dropdown
-      placement="bottom-end"
-      overlay={
-        <Box
-          minWidth={32}
-          borderColor={'weak'}
-          borderStyle={'solid'}
-          boxShadow={'z3'}
-          display={'flex'}
-          direction={'column'}
-          borderRadius={'default'}
-          backgroundColor={'primary'}
-          padding={1}
-          gap={0.5}
-          onClick={(e) => {
-            e.stopPropagation();
-          }}
-        >
-          {variables.map((variable) => (
-            <div className={styles.menuItem} key={variable.state.key}>
-              <VariableValueSelectWrapper variable={variable} inMenu />
-            </div>
-          ))}
-        </Box>
-      }
-    >
-      <ToolbarButton
-        aria-label={t('dashboard.controls.menu.aria-label', DROPDOWN_CONTROLS_ARIA_LABEL)}
-        title={t('dashboard.controls.menu.title', DROPDOWN_CONTROLS_TITLE)}
-        icon="ellipsis-v"
-        iconSize="md"
-        narrow
-        variant="canvas"
-      />
-    </Dropdown>
-  );
-}
-
-const getStyles = (theme: GrafanaTheme2) => ({
-  menuItem: css({
-    padding: theme.spacing(0.5),
-  }),
-});