From 773da0e3305ec9b40c186ae0f51869e66144d06e Mon Sep 17 00:00:00 2001 From: Kristina Date: Thu, 17 Feb 2022 07:39:02 -0600 Subject: [PATCH] Explore: Add t * keybindings to change time range (#45020) * Add keybindings to explore, allow override of dashboard model update for explore update * Remove edits to Dashboard Model, add definition when url params should be updated * Add tests * Add and expose util function instead of bringing in unrelated library do not define explicit path to library file * Use more generic model for TimeSrv * Remove url utility functions, use javascript function instead * Break out TimeModel into new type and bring it in * condense object creation --- e2e/various-suite/explore.spec.ts | 22 +++++ public/app/core/services/keybindingSrv.ts | 36 ++++---- .../DashNav/DashNavTimeControls.tsx | 10 +-- .../dashboard/services/TimeSrv.test.ts | 5 ++ .../features/dashboard/services/TimeSrv.ts | 88 ++++++++++--------- .../dashboard/state/DashboardModel.ts | 3 +- .../app/features/dashboard/state/TimeModel.ts | 10 +++ .../app/features/dashboard/state/actions.ts | 4 +- .../app/features/explore/state/explorePane.ts | 3 + public/app/features/explore/state/time.ts | 19 ++-- public/app/plugins/panel/graph/module.ts | 2 +- .../app/plugins/panel/heatmap/heatmap_ctrl.ts | 2 +- public/app/types/events.ts | 15 +++- 13 files changed, 140 insertions(+), 79 deletions(-) create mode 100644 public/app/features/dashboard/state/TimeModel.ts diff --git a/e2e/various-suite/explore.spec.ts b/e2e/various-suite/explore.spec.ts index 4e089a27b79..57dab64ceec 100644 --- a/e2e/various-suite/explore.spec.ts +++ b/e2e/various-suite/explore.spec.ts @@ -19,6 +19,28 @@ e2e.scenario({ cy.contains('CSV Metric Values').scrollIntoView().should('be.visible').click(); + cy.location().then((loc) => { + const params = new URLSearchParams(loc.search); + const leftJSON = JSON.parse(params.get('left')); + expect(leftJSON.range.to).to.equal('now'); + expect(leftJSON.range.from).to.equal('now-1h'); + + cy.get('body').click(); + cy.get('body').type('t{leftarrow}'); + + cy.location().then((locPostKeypress) => { + const params = new URLSearchParams(locPostKeypress.search); + const leftJSON = JSON.parse(params.get('left')); + // be sure the keypress affected the time window + expect(leftJSON.range.to).to.not.equal('now'); + expect(leftJSON.range.from).to.not.equal('now-1h'); + // be sure the url does not contain dashboard range values + // eslint wants this to be a function, so we use this instead of to.be.false + expect(params.has('to')).to.equal(false); + expect(params.has('from')).to.equal(false); + }); + }); + const canvases = e2e().get('canvas'); canvases.should('have.length', 1); }, diff --git a/public/app/core/services/keybindingSrv.ts b/public/app/core/services/keybindingSrv.ts index a232a133e4f..765b2a09b61 100644 --- a/public/app/core/services/keybindingSrv.ts +++ b/public/app/core/services/keybindingSrv.ts @@ -11,7 +11,7 @@ import { exitKioskMode, toggleKioskMode } from '../navigation/kiosk'; import { RemovePanelEvent, ShiftTimeEvent, - ShiftTimeEventPayload, + ShiftTimeEventDirection, ShowModalReactEvent, ZoomOutEvent, AbsoluteTimeEvent, @@ -171,6 +171,24 @@ export class KeybindingSrv { this.bind(keyArg, withFocusedPanel(fn)); } + setupTimeRangeBindings(updateUrl = true) { + this.bind('t z', () => { + appEvents.publish(new ZoomOutEvent({ scale: 2, updateUrl })); + }); + + this.bind('ctrl+z', () => { + appEvents.publish(new ZoomOutEvent({ scale: 2, updateUrl })); + }); + + this.bind('t left', () => { + appEvents.publish(new ShiftTimeEvent({ direction: ShiftTimeEventDirection.Left, updateUrl })); + }); + + this.bind('t right', () => { + appEvents.publish(new ShiftTimeEvent({ direction: ShiftTimeEventDirection.Right, updateUrl })); + }); + } + setupDashboardBindings(dashboard: DashboardModel) { this.bind('mod+o', () => { dashboard.graphTooltip = (dashboard.graphTooltip + 1) % 3; @@ -191,21 +209,7 @@ export class KeybindingSrv { } }); - this.bind('t z', () => { - appEvents.publish(new ZoomOutEvent(2)); - }); - - this.bind('ctrl+z', () => { - appEvents.publish(new ZoomOutEvent(2)); - }); - - this.bind('t left', () => { - appEvents.publish(new ShiftTimeEvent(ShiftTimeEventPayload.Left)); - }); - - this.bind('t right', () => { - appEvents.publish(new ShiftTimeEvent(ShiftTimeEventPayload.Right)); - }); + this.setupTimeRangeBindings(); // edit panel this.bindWithPanelId('e', (panelId) => { diff --git a/public/app/features/dashboard/components/DashNav/DashNavTimeControls.tsx b/public/app/features/dashboard/components/DashNav/DashNavTimeControls.tsx index 94313c0ec21..640efa92c3a 100644 --- a/public/app/features/dashboard/components/DashNav/DashNavTimeControls.tsx +++ b/public/app/features/dashboard/components/DashNav/DashNavTimeControls.tsx @@ -13,7 +13,7 @@ import { TimePickerWithHistory } from 'app/core/components/TimePicker/TimePicker // Utils & Services import { getTimeSrv } from 'app/features/dashboard/services/TimeSrv'; import { appEvents } from 'app/core/core'; -import { ShiftTimeEvent, ShiftTimeEventPayload, ZoomOutEvent } from '../../../../types/events'; +import { ShiftTimeEvent, ShiftTimeEventDirection, ZoomOutEvent } from '../../../../types/events'; import { Unsubscribable } from 'rxjs'; export interface Props { @@ -38,16 +38,16 @@ export class DashNavTimeControls extends Component { }; onRefresh = () => { - getTimeSrv().refreshDashboard(); + getTimeSrv().refreshTimeModel(); return Promise.resolve(); }; onMoveBack = () => { - appEvents.publish(new ShiftTimeEvent(ShiftTimeEventPayload.Left)); + appEvents.publish(new ShiftTimeEvent({ direction: ShiftTimeEventDirection.Left })); }; onMoveForward = () => { - appEvents.publish(new ShiftTimeEvent(ShiftTimeEventPayload.Right)); + appEvents.publish(new ShiftTimeEvent({ direction: ShiftTimeEventDirection.Right })); }; onChangeTimePicker = (timeRange: TimeRange) => { @@ -77,7 +77,7 @@ export class DashNavTimeControls extends Component { }; onZoom = () => { - appEvents.publish(new ZoomOutEvent(2)); + appEvents.publish(new ZoomOutEvent({ scale: 2 })); }; render() { diff --git a/public/app/features/dashboard/services/TimeSrv.test.ts b/public/app/features/dashboard/services/TimeSrv.test.ts index 34a03f6dfe0..90220dd63c1 100644 --- a/public/app/features/dashboard/services/TimeSrv.test.ts +++ b/public/app/features/dashboard/services/TimeSrv.test.ts @@ -244,6 +244,11 @@ describe('timeSrv', () => { expect(locationUpdates[1].search).toEqual('?kiosk&from=now-1h&to=now-10s'); }); + + it('should not change the URL if the updateUrl param is false', () => { + timeSrv.setTime({ from: '1644340584281', to: '1644340584281' }, false); + expect(locationUpdates.length).toBe(0); + }); }); describe('pauseAutoRefresh', () => { diff --git a/public/app/features/dashboard/services/TimeSrv.ts b/public/app/features/dashboard/services/TimeSrv.ts index 4892ac510a9..490f480ce55 100644 --- a/public/app/features/dashboard/services/TimeSrv.ts +++ b/public/app/features/dashboard/services/TimeSrv.ts @@ -9,14 +9,14 @@ import { TimeRange, toUtc, } from '@grafana/data'; -import { DashboardModel } from '../state/DashboardModel'; import { getShiftedTimeRange, getZoomedTimeRange } from 'app/core/utils/timePicker'; import { config } from 'app/core/config'; import { getRefreshFromUrl } from '../utils/getRefreshFromUrl'; import { locationService } from '@grafana/runtime'; -import { AbsoluteTimeEvent, ShiftTimeEvent, ShiftTimeEventPayload, ZoomOutEvent } from '../../../types/events'; +import { AbsoluteTimeEvent, ShiftTimeEvent, ShiftTimeEventDirection, ZoomOutEvent } from '../../../types/events'; import { contextSrv, ContextSrv } from 'app/core/services/context_srv'; import appEvents from 'app/core/app_events'; +import { TimeModel } from '../state/TimeModel'; export class TimeSrv { time: any; @@ -24,21 +24,21 @@ export class TimeSrv { refresh: any; previousAutoRefresh: any; oldRefresh: string | null | undefined; - dashboard?: DashboardModel; + timeModel?: TimeModel; timeAtLoad: any; private autoRefreshBlocked?: boolean; constructor(private contextSrv: ContextSrv) { // default time this.time = getDefaultTimeRange().raw; - this.refreshDashboard = this.refreshDashboard.bind(this); + this.refreshTimeModel = this.refreshTimeModel.bind(this); appEvents.subscribe(ZoomOutEvent, (e) => { - this.zoomOut(e.payload); + this.zoomOut(e.payload.scale, e.payload.updateUrl); }); appEvents.subscribe(ShiftTimeEvent, (e) => { - this.shiftTime(e.payload); + this.shiftTime(e.payload.direction, e.payload.updateUrl); }); appEvents.subscribe(AbsoluteTimeEvent, () => { @@ -48,15 +48,15 @@ export class TimeSrv { document.addEventListener('visibilitychange', () => { if (this.autoRefreshBlocked && document.visibilityState === 'visible') { this.autoRefreshBlocked = false; - this.refreshDashboard(); + this.refreshTimeModel(); } }); } - init(dashboard: DashboardModel) { - this.dashboard = dashboard; - this.time = dashboard.time; - this.refresh = dashboard.refresh; + init(timeModel: TimeModel) { + this.timeModel = timeModel; + this.time = timeModel.time; + this.refresh = timeModel.refresh; this.initTimeFromUrl(); this.parseTime(); @@ -66,8 +66,8 @@ export class TimeSrv { const range = rangeUtil.convertRawToRange( this.time, - this.dashboard?.getTimezone(), - this.dashboard?.fiscalYearStartMonth + this.timeModel?.getTimezone(), + this.timeModel?.fiscalYearStartMonth ); if (range.to.isBefore(range.from)) { @@ -159,11 +159,11 @@ export class TimeSrv { this.time.to = this.parseUrlParam(params.get('to')!) || this.time.to; } - // if absolute ignore refresh option saved to dashboard + // if absolute ignore refresh option saved to timeModel if (params.get('to') && params.get('to')!.indexOf('now') === -1) { this.refresh = false; - if (this.dashboard) { - this.dashboard.refresh = false; + if (this.timeModel) { + this.timeModel.refresh = false; } } @@ -176,8 +176,8 @@ export class TimeSrv { this.refresh = getRefreshFromUrl({ params: paramsJSON, currentRefresh: this.refresh, - refreshIntervals: Array.isArray(this.dashboard?.timepicker?.refresh_intervals) - ? this.dashboard?.timepicker?.refresh_intervals + refreshIntervals: Array.isArray(this.timeModel?.timepicker?.refresh_intervals) + ? this.timeModel?.timepicker?.refresh_intervals : undefined, isAllowedIntervalFn: this.contextSrv.isAllowedInterval, minRefreshInterval: config.minRefreshInterval, @@ -213,8 +213,8 @@ export class TimeSrv { } setAutoRefresh(interval: any) { - if (this.dashboard) { - this.dashboard.refresh = interval; + if (this.timeModel) { + this.timeModel.refresh = interval; } this.stopAutoRefresh(); @@ -235,7 +235,7 @@ export class TimeSrv { this.refreshTimer = setTimeout(() => { this.startNextRefreshTimer(intervalMs); - this.refreshDashboard(); + this.refreshTimeModel(); }, intervalMs); const refresh = this.contextSrv.getValidInterval(interval); @@ -245,15 +245,15 @@ export class TimeSrv { } } - refreshDashboard() { - this.dashboard?.timeRangeUpdated(this.timeRange()); + refreshTimeModel() { + this.timeModel?.timeRangeUpdated(this.timeRange()); } private startNextRefreshTimer(afterMs: number) { this.refreshTimer = setTimeout(() => { this.startNextRefreshTimer(afterMs); if (this.contextSrv.isGrafanaVisible()) { - this.refreshDashboard(); + this.refreshTimeModel(); } else { this.autoRefreshBlocked = true; } @@ -264,10 +264,10 @@ export class TimeSrv { clearTimeout(this.refreshTimer); } - // store dashboard refresh value and pause auto-refresh in some places + // store timeModel refresh value and pause auto-refresh in some places // i.e panel edit pauseAutoRefresh() { - this.previousAutoRefresh = this.dashboard?.refresh; + this.previousAutoRefresh = this.timeModel?.refresh; this.setAutoRefresh(''); } @@ -276,20 +276,19 @@ export class TimeSrv { this.setAutoRefresh(this.previousAutoRefresh); } - setTime(time: RawTimeRange, fromRouteUpdate?: boolean) { + setTime(time: RawTimeRange, updateUrl = true) { extend(this.time, time); // disable refresh if zoom in or zoom out if (isDateTime(time.to)) { - this.oldRefresh = this.dashboard?.refresh || this.oldRefresh; + this.oldRefresh = this.timeModel?.refresh || this.oldRefresh; this.setAutoRefresh(false); - } else if (this.oldRefresh && this.oldRefresh !== this.dashboard?.refresh) { + } else if (this.oldRefresh && this.oldRefresh !== this.timeModel?.refresh) { this.setAutoRefresh(this.oldRefresh); this.oldRefresh = null; } - // update url - if (fromRouteUpdate !== true) { + if (updateUrl === true) { const urlRange = this.timeRangeForUrl(); const urlParams = locationService.getSearchObject(); @@ -303,7 +302,7 @@ export class TimeSrv { locationService.partial(urlParams); } - this.refreshDashboard(); + this.refreshTimeModel(); } timeRangeForUrl = () => { @@ -326,30 +325,33 @@ export class TimeSrv { to: isDateTime(this.time.to) ? dateTime(this.time.to) : this.time.to, }; - const timezone = this.dashboard ? this.dashboard.getTimezone() : undefined; + const timezone = this.timeModel ? this.timeModel.getTimezone() : undefined; return { - from: dateMath.parse(raw.from, false, timezone, this.dashboard?.fiscalYearStartMonth)!, - to: dateMath.parse(raw.to, true, timezone, this.dashboard?.fiscalYearStartMonth)!, + from: dateMath.parse(raw.from, false, timezone, this.timeModel?.fiscalYearStartMonth)!, + to: dateMath.parse(raw.to, true, timezone, this.timeModel?.fiscalYearStartMonth)!, raw: raw, }; } - zoomOut(factor: number) { + zoomOut(factor: number, updateUrl = true) { const range = this.timeRange(); const { from, to } = getZoomedTimeRange(range, factor); - this.setTime({ from: toUtc(from), to: toUtc(to) }); + this.setTime({ from: toUtc(from), to: toUtc(to) }, updateUrl); } - shiftTime(direction: ShiftTimeEventPayload) { + shiftTime(direction: ShiftTimeEventDirection, updateUrl = true) { const range = this.timeRange(); const { from, to } = getShiftedTimeRange(direction, range); - this.setTime({ - from: toUtc(from), - to: toUtc(to), - }); + this.setTime( + { + from: toUtc(from), + to: toUtc(to), + }, + updateUrl + ); } makeAbsoluteTime() { @@ -359,7 +361,7 @@ export class TimeSrv { } const { from, to } = this.timeRange(); - this.setTime({ from, to }); + this.setTime({ from, to }, true); } // isRefreshOutsideThreshold function calculates the difference between last refresh and now diff --git a/public/app/features/dashboard/state/DashboardModel.ts b/public/app/features/dashboard/state/DashboardModel.ts index 212cb450d40..bbda7085b5d 100644 --- a/public/app/features/dashboard/state/DashboardModel.ts +++ b/public/app/features/dashboard/state/DashboardModel.ts @@ -20,6 +20,7 @@ import { GRID_CELL_HEIGHT, GRID_CELL_VMARGIN, GRID_COLUMN_COUNT, REPEAT_DIR_VERT import { contextSrv } from 'app/core/services/context_srv'; // Types import { GridPos, PanelModel } from './PanelModel'; +import { TimeModel } from './TimeModel'; import { DashboardMigrator } from './DashboardMigrator'; import { AnnotationQuery, @@ -78,7 +79,7 @@ export interface DashboardLink { includeVars: boolean; } -export class DashboardModel { +export class DashboardModel implements TimeModel { id: any; uid: string; title: string; diff --git a/public/app/features/dashboard/state/TimeModel.ts b/public/app/features/dashboard/state/TimeModel.ts new file mode 100644 index 00000000000..b7df209d6f7 --- /dev/null +++ b/public/app/features/dashboard/state/TimeModel.ts @@ -0,0 +1,10 @@ +import { TimeRange, TimeZone } from '@grafana/data'; + +export interface TimeModel { + time: any; + fiscalYearStartMonth?: number; + refresh: any; + timepicker: any; + getTimezone(): TimeZone; + timeRangeUpdated(timeRange: TimeRange): void; +} diff --git a/public/app/features/dashboard/state/actions.ts b/public/app/features/dashboard/state/actions.ts index 9fdd7d1b5eb..1a503a276ad 100644 --- a/public/app/features/dashboard/state/actions.ts +++ b/public/app/features/dashboard/state/actions.ts @@ -131,12 +131,12 @@ export const updateTimeZoneDashboard = (timeZone: TimeZone): ThunkResult => (dispatch) => { dispatch(updateTimeZoneForSession(timeZone)); - getTimeSrv().refreshDashboard(); + getTimeSrv().refreshTimeModel(); }; export const updateWeekStartDashboard = (weekStart: string): ThunkResult => (dispatch) => { dispatch(updateWeekStartForSession(weekStart)); - getTimeSrv().refreshDashboard(); + getTimeSrv().refreshTimeModel(); }; diff --git a/public/app/features/explore/state/explorePane.ts b/public/app/features/explore/state/explorePane.ts index 31bb1b1da06..a40de3799cb 100644 --- a/public/app/features/explore/state/explorePane.ts +++ b/public/app/features/explore/state/explorePane.ts @@ -38,6 +38,7 @@ import { getFiscalYearStartMonth, getTimeZone } from 'app/features/profile/state import { getDataSourceSrv } from '@grafana/runtime'; import { getRichHistory } from '../../../core/utils/richHistory'; import { richHistoryUpdatedAction, stateSave } from './main'; +import { keybindingSrv } from 'app/core/services/keybindingSrv'; // // Actions and Payloads @@ -172,6 +173,8 @@ export function initializeExplore( } dispatch(updateTime({ exploreId })); + keybindingSrv.setupTimeRangeBindings(false); + if (instance) { // We do not want to add the url to browser history on init because when the pane is initialised it's because // we already have something in the url. Adding basically the same state as additional history item prevents diff --git a/public/app/features/explore/state/time.ts b/public/app/features/explore/state/time.ts index 9d8e7ab3c4b..5ab326f4646 100644 --- a/public/app/features/explore/state/time.ts +++ b/public/app/features/explore/state/time.ts @@ -14,7 +14,7 @@ import { ExploreItemState, ThunkResult } from 'app/types'; import { ExploreId } from 'app/types/explore'; import { getFiscalYearStartMonth, getTimeZone } from 'app/features/profile/state/selectors'; import { getTimeSrv } from '../../dashboard/services/TimeSrv'; -import { DashboardModel } from 'app/features/dashboard/state'; +import { TimeModel } from '../../dashboard/state/TimeModel'; import { runQueries } from './query'; import { syncTimesAction, stateSave } from './main'; @@ -95,14 +95,17 @@ export const updateTime = (config: { const range = getTimeRange(timeZone, rawRange, fiscalYearStartMonth); const absoluteRange: AbsoluteTimeRange = { from: range.from.valueOf(), to: range.to.valueOf() }; + const timeModel: TimeModel = { + time: range.raw, + refresh: false, + timepicker: {}, + getTimezone: () => timeZone, + timeRangeUpdated: (rawTimeRange: RawTimeRange) => { + dispatch(updateTimeRange({ exploreId: exploreId, rawRange: rawTimeRange })); + }, + }; - getTimeSrv().init( - new DashboardModel({ - time: range.raw, - refresh: false, - timeZone, - }) - ); + getTimeSrv().init(timeModel); dispatch(changeRangeAction({ exploreId, range, absoluteRange })); }; diff --git a/public/app/plugins/panel/graph/module.ts b/public/app/plugins/panel/graph/module.ts index 37d0f433dba..cd7a23f6b23 100644 --- a/public/app/plugins/panel/graph/module.ts +++ b/public/app/plugins/panel/graph/module.ts @@ -187,7 +187,7 @@ export class GraphCtrl extends MetricsPanelCtrl { } zoomOut(evt: any) { - appEvents.publish(new ZoomOutEvent(2)); + appEvents.publish(new ZoomOutEvent({ scale: 2 })); } onDataSnapshotLoad(snapshotData: any) { diff --git a/public/app/plugins/panel/heatmap/heatmap_ctrl.ts b/public/app/plugins/panel/heatmap/heatmap_ctrl.ts index 54a24833cb3..c9effe62ba9 100644 --- a/public/app/plugins/panel/heatmap/heatmap_ctrl.ts +++ b/public/app/plugins/panel/heatmap/heatmap_ctrl.ts @@ -161,7 +161,7 @@ export class HeatmapCtrl extends MetricsPanelCtrl { } zoomOut(evt: any) { - appEvents.publish(new ZoomOutEvent(2)); + appEvents.publish(new ZoomOutEvent({ scale: 2 })); } onRender() { diff --git a/public/app/types/events.ts b/public/app/types/events.ts index 1acce376945..fc4cc3ff05e 100644 --- a/public/app/types/events.ts +++ b/public/app/types/events.ts @@ -132,14 +132,25 @@ export class RenderEvent extends BusEventBase { static type = 'render'; } -export class ZoomOutEvent extends BusEventWithPayload { +interface ZoomOutEventPayload { + scale: number; + updateUrl?: boolean; +} + +export class ZoomOutEvent extends BusEventWithPayload { static type = 'zoom-out'; } -export enum ShiftTimeEventPayload { +export enum ShiftTimeEventDirection { Left = -1, Right = 1, } + +interface ShiftTimeEventPayload { + direction: ShiftTimeEventDirection; + updateUrl?: boolean; +} + export class ShiftTimeEvent extends BusEventWithPayload { static type = 'shift-time'; }