TopNav: KioskMode rewrite move to AppChrome responsibility and make it a global feature (#55149)

* Initial progress

* Moving keybindingSrv to context

* Simplfy KioskMode

* Removed unused logic

* Make kiosk=tv behave as before but when topnav is enabled

* Minor fix

* Fixing tests

* Fixing bug with notice when entering kiosk mode

* Fixed test
This commit is contained in:
Torkel Ödegaard
2022-09-15 14:04:58 +02:00
committed by GitHub
parent 7352c181c2
commit b8e72d6173
24 changed files with 216 additions and 143 deletions

View File

@@ -18,10 +18,9 @@ import { useSelector } from 'react-redux';
import { GrafanaTheme2 } from '@grafana/data';
import { reportInteraction, locationService } from '@grafana/runtime';
import { useStyles2 } from '@grafana/ui';
import { useGrafana } from 'app/core/context/GrafanaContext';
import { StoreState } from 'app/types';
import { keybindingSrv } from '../../core/services/keybindingSrv';
import { ResultItem } from './ResultItem';
import getDashboardNavActions from './actions/dashboard.nav.actions';
import getGlobalActions from './actions/global.static.actions';
@@ -33,6 +32,7 @@ import getGlobalActions from './actions/global.static.actions';
export const CommandPalette = () => {
const styles = useStyles2(getSearchStyles);
const { keybindings } = useGrafana();
const [actions, setActions] = useState<Action[]>([]);
const [staticActions, setStaticActions] = useState<Action[]>([]);
const { query, showing } = useKBar((state) => ({
@@ -63,14 +63,14 @@ export const CommandPalette = () => {
setActions([...staticActions, ...dashAct]);
});
keybindingSrv.bindGlobal('esc', () => {
keybindings.bindGlobal('esc', () => {
query.setVisualState(VisualState.animatingOut);
});
}
return () => {
keybindingSrv.bindGlobal('esc', () => {
keybindingSrv.globalEsc();
keybindings.bindGlobal('esc', () => {
keybindings.globalEsc();
});
};
// eslint-disable-next-line react-hooks/exhaustive-deps

View File

@@ -18,7 +18,7 @@ import {
import { AppChromeUpdate } from 'app/core/components/AppChrome/AppChromeUpdate';
import { NavToolbarSeparator } from 'app/core/components/AppChrome/NavToolbarSeparator';
import config from 'app/core/config';
import { toggleKioskMode } from 'app/core/navigation/kiosk';
import { useGrafana } from 'app/core/context/GrafanaContext';
import { DashboardCommentsModal } from 'app/features/dashboard/components/DashboardComments/DashboardCommentsModal';
import { SaveDashboardDrawer } from 'app/features/dashboard/components/SaveDashboard/SaveDashboardDrawer';
import { ShareModal } from 'app/features/dashboard/components/ShareModal';
@@ -45,7 +45,7 @@ const selectors = e2eSelectors.pages.Dashboard.DashNav;
export interface OwnProps {
dashboard: DashboardModel;
isFullscreen: boolean;
kioskMode: KioskMode;
kioskMode?: KioskMode | null;
hideTimePicker: boolean;
folderTitle?: string;
title: string;
@@ -73,6 +73,7 @@ type Props = OwnProps & ConnectedProps<typeof connector>;
export const DashNav = React.memo<Props>((props) => {
const forceUpdate = useForceUpdate();
const { chrome } = useGrafana();
const onStarDashboard = () => {
const dashboardSrv = getDashboardSrv();
@@ -90,7 +91,7 @@ export const DashNav = React.memo<Props>((props) => {
};
const onToggleTVMode = () => {
toggleKioskMode();
chrome.onToggleKioskMode();
};
const onOpenSettings = () => {
@@ -127,7 +128,7 @@ export const DashNav = React.memo<Props>((props) => {
const { canStar, canShare, isStarred } = dashboard.meta;
const buttons: ReactNode[] = [];
if (kioskMode !== KioskMode.Off || isPlaylistRunning()) {
if (kioskMode || isPlaylistRunning()) {
return [];
}
@@ -235,7 +236,7 @@ export const DashNav = React.memo<Props>((props) => {
const { snapshot } = dashboard;
const snapshotUrl = snapshot && snapshot.originalUrl;
const buttons: ReactNode[] = [];
const tvButton = (
const tvButton = config.featureToggles.topnav ? null : (
<ToolbarButton
tooltip={t({ id: 'dashboard.toolbar.tv-button', message: 'Cycle view mode' })}
icon="monitor"

View File

@@ -5,11 +5,13 @@ import { Router } from 'react-router-dom';
import { useEffectOnce } from 'react-use';
import { AutoSizerProps } from 'react-virtualized-auto-sizer';
import { mockToolkitActionCreator } from 'test/core/redux/mocks';
import { getGrafanaContextMock } from 'test/mocks/getGrafanaContextMock';
import { createTheme } from '@grafana/data';
import { selectors } from '@grafana/e2e-selectors';
import { config, locationService, setDataSourceSrv } from '@grafana/runtime';
import { notifyApp } from 'app/core/actions';
import { GrafanaContext } from 'app/core/context/GrafanaContext';
import { getRouteComponentProps } from 'app/core/navigation/__mocks__/routeProps';
import { DashboardInitPhase, DashboardMeta, DashboardRoutes } from 'app/types';
@@ -130,12 +132,16 @@ function dashboardPageScenario(description: string, scenarioFn: (ctx: ScenarioCo
ctx.props = props;
ctx.dashboard = props.dashboard;
const context = getGrafanaContextMock();
const { container, rerender, unmount } = render(
<Provider store={store}>
<Router history={locationService.getHistory()}>
<UnthemedDashboardPage {...props} />
</Router>
</Provider>
<GrafanaContext.Provider value={context}>
<Provider store={store}>
<Router history={locationService.getHistory()}>
<UnthemedDashboardPage {...props} />
</Router>
</Provider>
</GrafanaContext.Provider>
);
ctx.container = container;
@@ -144,11 +150,13 @@ function dashboardPageScenario(description: string, scenarioFn: (ctx: ScenarioCo
Object.assign(props, newProps);
rerender(
<Provider store={store}>
<Router history={locationService.getHistory()}>
<UnthemedDashboardPage {...props} />
</Router>
</Provider>
<GrafanaContext.Provider value={context}>
<Provider store={store}>
<Router history={locationService.getHistory()}>
<UnthemedDashboardPage {...props} />
</Router>
</Provider>
</GrafanaContext.Provider>
);
};
@@ -179,6 +187,7 @@ describe('DashboardPage', () => {
routeName: 'normal-dashboard',
urlSlug: 'my-dash',
urlUid: '11',
keybindingSrv: expect.anything(),
});
});
});

View File

@@ -9,6 +9,7 @@ import { Themeable2, withTheme2 } from '@grafana/ui';
import { notifyApp } from 'app/core/actions';
import { Page } from 'app/core/components/Page/Page';
import { config } from 'app/core/config';
import { GrafanaContext } from 'app/core/context/GrafanaContext';
import { createErrorNotification } from 'app/core/copy/appNotification';
import { getKioskMode } from 'app/core/navigation/kiosk';
import { GrafanaRouteComponentProps } from 'app/core/navigation/types';
@@ -96,6 +97,8 @@ export interface State {
}
export class UnthemedDashboardPage extends PureComponent<Props, State> {
static contextType = GrafanaContext;
private forceRouteReloadCounter = 0;
state: State = this.getCleanState();
@@ -139,6 +142,7 @@ export class UnthemedDashboardPage extends PureComponent<Props, State> {
routeName: this.props.route.routeName,
fixUrl: !isPublic,
accessToken: match.params.accessToken,
keybindingSrv: this.context.keybindings,
});
// small delay to start live updates
@@ -336,7 +340,7 @@ export class UnthemedDashboardPage extends PureComponent<Props, State> {
}
const inspectPanel = this.getInspectPanel();
const showSubMenu = !editPanel && kioskMode === KioskMode.Off && !this.props.queryParams.editview;
const showSubMenu = !editPanel && !kioskMode && !this.props.queryParams.editview;
const toolbar = kioskMode !== KioskMode.Full && !queryParams.editview && (
<header data-testid={selectors.pages.Dashboard.DashNav.navV2}>

View File

@@ -1,6 +1,8 @@
import { render, screen } from '@testing-library/react';
import React from 'react';
import { getGrafanaContextMock } from 'test/mocks/getGrafanaContextMock';
import { GrafanaContext } from 'app/core/context/GrafanaContext';
import { DashboardMeta, DashboardRoutes } from 'app/types';
import { getRouteComponentProps } from '../../../core/navigation/__mocks__/routeProps';
@@ -83,13 +85,22 @@ function soloPanelPageScenario(description: string, scenarioFn: (ctx: ScenarioCo
Object.assign(props, propOverrides);
ctx.dashboard = props.dashboard;
let { rerender } = render(<SoloPanelPage {...props} />);
const context = getGrafanaContextMock();
const renderPage = (props: Props) => (
<GrafanaContext.Provider value={context}>
<SoloPanelPage {...props} />
</GrafanaContext.Provider>
);
let { rerender } = render(renderPage(props));
// prop updates will be submitted by rerendering the same component with different props
ctx.rerender = (newProps?: Partial<Props>) => {
Object.assign(props, newProps);
rerender(<SoloPanelPage {...props} />);
rerender(renderPage(Object.assign(props, newProps)));
};
},
rerender: () => {
// will be replaced while mount() is called
},

View File

@@ -2,6 +2,7 @@ import React, { Component } from 'react';
import { connect, ConnectedProps } from 'react-redux';
import AutoSizer from 'react-virtualized-auto-sizer';
import { GrafanaContext } from 'app/core/context/GrafanaContext';
import { GrafanaRouteComponentProps } from 'app/core/navigation/types';
import { DashboardModel, PanelModel } from 'app/features/dashboard/state';
import { StoreState } from 'app/types';
@@ -34,6 +35,8 @@ export interface State {
}
export class SoloPanelPage extends Component<Props, State> {
static contextType = GrafanaContext;
state: State = {
panel: null,
notFound: false,
@@ -48,6 +51,7 @@ export class SoloPanelPage extends Component<Props, State> {
urlType: match.params.type,
routeName: route.routeName,
fixUrl: false,
keybindingSrv: this.context.keybindings,
});
}

View File

@@ -5,7 +5,7 @@ import { Subject } from 'rxjs';
import { FetchError, locationService, setEchoSrv } from '@grafana/runtime';
import appEvents from 'app/core/app_events';
import { getBackendSrv } from 'app/core/services/backend_srv';
import { keybindingSrv } from 'app/core/services/keybindingSrv';
import { KeybindingSrv } from 'app/core/services/keybindingSrv';
import { variableAdapters } from 'app/features/variables/adapters';
import { createConstantVariableAdapter } from 'app/features/variables/constant/adapter';
import { constantBuilder } from 'app/features/variables/shared/testing/builders';
@@ -193,6 +193,9 @@ function describeInitScenario(description: string, scenarioFn: ScenarioFn) {
urlUid: DASH_UID,
fixUrl: false,
routeName: DashboardRoutes.Normal,
keybindingSrv: {
setupDashboardBindings: jest.fn(),
} as unknown as KeybindingSrv,
},
backendSrv: getBackendSrv(),
loaderSrv,
@@ -221,8 +224,6 @@ function describeInitScenario(description: string, scenarioFn: ScenarioFn) {
};
beforeEach(async () => {
keybindingSrv.setupDashboardBindings = jest.fn();
setDashboardSrv({
setCurrent: jest.fn(),
} as any);
@@ -273,7 +274,7 @@ describeInitScenario('Initializing new dashboard', (ctx) => {
expect(getTimeSrv().init).toBeCalled();
expect(getDashboardSrv().setCurrent).toBeCalled();
expect(getDashboardQueryRunner().run).toBeCalled();
expect(keybindingSrv.setupDashboardBindings).toBeCalled();
expect(ctx.args.keybindingSrv.setupDashboardBindings).toBeCalled();
});
});
@@ -408,7 +409,7 @@ describeInitScenario('Initializing existing dashboard', (ctx) => {
expect(getTimeSrv().init).toBeCalled();
expect(getDashboardSrv().setCurrent).toBeCalled();
expect(getDashboardQueryRunner().run).toBeCalled();
expect(keybindingSrv.setupDashboardBindings).toBeCalled();
expect(ctx.args.keybindingSrv.setupDashboardBindings).toBeCalled();
});
it('Should initialize redux variables if newVariables is enabled', () => {

View File

@@ -4,7 +4,7 @@ import { notifyApp } from 'app/core/actions';
import appEvents from 'app/core/app_events';
import { createErrorNotification } from 'app/core/copy/appNotification';
import { backendSrv } from 'app/core/services/backend_srv';
import { keybindingSrv } from 'app/core/services/keybindingSrv';
import { KeybindingSrv } from 'app/core/services/keybindingSrv';
import store from 'app/core/store';
import { dashboardLoaderSrv } from 'app/features/dashboard/services/DashboardLoaderSrv';
import { DashboardSrv, getDashboardSrv } from 'app/features/dashboard/services/DashboardSrv';
@@ -32,6 +32,7 @@ export interface InitDashboardArgs {
accessToken?: string;
routeName?: string;
fixUrl: boolean;
keybindingSrv: KeybindingSrv;
}
async function fetchDashboard(
@@ -213,7 +214,7 @@ export function initDashboard(args: InitDashboardArgs): ThunkResult<void> {
dashboard.autoFitPanels(window.innerHeight, queryParams.kiosk);
}
keybindingSrv.setupDashboardBindings(dashboard);
args.keybindingSrv.setupDashboardBindings(dashboard);
} catch (err) {
if (err instanceof Error) {
dispatch(notifyApp(createErrorNotification('Dashboard init failed', err)));

View File

@@ -57,6 +57,7 @@ class WrapperUnconnected extends PureComponent<Props> {
//This is needed for breadcrumbs and topnav.
//We should probably abstract this out at some point
this.context.chrome.update({ sectionNav: this.props.navModel.node });
this.context.keybindings.setupTimeRangeBindings(false);
lastSavedUrl.left = undefined;
lastSavedUrl.right = undefined;

View File

@@ -14,7 +14,6 @@ import {
DataSourceRef,
} from '@grafana/data';
import { getDataSourceSrv } from '@grafana/runtime';
import { keybindingSrv } from 'app/core/services/keybindingSrv';
import {
DEFAULT_RANGE,
getQueryKeys,
@@ -178,8 +177,6 @@ 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