From c13f4542d8de59e262aeddb61cd07c52b5db2d5e Mon Sep 17 00:00:00 2001 From: Ashley Harrison Date: Thu, 24 Mar 2022 16:01:43 +0000 Subject: [PATCH] Dashboard: Template variables are now correctly persisted when clicking breadcrumb links (#46790) * Add history listener to update titleHref/parentHref when location changes * Convert to functional component and use useLocation * Wrap component in React.memo * Add new `getUrlForPartial` method, deprecate `updateSearchParams` --- packages/grafana-data/package.json | 2 + packages/grafana-data/src/utils/location.ts | 24 +++ .../dashboard/components/DashNav/DashNav.tsx | 154 +++++++++--------- .../DashboardSettings/DashboardSettings.tsx | 2 +- yarn.lock | 2 + 5 files changed, 106 insertions(+), 78 deletions(-) diff --git a/packages/grafana-data/package.json b/packages/grafana-data/package.json index e00890fcf0c..9fa2b6f0af8 100644 --- a/packages/grafana-data/package.json +++ b/packages/grafana-data/package.json @@ -53,6 +53,7 @@ "@testing-library/react-hooks": "7.0.2", "@testing-library/user-event": "13.5.0", "@types/braintree__sanitize-url": "4.1.0", + "@types/history": "4.7.11", "@types/jest": "27.4.1", "@types/jquery": "3.5.14", "@types/lodash": "4.14.149", @@ -65,6 +66,7 @@ "@types/testing-library__jest-dom": "5.14.3", "@types/testing-library__react-hooks": "^3.2.0", "@types/tinycolor2": "1.4.3", + "history": "4.10.1", "react-test-renderer": "17.0.2", "rimraf": "3.0.2", "rollup": "2.70.1", diff --git a/packages/grafana-data/src/utils/location.ts b/packages/grafana-data/src/utils/location.ts index fa9c2b45d24..83f481067bb 100644 --- a/packages/grafana-data/src/utils/location.ts +++ b/packages/grafana-data/src/utils/location.ts @@ -1,3 +1,4 @@ +import { Location } from 'history'; import { GrafanaConfig, RawTimeRange, ScopedVars } from '../types'; import { UrlQueryMap, urlUtil } from './url'; import { textUtil } from '../text'; @@ -37,6 +38,28 @@ const assureBaseUrl = (url: string): string => { }; /** + * + * @param location + * @param searchParamsToUpdate + * @returns + */ +const getUrlForPartial = (location: Location, searchParamsToUpdate: Record) => { + const searchParams = urlUtil.parseKeyValue( + location.search.startsWith('?') ? location.search.substring(1) : location.search + ); + for (const key of Object.keys(searchParamsToUpdate)) { + // removing params with null | undefined + if (searchParamsToUpdate[key] === null || searchParamsToUpdate[key] === undefined) { + delete searchParams[key]; + } else { + searchParams[key] = searchParamsToUpdate[key]; + } + } + return urlUtil.renderUrl(location.pathname, searchParams); +}; + +/** + * @deprecated use `getUrlForPartial` instead * Update URL or search param string `init` with new params `partial`. */ const updateSearchParams = (init: string, partial: string) => { @@ -92,6 +115,7 @@ export const locationUtil = { const params = getVariablesUrlParams(scopedVars); return urlUtil.toUrlParams(params); }, + getUrlForPartial, processUrl: (url: string) => { return grafanaConfig.disableSanitizeHtml ? url : textUtil.sanitizeUrl(url); }, diff --git a/public/app/features/dashboard/components/DashNav/DashNav.tsx b/public/app/features/dashboard/components/DashNav/DashNav.tsx index 9b445819720..b4426369ecb 100644 --- a/public/app/features/dashboard/components/DashNav/DashNav.tsx +++ b/public/app/features/dashboard/components/DashNav/DashNav.tsx @@ -1,12 +1,13 @@ // Libaries -import React, { PureComponent, FC, ReactNode } from 'react'; +import React, { FC, ReactNode } from 'react'; import { connect, ConnectedProps } from 'react-redux'; +import { useLocation } from 'react-router-dom'; // Utils & Services import { playlistSrv } from 'app/features/playlist/PlaylistSrv'; // Components import { DashNavButton } from './DashNavButton'; import { DashNavTimeControls } from './DashNavTimeControls'; -import { ButtonGroup, ModalsController, ToolbarButton, PageToolbar } from '@grafana/ui'; +import { ButtonGroup, ModalsController, ToolbarButton, PageToolbar, useForceUpdate } from '@grafana/ui'; import { locationUtil, textUtil } from '@grafana/data'; // State import { updateTimeZoneForSession } from 'app/features/profile/state/reducers'; @@ -56,64 +57,62 @@ export function addCustomRightAction(content: DashNavButtonModel) { type Props = OwnProps & ConnectedProps; -class DashNav extends PureComponent { - constructor(props: Props) { - super(props); - } +export const DashNav = React.memo((props) => { + const forceUpdate = useForceUpdate(); - onClose = () => { - locationService.partial({ viewPanel: null }); - }; - - onToggleTVMode = () => { - toggleKioskMode(); - }; - - onOpenSettings = () => { - locationService.partial({ editview: 'settings' }); - }; - - onStarDashboard = () => { - const { dashboard } = this.props; + const onStarDashboard = () => { const dashboardSrv = getDashboardSrv(); + const { dashboard } = props; dashboardSrv.starDashboard(dashboard.id, dashboard.meta.isStarred).then((newState: any) => { dashboard.meta.isStarred = newState; - this.forceUpdate(); + forceUpdate(); }); }; - onPlaylistPrev = () => { + const onClose = () => { + locationService.partial({ viewPanel: null }); + }; + + const onToggleTVMode = () => { + toggleKioskMode(); + }; + + const onOpenSettings = () => { + locationService.partial({ editview: 'settings' }); + }; + + const onPlaylistPrev = () => { playlistSrv.prev(); }; - onPlaylistNext = () => { + const onPlaylistNext = () => { playlistSrv.next(); }; - onPlaylistStop = () => { + const onPlaylistStop = () => { playlistSrv.stop(); - this.forceUpdate(); + forceUpdate(); }; - addCustomContent(actions: DashNavButtonModel[], buttons: ReactNode[]) { + const addCustomContent = (actions: DashNavButtonModel[], buttons: ReactNode[]) => { actions.map((action, index) => { const Component = action.component; - const element = ; + const element = ; typeof action.index === 'number' ? buttons.splice(action.index, 0, element) : buttons.push(element); }); - } + }; - isPlaylistRunning() { + const isPlaylistRunning = () => { return playlistSrv.isPlaying; - } + }; - renderLeftActionsButton() { - const { dashboard, kioskMode } = this.props; + const renderLeftActionsButton = () => { + const { dashboard, kioskMode } = props; const { canStar, canShare, isStarred } = dashboard.meta; const buttons: ReactNode[] = []; - if (kioskMode !== KioskMode.Off || this.isPlaylistRunning()) { + if (kioskMode !== KioskMode.Off || isPlaylistRunning()) { return []; } @@ -125,7 +124,7 @@ class DashNav extends PureComponent { icon={isStarred ? 'favorite' : 'star'} iconType={isStarred ? 'mono' : 'default'} iconSize="lg" - onClick={this.onStarDashboard} + onClick={onStarDashboard} key="button-star" /> ); @@ -172,22 +171,22 @@ class DashNav extends PureComponent { ); } - this.addCustomContent(customLeftActions, buttons); + addCustomContent(customLeftActions, buttons); return buttons; - } + }; - renderPlaylistControls() { + const renderPlaylistControls = () => { return ( - - Stop playlist - + + Stop playlist + ); - } + }; - renderTimeControls() { - const { dashboard, updateTimeZoneForSession, hideTimePicker } = this.props; + const renderTimeControls = () => { + const { dashboard, updateTimeZoneForSession, hideTimePicker } = props; if (hideTimePicker) { return null; @@ -196,24 +195,24 @@ class DashNav extends PureComponent { return ( ); - } + }; - renderRightActionsButton() { - const { dashboard, onAddPanel, isFullscreen, kioskMode } = this.props; + const renderRightActionsButton = () => { + const { dashboard, onAddPanel, isFullscreen, kioskMode } = props; const { canSave, canEdit, showSettings } = dashboard.meta; const { snapshot } = dashboard; const snapshotUrl = snapshot && snapshot.originalUrl; const buttons: ReactNode[] = []; const tvButton = ( - + ); - if (this.isPlaylistRunning()) { - return [this.renderPlaylistControls(), this.renderTimeControls()]; + if (isPlaylistRunning()) { + return [renderPlaylistControls(), renderTimeControls()]; } if (kioskMode === KioskMode.TV) { - return [this.renderTimeControls(), tvButton]; + return [renderTimeControls(), tvButton]; } if (canEdit && !isFullscreen) { @@ -243,7 +242,7 @@ class DashNav extends PureComponent { buttons.push( this.gotoSnapshotOrigin(snapshotUrl)} + onClick={() => gotoSnapshotOrigin(snapshotUrl)} icon="link" key="button-snapshot" /> @@ -252,42 +251,43 @@ class DashNav extends PureComponent { if (showSettings) { buttons.push( - + ); } - this.addCustomContent(customRightActions, buttons); + addCustomContent(customRightActions, buttons); - buttons.push(this.renderTimeControls()); + buttons.push(renderTimeControls()); buttons.push(tvButton); return buttons; - } + }; - gotoSnapshotOrigin(snapshotUrl: string) { + const gotoSnapshotOrigin = (snapshotUrl: string) => { window.location.href = textUtil.sanitizeUrl(snapshotUrl); - } + }; - render() { - const { isFullscreen, title, folderTitle } = this.props; - const onGoBack = isFullscreen ? this.onClose : undefined; + const { isFullscreen, title, folderTitle } = props; + // this ensures the component rerenders when the location changes + const location = useLocation(); + const titleHref = locationUtil.getUrlForPartial(location, { search: 'open' }); + const parentHref = locationUtil.getUrlForPartial(location, { search: 'open', folder: 'current' }); + const onGoBack = isFullscreen ? onClose : undefined; - const titleHref = locationUtil.updateSearchParams(window.location.href, '?search=open'); - const parentHref = locationUtil.updateSearchParams(window.location.href, '?search=open&folder=current'); + return ( + + {renderRightActionsButton()} + + ); +}); - return ( - - {this.renderRightActionsButton()} - - ); - } -} +DashNav.displayName = 'DashNav'; export default connector(DashNav); diff --git a/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx b/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx index 71dd8d9b870..6d6aa3b3ab8 100644 --- a/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx @@ -150,7 +150,7 @@ export function DashboardSettings({ dashboard, editview }: Props) { {pages.map((page) => ( reportInteraction(`Dashboard settings navigation to ${page.id}`)} - to={(loc) => locationUtil.updateSearchParams(loc.search, `editview=${page.id}`)} + to={(loc) => locationUtil.getUrlForPartial(loc, { editview: page.id })} className={cx('dashboard-settings__nav-item', { active: page.id === editview })} key={page.id} > diff --git a/yarn.lock b/yarn.lock index 2cca9a87a2f..369f14504e9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4106,6 +4106,7 @@ __metadata: "@testing-library/user-event": 13.5.0 "@types/braintree__sanitize-url": 4.1.0 "@types/d3-interpolate": ^1.4.0 + "@types/history": 4.7.11 "@types/jest": 27.4.1 "@types/jquery": 3.5.14 "@types/lodash": 4.14.149 @@ -4121,6 +4122,7 @@ __metadata: d3-interpolate: 1.4.0 date-fns: 2.28.0 eventemitter3: 4.0.7 + history: 4.10.1 lodash: 4.17.21 marked: 4.0.12 moment: 2.29.1