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
This commit is contained in:
Ivan Ortega Alba
2025-09-19 16:03:02 +00:00
committed by GitHub
parent 6203a6f3c5
commit c0ce4ff1f2
4 changed files with 329 additions and 2 deletions
@@ -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
});
}
);
+1
View File
@@ -1055,6 +1055,7 @@ github.com/grafana/dskit v0.0.0-20250818234656-8ff9c6532e85/go.mod h1:kImsvJ1xnm
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=
@@ -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();
});
});
});
@@ -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';
@@ -168,6 +168,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({