[release-12.2.1] Dashboards: Fix missing Ctrl+O keyboard shortcut for crosshair toggle (#111402)

Dashboards: Fix missing Ctrl+O keyboard shortcut for crosshair toggle (#111310)

* Dashboard Scenes: Fix missing Ctrl+O keyboard shortcut for crosshair toggle

- Add missing mod+o keybind to dashboard scenes keyboard shortcuts
- Implement crosshair state cycling (Default -> Crosshair -> Tooltip -> Default)
- Add comprehensive unit tests for keyboard shortcuts functionality
- Add e2e test to verify shortcut works and prevents browser file dialog
- Fix ensures parity between legacy and scenes dashboard implementations

Fixes issue where Ctrl+O/Cmd+O was opening browser file dialog instead of
toggling shared crosshair modes in scenes-based dashboards.

* Remove waitForTimeout from e2e test

- Replace arbitrary timeouts with proper element waiting
- Use waitFor with visible state instead of setTimeout
- Improve test reliability and follow Playwright best practices

* Optimize e2e test for crosshair keyboard shortcut

- Remove unnecessary console logging and timeout settings
- Simplify assertions to only check the currently selected radio button
- Improve test performance by reducing DOM queries
- Focus on essential functionality verification

* Fix linting

(cherry picked from commit c0ce4ff1f2)

Co-authored-by: Ivan Ortega Alba <ivanortegaalba@gmail.com>

e2e-playwright/various-suite/keybinds.spec.ts

@@ -85,5 +85,63 @@ test.describe(
       expectedRange = 'Time range selected: 2024-06-05 10:04:00 to 2024-06-05 10:05:00'; // 1 min back
       await expect(timePickerButton).toHaveAttribute('aria-label', expectedRange);
     });
+
+    test('ctrl+o should toggle shared crosshair', async ({ page, selectors }) => {
+      // Navigate to a new dashboard
+      await page.goto('/dashboard/new?orgId=1');
+
+      // Wait for dashboard to load
+      await page.waitForLoadState('networkidle');
+
+      // Wait for dashboard to be fully initialized by checking for dashboard content
+      await page
+        .locator('[data-testid*="dashboard"]')
+        .or(page.locator('text=Start your new dashboard'))
+        .first()
+        .waitFor({ state: 'visible' });
+
+      // Test the keyboard shortcut first in the main dashboard view
+      const currentUrl = page.url();
+      const modKey = process.platform === 'darwin' ? 'Meta' : 'Control';
+
+      // Test that mod+o works in the main dashboard (should not trigger file dialog)
+      console.log('Testing mod+o in main dashboard view...');
+      await page.keyboard.press(`${modKey}+o`);
+      expect(page.url()).toBe(currentUrl); // Should not navigate away
+
+      // Now open settings to check if the state actually changed
+      await page.keyboard.press('d');
+      await page.keyboard.press('s');
+
+      // Wait for settings page to load by checking for the General tab or settings content
+      await page
+        .locator('text=General')
+        .or(page.locator('[data-testid*="dashboard-settings"]'))
+        .waitFor({ state: 'visible' });
+
+      // Wait for Panel options section to be visible and scroll to it
+      const panelOptionsSection = page.locator('text=Panel options');
+      await panelOptionsSection.waitFor({ state: 'visible' });
+      await panelOptionsSection.scrollIntoViewIfNeeded();
+
+      // Wait for radio buttons to be visible
+      await page
+        .locator('[role="radiogroup"]')
+        .last()
+        .locator('input[type="radio"]')
+        .first()
+        .waitFor({ state: 'visible' });
+
+      // Check current state - after one mod+o press, it should be crosshair (1)
+      await expect(page.locator('[role="radiogroup"]').last().locator('input[type="radio"]').nth(1)).toBeChecked(); // Shared crosshair
+
+      // Test second press in the main dashboard view (should go to tooltip)
+      await page.keyboard.press(`${modKey}+o`);
+      await expect(page.locator('[role="radiogroup"]').last().locator('input[type="radio"]').nth(2)).toBeChecked(); // Shared tooltip
+
+      // Test third press in the main dashboard view (should go back to default)
+      await page.keyboard.press(`${modKey}+o`);
+      await expect(page.locator('[role="radiogroup"]').last().locator('input[type="radio"]').nth(0)).toBeChecked(); // Default
+    });
   }
 );

go.work.sum

@@ -996,6 +996,7 @@ github.com/grafana/cog v0.0.38/go.mod h1:UDstzYqMdgIROmbfkHL8fB9XWQO2lnf5z+4W/eJ
 github.com/grafana/go-gelf/v2 v2.0.1 h1:BOChP0h/jLeD+7F9mL7tq10xVkDG15he3T1zHuQaWak=
 github.com/grafana/go-gelf/v2 v2.0.1/go.mod h1:lexHie0xzYGwCgiRGcvZ723bSNyNI8ZRD4s0CLobh90=
 github.com/grafana/gomemcache v0.0.0-20250228145437-da7b95fd2ac1/go.mod h1:j/s0jkda4UXTemDs7Pgw/vMT06alWc42CHisvYac0qw=
+github.com/grafana/gomemcache v0.0.0-20250828162811-a96f6acee2fe/go.mod h1:j/s0jkda4UXTemDs7Pgw/vMT06alWc42CHisvYac0qw=
 github.com/grafana/grafana-app-sdk v0.40.1/go.mod h1:4P8h7VB6KcDjX9bAoBQc6IP8iNylxe6bSXLR9gA39gM=
 github.com/grafana/grafana-app-sdk v0.41.0 h1:SYHN3U7B1myRKY3UZZDkFsue9TDmAOap0UrQVTqtYBU=
 github.com/grafana/grafana-app-sdk v0.41.0/go.mod h1:Wg/3vEZfok1hhIWiHaaJm+FwkosfO98o8KbeLFEnZpY=

public/app/features/dashboard-scene/scene/keyboardShortcuts.test.ts

@@ -0,0 +1,254 @@
+import { LegacyGraphHoverClearEvent } from '@grafana/data';
+import { behaviors, sceneGraph, SceneTimeRange } from '@grafana/scenes';
+import { DashboardCursorSync } from '@grafana/schema';
+import appEvents from 'app/core/app_events';
+import { KeybindingSet } from 'app/core/services/KeybindingSet';
+
+import { DashboardScene } from './DashboardScene';
+import { setupKeyboardShortcuts } from './keyboardShortcuts';
+
+// Mock dependencies
+jest.mock('app/core/app_events', () => ({
+  subscribe: jest.fn(() => ({ unsubscribe: jest.fn() })),
+  publish: jest.fn(),
+}));
+jest.mock('app/core/services/KeybindingSet');
+const mockOnRefresh = jest.fn();
+jest.mock('@grafana/scenes', () => ({
+  ...jest.requireActual('@grafana/scenes'),
+  sceneGraph: {
+    getTimeRange: jest.fn(() => ({
+      onRefresh: mockOnRefresh,
+    })),
+  },
+}));
+
+describe('setupKeyboardShortcuts', () => {
+  let mockScene: DashboardScene;
+  let mockKeybindingSet: jest.Mocked<KeybindingSet>;
+  let mockCursorSync: behaviors.CursorSync;
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    mockOnRefresh.mockClear();
+
+    // Mock KeybindingSet
+    mockKeybindingSet = jest.mocked(new KeybindingSet());
+    jest.spyOn(mockKeybindingSet, 'addBinding').mockImplementation();
+    jest.spyOn(mockKeybindingSet, 'removeAll').mockImplementation();
+    (KeybindingSet as jest.Mock).mockImplementation(() => mockKeybindingSet);
+
+    // Create mock CursorSync behavior
+    mockCursorSync = new behaviors.CursorSync({ sync: DashboardCursorSync.Off });
+    jest.spyOn(mockCursorSync, 'setState');
+
+    // Create mock DashboardScene
+    mockScene = new DashboardScene({
+      title: 'Test Dashboard',
+      uid: 'test-uid',
+      $timeRange: new SceneTimeRange({ from: 'now-6h', to: 'now' }),
+      $behaviors: [mockCursorSync],
+    });
+
+    // Mock canEditDashboard
+    jest.spyOn(mockScene, 'canEditDashboard').mockReturnValue(true);
+  });
+
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+
+  it('should setup keyboard shortcuts and return cleanup function', () => {
+    const cleanup = setupKeyboardShortcuts(mockScene);
+
+    expect(KeybindingSet).toHaveBeenCalled();
+    expect(mockKeybindingSet.addBinding).toHaveBeenCalled();
+    expect(typeof cleanup).toBe('function');
+
+    // Call cleanup function
+    cleanup();
+    expect(mockKeybindingSet.removeAll).toHaveBeenCalled();
+  });
+
+  describe('mod+o shortcut (toggle shared crosshair)', () => {
+    let modOHandler: () => void;
+
+    beforeEach(() => {
+      setupKeyboardShortcuts(mockScene);
+
+      // Find the mod+o binding
+      const modOBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 'mod+o');
+      expect(modOBinding).toBeDefined();
+      modOHandler = modOBinding![0].onTrigger;
+    });
+
+    it('should toggle cursor sync from Off to Crosshair', () => {
+      // Initial state: Off (0)
+      mockCursorSync.setState({ sync: DashboardCursorSync.Off });
+
+      modOHandler();
+
+      expect(mockCursorSync.setState).toHaveBeenCalledWith({ sync: DashboardCursorSync.Crosshair });
+      expect(appEvents.publish).toHaveBeenCalledWith(expect.any(LegacyGraphHoverClearEvent));
+      expect(sceneGraph.getTimeRange).toHaveBeenCalledWith(mockScene);
+      expect(mockOnRefresh).toHaveBeenCalled();
+    });
+
+    it('should toggle cursor sync from Crosshair to Tooltip', () => {
+      // Initial state: Crosshair (1)
+      mockCursorSync.setState({ sync: DashboardCursorSync.Crosshair });
+      jest.clearAllMocks();
+
+      modOHandler();
+
+      expect(mockCursorSync.setState).toHaveBeenCalledWith({ sync: DashboardCursorSync.Tooltip });
+      expect(appEvents.publish).toHaveBeenCalledWith(expect.any(LegacyGraphHoverClearEvent));
+      expect(sceneGraph.getTimeRange).toHaveBeenCalledWith(mockScene);
+      expect(mockOnRefresh).toHaveBeenCalled();
+    });
+
+    it('should toggle cursor sync from Tooltip to Off', () => {
+      // Initial state: Tooltip (2)
+      mockCursorSync.setState({ sync: DashboardCursorSync.Tooltip });
+      jest.clearAllMocks();
+
+      modOHandler();
+
+      expect(mockCursorSync.setState).toHaveBeenCalledWith({ sync: DashboardCursorSync.Off });
+      expect(appEvents.publish).toHaveBeenCalledWith(expect.any(LegacyGraphHoverClearEvent));
+      expect(sceneGraph.getTimeRange).toHaveBeenCalledWith(mockScene);
+      expect(mockOnRefresh).toHaveBeenCalled();
+    });
+
+    it('should handle missing CursorSync behavior gracefully', () => {
+      // Create scene without CursorSync behavior by overriding state
+      const sceneWithoutCursorSync = new DashboardScene({
+        title: 'Test Dashboard',
+        uid: 'test-uid',
+        $timeRange: new SceneTimeRange({ from: 'now-6h', to: 'now' }),
+      });
+
+      // Force remove CursorSync behavior
+      sceneWithoutCursorSync.setState({ $behaviors: [] });
+      jest.clearAllMocks();
+
+      setupKeyboardShortcuts(sceneWithoutCursorSync);
+
+      const modOBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 'mod+o');
+      const handler = modOBinding![0].onTrigger;
+
+      // Should not throw error when CursorSync is missing
+      expect(() => handler()).not.toThrow();
+      // When CursorSync is missing, no state changes or refresh should happen
+      expect(sceneGraph.getTimeRange).not.toHaveBeenCalled();
+    });
+
+    it('should handle non-CursorSync behavior gracefully', () => {
+      // Create scene with different behavior type
+      const sceneWithOtherBehavior = new DashboardScene({
+        title: 'Test Dashboard',
+        uid: 'test-uid',
+        $timeRange: new SceneTimeRange({ from: 'now-6h', to: 'now' }),
+      });
+
+      // Force set only non-CursorSync behavior
+      sceneWithOtherBehavior.setState({ $behaviors: [new behaviors.LiveNowTimer({ enabled: false })] });
+      jest.clearAllMocks();
+
+      setupKeyboardShortcuts(sceneWithOtherBehavior);
+
+      const modOBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 'mod+o');
+      const handler = modOBinding![0].onTrigger;
+
+      // Should not throw error when CursorSync is not found
+      expect(() => handler()).not.toThrow();
+      // When CursorSync is not found, no state changes or refresh should happen
+      expect(sceneGraph.getTimeRange).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('other keyboard shortcuts', () => {
+    beforeEach(() => {
+      setupKeyboardShortcuts(mockScene);
+    });
+
+    it('should setup view panel shortcut (v)', () => {
+      const vBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 'v');
+      expect(vBinding).toBeDefined();
+    });
+
+    it('should setup refresh shortcut (d r)', () => {
+      const drBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 'd r');
+      expect(drBinding).toBeDefined();
+    });
+
+    it('should setup zoom out shortcut (t z)', () => {
+      const tzBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 't z');
+      expect(tzBinding).toBeDefined();
+    });
+
+    it('should setup zoom out shortcut (ctrl+z)', () => {
+      const ctrlZBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 'ctrl+z');
+      expect(ctrlZBinding).toBeDefined();
+    });
+
+    it('should setup time range shortcuts', () => {
+      const taBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 't a');
+      const tLeftBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 't left');
+      const tRightBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 't right');
+
+      expect(taBinding).toBeDefined();
+      expect(tLeftBinding).toBeDefined();
+      expect(tRightBinding).toBeDefined();
+    });
+  });
+
+  describe('edit mode shortcuts', () => {
+    beforeEach(() => {
+      jest.spyOn(mockScene, 'canEditDashboard').mockReturnValue(true);
+      setupKeyboardShortcuts(mockScene);
+    });
+
+    it('should setup edit panel shortcut (e) when can edit', () => {
+      const eBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 'e');
+      expect(eBinding).toBeDefined();
+    });
+
+    it('should setup save shortcut (mod+s) when can edit', () => {
+      const modSBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 'mod+s');
+      expect(modSBinding).toBeDefined();
+    });
+
+    it('should setup dashboard settings shortcut (d s) when can edit', () => {
+      const dsBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 'd s');
+      expect(dsBinding).toBeDefined();
+    });
+  });
+
+  describe('non-edit mode', () => {
+    beforeEach(() => {
+      jest.spyOn(mockScene, 'canEditDashboard').mockReturnValue(false);
+      setupKeyboardShortcuts(mockScene);
+    });
+
+    it('should not setup edit-only shortcuts when cannot edit', () => {
+      const eBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 'e');
+      const modSBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 'mod+s');
+      const dsBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 'd s');
+
+      expect(eBinding).toBeUndefined();
+      expect(modSBinding).toBeUndefined();
+      expect(dsBinding).toBeUndefined();
+    });
+
+    it('should still setup non-edit shortcuts when cannot edit', () => {
+      const modOBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 'mod+o');
+      const vBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 'v');
+      const drBinding = mockKeybindingSet.addBinding.mock.calls.find((call) => call[0].key === 'd r');
+
+      expect(modOBinding).toBeDefined();
+      expect(vBinding).toBeDefined();
+      expect(drBinding).toBeDefined();
+    });
+  });
+});

public/app/features/dashboard-scene/scene/keyboardShortcuts.ts

@@ -1,6 +1,6 @@
-import { locationUtil, SetPanelAttentionEvent } from '@grafana/data';
+import { locationUtil, SetPanelAttentionEvent, LegacyGraphHoverClearEvent } from '@grafana/data';
 import { config, locationService } from '@grafana/runtime';
-import { sceneGraph, VizPanel } from '@grafana/scenes';
+import { behaviors, sceneGraph, VizPanel } from '@grafana/scenes';
 import appEvents from 'app/core/app_events';
 import { KeybindingSet } from 'app/core/services/KeybindingSet';
 import { contextSrv } from 'app/core/services/context_srv';
@@ -178,6 +178,20 @@ export function setupKeyboardShortcuts(scene: DashboardScene) {
     },
   });
 
+  keybindings.addBinding({
+    key: 'mod+o',
+    onTrigger: () => {
+      const cursorSync = scene.state.$behaviors?.find((b) => b instanceof behaviors.CursorSync);
+      if (cursorSync instanceof behaviors.CursorSync) {
+        const currentSync = cursorSync.state.sync;
+        const nextSync = (currentSync + 1) % 3;
+        cursorSync.setState({ sync: nextSync });
+        appEvents.publish(new LegacyGraphHoverClearEvent());
+        sceneGraph.getTimeRange(scene).onRefresh();
+      }
+    },
+  });
+
   if (canEdit) {
     // Panel edit
     keybindings.addBinding({