From 18673e6eefe73250034bcde887921f2da3de6a8a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Torkel=20=C3=96degaard?= <torkel@grafana.com>
Date: Thu, 11 Sep 2025 14:51:53 +0200
Subject: [PATCH] Variables: Style tweaks to new variables menu (#110946)

---
 .../scene/DashboardControls.tsx               |  4 +-
 .../scene/DashboardControlsMenu.test.tsx      | 12 +--
 .../scene/DashboardControlsMenu.tsx           | 76 ++++++++++++-------
 .../scene/DropdownVariableControls.tsx        | 68 +++++++++++++++++
 .../scene/VariableControls.tsx                | 37 ++++++++-
 5 files changed, 158 insertions(+), 39 deletions(-)
 create mode 100644 public/app/features/dashboard-scene/scene/DropdownVariableControls.tsx

diff --git a/public/app/features/dashboard-scene/scene/DashboardControls.tsx b/public/app/features/dashboard-scene/scene/DashboardControls.tsx
index 6d25737dda1..f1d5a83abe4 100644
--- a/public/app/features/dashboard-scene/scene/DashboardControls.tsx
+++ b/public/app/features/dashboard-scene/scene/DashboardControls.tsx
@@ -20,7 +20,7 @@ import { Box, Stack, useStyles2 } from '@grafana/ui';
 import { PanelEditControls } from '../panel-edit/PanelEditControls';
 import { getDashboardSceneFor } from '../utils/utils';
 
-import { DashboardControlsMenu } from './DashboardControlsMenu';
+import { DashboardControlsButton } from './DashboardControlsMenu';
 import { DashboardLinksControls } from './DashboardLinksControls';
 import { DashboardScene } from './DashboardScene';
 import { VariableControls } from './VariableControls';
@@ -159,7 +159,7 @@ function DashboardControlsRenderer({ model }: SceneComponentProps<DashboardContr
       )}
       {(hasControlMenuVariables || hasControlMenuLinks) && (
         <Stack>
-          <DashboardControlsMenu dashboard={dashboard} />
+          <DashboardControlsButton dashboard={dashboard} />
         </Stack>
       )}
       {showDebugger && <SceneDebugger scene={model} key={'scene-debugger'} />}
diff --git a/public/app/features/dashboard-scene/scene/DashboardControlsMenu.test.tsx b/public/app/features/dashboard-scene/scene/DashboardControlsMenu.test.tsx
index 9552b19e2b8..ebe47918827 100644
--- a/public/app/features/dashboard-scene/scene/DashboardControlsMenu.test.tsx
+++ b/public/app/features/dashboard-scene/scene/DashboardControlsMenu.test.tsx
@@ -6,13 +6,13 @@ import { SceneVariableSet, TextBoxVariable, QueryVariable, CustomVariable, Scene
 import {
   DASHBOARD_CONTROLS_MENU_ARIA_LABEL,
   DASHBOARD_CONTROLS_MENU_TITLE,
-  DashboardControlsMenu,
+  DashboardControlsButton,
 } from './DashboardControlsMenu';
 import { DashboardScene } from './DashboardScene';
 
 describe('DashboardControlsMenu', () => {
   it('should return null and not render anything when there are no variables', () => {
-    const { container } = render(<DashboardControlsMenu dashboard={getDashboard([])} />);
+    const { container } = render(<DashboardControlsButton dashboard={getDashboard([])} />);
     expect(container.firstChild).toBeNull();
   });
 
@@ -24,7 +24,7 @@ describe('DashboardControlsMenu', () => {
         showInControlsMenu: false,
       }),
     ];
-    const { container } = render(<DashboardControlsMenu dashboard={getDashboard(variables)} />);
+    const { container } = render(<DashboardControlsButton dashboard={getDashboard(variables)} />);
     expect(container.firstChild).toBeNull();
   });
 
@@ -37,7 +37,7 @@ describe('DashboardControlsMenu', () => {
       }),
     ];
 
-    render(<DashboardControlsMenu dashboard={getDashboard(variables)} />);
+    render(<DashboardControlsButton dashboard={getDashboard(variables)} />);
 
     // Should render the toolbar button
     const button = screen.getByRole('button');
@@ -66,7 +66,7 @@ describe('DashboardControlsMenu', () => {
     ];
 
     act(() => {
-      render(<DashboardControlsMenu dashboard={getDashboard(variables)} />);
+      render(<DashboardControlsButton dashboard={getDashboard(variables)} />);
     });
 
     // Should have rendered a dropdown
@@ -98,7 +98,7 @@ describe('DashboardControlsMenu', () => {
       }),
     ];
 
-    render(<DashboardControlsMenu dashboard={getDashboard(variables)} />);
+    render(<DashboardControlsButton dashboard={getDashboard(variables)} />);
 
     // Should still render dropdown since we have variables with showInControlsMenu=true
     expect(screen.getByRole('button')).toBeInTheDocument();
diff --git a/public/app/features/dashboard-scene/scene/DashboardControlsMenu.tsx b/public/app/features/dashboard-scene/scene/DashboardControlsMenu.tsx
index b3eba43cd8f..ecf54b057bf 100644
--- a/public/app/features/dashboard-scene/scene/DashboardControlsMenu.tsx
+++ b/public/app/features/dashboard-scene/scene/DashboardControlsMenu.tsx
@@ -2,8 +2,9 @@ import { css } from '@emotion/css';
 
 import { GrafanaTheme2 } from '@grafana/data';
 import { t } from '@grafana/i18n';
-import { sceneGraph } from '@grafana/scenes';
-import { Dropdown, Menu, ToolbarButton, useStyles2 } from '@grafana/ui';
+import { sceneGraph, SceneVariable } from '@grafana/scenes';
+import { DashboardLink } from '@grafana/schema';
+import { Box, Dropdown, ToolbarButton, useStyles2 } from '@grafana/ui';
 
 import { DashboardLinkRenderer } from './DashboardLinkRenderer';
 import { DashboardScene } from './DashboardScene';
@@ -12,8 +13,7 @@ import { VariableValueSelectWrapper } from './VariableControls';
 export const DASHBOARD_CONTROLS_MENU_ARIA_LABEL = 'Dashboard controls menu';
 export const DASHBOARD_CONTROLS_MENU_TITLE = 'Dashboard controls';
 
-export function DashboardControlsMenu({ dashboard }: { dashboard: DashboardScene }) {
-  const styles = useStyles2(getStyles);
+export function DashboardControlsButton({ dashboard }: { dashboard: DashboardScene }) {
   const { links, uid } = dashboard.useState();
   const filteredLinks = links.filter((link) => link.placement === 'inControlsMenu');
   const variables = sceneGraph
@@ -27,29 +27,8 @@ export function DashboardControlsMenu({ dashboard }: { dashboard: DashboardScene
 
   return (
     <Dropdown
-      overlay={
-        <Menu
-          onClick={(e) => {
-            e.stopPropagation();
-          }}
-        >
-          {/* Variables */}
-          {variables.map((variable) => (
-            <div className={styles.menuItem} key={variable.state.key}>
-              <VariableValueSelectWrapper variable={variable} />
-            </div>
-          ))}
-
-          {variables.length > 0 && filteredLinks.length > 0 && <Menu.Divider />}
-
-          {/* Links */}
-          {filteredLinks.map((link, index) => (
-            <div className={styles.menuItem} key={`${link.title}-$${index}`}>
-              <DashboardLinkRenderer link={link} dashboardUID={uid} />
-            </div>
-          ))}
-        </Menu>
-      }
+      placement="bottom-end"
+      overlay={<DashboardControlsMenu variables={variables} links={filteredLinks} dashboardUID={uid} />}
     >
       <ToolbarButton
         aria-label={t('dashboard.controls.menu.aria-label', DASHBOARD_CONTROLS_MENU_ARIA_LABEL)}
@@ -57,11 +36,54 @@ export function DashboardControlsMenu({ dashboard }: { dashboard: DashboardScene
         icon="ellipsis-v"
         iconSize="md"
         narrow
+        variant="canvas"
       />
     </Dropdown>
   );
 }
 
+interface VariablesMenuProps {
+  variables: SceneVariable[];
+  links: DashboardLink[];
+  dashboardUID: string;
+}
+
+function DashboardControlsMenu({ variables, links, dashboardUID }: VariablesMenuProps) {
+  const styles = useStyles2(getStyles);
+
+  return (
+    <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 */}
+      {variables.map((variable) => (
+        <div className={styles.menuItem} key={variable.state.key}>
+          <VariableValueSelectWrapper variable={variable} inMenu />
+        </div>
+      ))}
+
+      {/* Links */}
+      {links.map((link, index) => (
+        <div className={styles.menuItem} key={`${link.title}-$${index}`}>
+          <DashboardLinkRenderer link={link} dashboardUID={dashboardUID} />
+        </div>
+      ))}
+    </Box>
+  );
+}
+
 const getStyles = (theme: GrafanaTheme2) => ({
   menuItem: css({
     padding: theme.spacing(0.5),
diff --git a/public/app/features/dashboard-scene/scene/DropdownVariableControls.tsx b/public/app/features/dashboard-scene/scene/DropdownVariableControls.tsx
new file mode 100644
index 00000000000..b9a49aad6f9
--- /dev/null
+++ b/public/app/features/dashboard-scene/scene/DropdownVariableControls.tsx
@@ -0,0 +1,68 @@
+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),
+  }),
+});
diff --git a/public/app/features/dashboard-scene/scene/VariableControls.tsx b/public/app/features/dashboard-scene/scene/VariableControls.tsx
index cbe5fde1470..6b43cacd553 100644
--- a/public/app/features/dashboard-scene/scene/VariableControls.tsx
+++ b/public/app/features/dashboard-scene/scene/VariableControls.tsx
@@ -2,7 +2,14 @@ import { css, cx } from '@emotion/css';
 
 import { VariableHide, GrafanaTheme2 } from '@grafana/data';
 import { selectors } from '@grafana/e2e-selectors';
-import { sceneGraph, useSceneObjectState, SceneVariable, SceneVariableState, ControlsLabel } from '@grafana/scenes';
+import {
+  sceneGraph,
+  useSceneObjectState,
+  SceneVariable,
+  SceneVariableState,
+  ControlsLabel,
+  ControlsLayout,
+} from '@grafana/scenes';
 import { useElementSelection, useStyles2 } from '@grafana/ui';
 
 import { DashboardScene } from './DashboardScene';
@@ -23,9 +30,10 @@ export function VariableControls({ dashboard }: { dashboard: DashboardScene }) {
 
 interface VariableSelectProps {
   variable: SceneVariable;
+  inMenu?: boolean;
 }
 
-export function VariableValueSelectWrapper({ variable }: VariableSelectProps) {
+export function VariableValueSelectWrapper({ variable, inMenu }: VariableSelectProps) {
   const state = useSceneObjectState<SceneVariableState>(variable, { shouldActivateOrKeepAlive: true });
   const { isSelected, onSelect, isSelectable } = useElementSelection(variable.state.key);
   const styles = useStyles2(getStyles);
@@ -56,6 +64,15 @@ export function VariableValueSelectWrapper({ variable }: VariableSelectProps) {
     }
   };
 
+  if (inMenu) {
+    return (
+      <div className={styles.verticalContainer} data-testid={selectors.pages.Dashboard.SubMenu.submenuItem}>
+        <VariableLabel variable={variable} layout={'vertical'} />
+        <variable.Component model={variable} />
+      </div>
+    );
+  }
+
   return (
     <div
       className={cx(
@@ -72,7 +89,15 @@ export function VariableValueSelectWrapper({ variable }: VariableSelectProps) {
   );
 }
 
-function VariableLabel({ variable, className }: { variable: SceneVariable; className?: string }) {
+function VariableLabel({
+  variable,
+  className,
+  layout,
+}: {
+  variable: SceneVariable;
+  className?: string;
+  layout?: ControlsLayout;
+}) {
   const { state } = variable;
 
   if (variable.state.hide === VariableHide.hideLabel) {
@@ -89,7 +114,7 @@ function VariableLabel({ variable, className }: { variable: SceneVariable; class
       onCancel={() => variable.onCancel?.()}
       label={labelOrName}
       error={state.error}
-      layout={'horizontal'}
+      layout={layout ?? 'horizontal'}
       description={state.description ?? undefined}
       className={className}
     />
@@ -105,6 +130,10 @@ const getStyles = (theme: GrafanaTheme2) => ({
       borderBottomLeftRadius: 'unset',
     }),
   }),
+  verticalContainer: css({
+    display: 'flex',
+    flexDirection: 'column',
+  }),
   labelWrapper: css({
     display: 'flex',
     alignItems: 'center',