From 06304894a13ab9fbf3386eddcf5e870adc477989 Mon Sep 17 00:00:00 2001 From: Tobias Skarhed <1438972+tskarhed@users.noreply.github.com> Date: Wed, 29 May 2024 09:11:23 +0200 Subject: [PATCH] Dashboard: Keyboard and mouse panel shortcuts improvement (#87317) * Add custom attention state to dashboard * Add attention state to DashboardGrid * Remove old functionality * Add attention state to keybindingSrv * Create PanelAttentionService * Add PanelAttentionService with scenes support * Remove unused code * Use viz panel key instead of VizPanel * Add type assertion * Add e2e test * Update comments * Use panel id for non-scenes use case * Support undefined service use case * Memoize singleton call * Debounce mouseover * Set panelAttention with appEvents * Use AppEvents for Scenes * Remove panelAttentionSrv * Wait in e2e to handle debounce * Move subscription to KeybindingSrv * Remove imports and reset keyboardShortcuts from main * Fix on* event handlers --- .../dashboard-panel-attention.spec.ts | 30 +++++++++++++++++++ packages/grafana-data/src/events/common.ts | 4 +++ .../components/PanelChrome/PanelChrome.tsx | 14 ++++++++- public/app/core/services/keybindingSrv.ts | 23 +++++++++++--- .../app/core/services/withFocusedPanelId.ts | 12 -------- .../dashboard/dashgrid/PanelStateWrapper.tsx | 13 +++++++- 6 files changed, 78 insertions(+), 18 deletions(-) create mode 100644 e2e/dashboards-suite/dashboard-panel-attention.spec.ts delete mode 100644 public/app/core/services/withFocusedPanelId.ts diff --git a/e2e/dashboards-suite/dashboard-panel-attention.spec.ts b/e2e/dashboards-suite/dashboard-panel-attention.spec.ts new file mode 100644 index 00000000000..d35b4fd913f --- /dev/null +++ b/e2e/dashboards-suite/dashboard-panel-attention.spec.ts @@ -0,0 +1,30 @@ +import { e2e } from '../utils'; + +describe('Dashboard Panel Attention', () => { + beforeEach(() => { + e2e.flows.login(Cypress.env('USERNAME'), Cypress.env('PASSWORD')); + // Open all panels dashboard + e2e.flows.openDashboard({ uid: 'n1jR8vnnz' }); + }); + + it('Should give panel attention on focus', () => { + e2e.components.Panels.Panel.title('State timeline').focus(); + cy.get('body').type('v'); + cy.url().should('include', 'viewPanel=41'); + }); + + it('Should give panel attention on hover', () => { + e2e.components.Panels.Panel.title('State timeline').trigger('mousemove'); + cy.wait(100); // Wait because of debounce + cy.get('body').type('v'); + cy.url().should('include', 'viewPanel=41'); + }); + + it('Should change panel attention between focus and mousemove', () => { + e2e.components.Panels.Panel.title('Size, color mapped to different fields + share view').focus(); + e2e.components.Panels.Panel.title('State timeline').trigger('mousemove'); + cy.wait(100); // Wait because of debounce + cy.get('body').type('v'); + cy.url().should('include', 'viewPanel=41'); + }); +}); diff --git a/packages/grafana-data/src/events/common.ts b/packages/grafana-data/src/events/common.ts index 6b480a834f3..aa94afd843a 100644 --- a/packages/grafana-data/src/events/common.ts +++ b/packages/grafana-data/src/events/common.ts @@ -64,3 +64,7 @@ export class DataSourceTestSucceeded extends BusEventBase { export class DataSourceTestFailed extends BusEventBase { static type = 'datasource-test-failed'; } + +export class SetPanelAttentionEvent extends BusEventWithPayload<{ panelId: string | number }> { + static type = 'set-panel-attention'; +} diff --git a/packages/grafana-ui/src/components/PanelChrome/PanelChrome.tsx b/packages/grafana-ui/src/components/PanelChrome/PanelChrome.tsx index e1ae34984b8..ef72c19eca9 100644 --- a/packages/grafana-ui/src/components/PanelChrome/PanelChrome.tsx +++ b/packages/grafana-ui/src/components/PanelChrome/PanelChrome.tsx @@ -56,6 +56,14 @@ interface BaseProps { * callback when opening the panel menu */ onOpenMenu?: () => void; + /** + * Used for setting panel attention + */ + onFocus?: () => void; + /** + * Debounce the event handler, if possible + */ + onMouseMove?: () => void; } interface FixedDimensions extends BaseProps { @@ -121,6 +129,8 @@ export function PanelChrome({ collapsible = false, collapsed, onToggleCollapse, + onFocus, + onMouseMove, }: PanelChromeProps) { const theme = useTheme2(); const styles = useStyles2(getStyles); @@ -240,7 +250,9 @@ export function PanelChrome({ className={cx(styles.container, { [styles.transparentContainer]: isPanelTransparent })} style={containerStyles} data-testid={testid} - tabIndex={0} //eslint-disable-line jsx-a11y/no-noninteractive-tabindex + tabIndex={0} // eslint-disable-line jsx-a11y/no-noninteractive-tabindex + onFocus={onFocus} + onMouseMove={onMouseMove} ref={ref} >
diff --git a/public/app/core/services/keybindingSrv.ts b/public/app/core/services/keybindingSrv.ts index f2f0419ada0..66a58d5c4f8 100644 --- a/public/app/core/services/keybindingSrv.ts +++ b/public/app/core/services/keybindingSrv.ts @@ -2,7 +2,7 @@ import Mousetrap from 'mousetrap'; import 'mousetrap-global-bind'; import 'mousetrap/plugins/global-bind/mousetrap-global-bind'; -import { LegacyGraphHoverClearEvent, locationUtil } from '@grafana/data'; +import { LegacyGraphHoverClearEvent, SetPanelAttentionEvent, locationUtil } from '@grafana/data'; import { LocationService } from '@grafana/runtime'; import appEvents from 'app/core/app_events'; import { getExploreUrl } from 'app/core/utils/explore'; @@ -27,13 +27,19 @@ import { contextSrv } from '../core'; import { RouteDescriptor } from '../navigation/types'; import { toggleTheme } from './theme'; -import { withFocusedPanel } from './withFocusedPanelId'; export class KeybindingSrv { constructor( private locationService: LocationService, private chromeService: AppChromeService - ) {} + ) { + // No cleanup needed, since KeybindingSrv is a singleton + appEvents.subscribe(SetPanelAttentionEvent, (event) => { + this.panelId = event.payload.panelId; + }); + } + /** string for VizPanel key and number for panelId */ + private panelId: string | number | null = null; clearAndInitGlobalBindings(route: RouteDescriptor) { Mousetrap.reset(); @@ -182,7 +188,16 @@ export class KeybindingSrv { } bindWithPanelId(keyArg: string, fn: (panelId: number) => void) { - this.bind(keyArg, withFocusedPanel(fn)); + this.bind(keyArg, this.withFocusedPanel(fn)); + } + + withFocusedPanel(fn: (panelId: number) => void) { + return () => { + if (typeof this.panelId === 'number') { + fn(this.panelId); + return; + } + }; } setupTimeRangeBindings(updateUrl = true) { diff --git a/public/app/core/services/withFocusedPanelId.ts b/public/app/core/services/withFocusedPanelId.ts deleted file mode 100644 index 857144ee706..00000000000 --- a/public/app/core/services/withFocusedPanelId.ts +++ /dev/null @@ -1,12 +0,0 @@ -export function withFocusedPanel(fn: (panelId: number) => void) { - return () => { - const elements = document.querySelectorAll(':hover'); - - for (let i = elements.length - 1; i > 0; i--) { - const element = elements[i]; - if (element instanceof HTMLElement && element.dataset?.panelid) { - fn(parseInt(element.dataset?.panelid, 10)); - } - } - }; -} diff --git a/public/app/features/dashboard/dashgrid/PanelStateWrapper.tsx b/public/app/features/dashboard/dashgrid/PanelStateWrapper.tsx index 32cd4ba0fff..405cf80dd18 100644 --- a/public/app/features/dashboard/dashgrid/PanelStateWrapper.tsx +++ b/public/app/features/dashboard/dashgrid/PanelStateWrapper.tsx @@ -1,3 +1,4 @@ +import { debounce } from 'lodash'; import React, { PureComponent } from 'react'; import { Subscription } from 'rxjs'; @@ -16,6 +17,7 @@ import { PanelData, PanelPlugin, PanelPluginMeta, + SetPanelAttentionEvent, TimeRange, toDataFrameDTO, toUtc, @@ -30,6 +32,7 @@ import { SeriesVisibilityChangeMode, AdHocFilterItem, } from '@grafana/ui'; +import appEvents from 'app/core/app_events'; import config from 'app/core/config'; import { profiler } from 'app/core/profiler'; import { applyPanelTimeOverrides } from 'app/features/dashboard/utils/panel'; @@ -90,6 +93,7 @@ export class PanelStateWrapper extends PureComponent { // Can this eventBus be on PanelModel? when we have more complex event filtering, that may be a better option const eventBus = props.dashboard.events.newScopedBus(`panel:${props.panel.id}`, this.eventFilter); + this.debouncedSetPanelAttention = debounce(this.setPanelAttention.bind(this), 100); this.state = { isFirstLoad: true, @@ -544,11 +548,16 @@ export class PanelStateWrapper extends PureComponent { ); } + setPanelAttention() { + appEvents.publish(new SetPanelAttentionEvent({ panelId: this.props.panel.id })); + } + + debouncedSetPanelAttention() {} + render() { const { dashboard, panel, width, height, plugin } = this.props; const { errorMessage, data } = this.state; const { transparent } = panel; - const panelChromeProps = getPanelChromeProps({ ...this.props, data }); // Shift the hover menu down if it's on the top row so it doesn't get clipped by topnav @@ -579,6 +588,8 @@ export class PanelStateWrapper extends PureComponent { displayMode={transparent ? 'transparent' : 'default'} onCancelQuery={panelChromeProps.onCancelQuery} onOpenMenu={panelChromeProps.onOpenMenu} + onFocus={() => this.setPanelAttention()} + onMouseMove={() => this.debouncedSetPanelAttention()} > {(innerWidth, innerHeight) => ( <>