AppChrome: Unify logic for chromeless pages that should not have NavBar, CommandPalette, Search etc (#62281)

* Keybindings: No global keybindings on chromeless pages

* simplify condition

* Refactoring

* Align name and file

* Move logic into AppChrome

* minor fix

* Update Page.tsx

* Fixing test

* Fixed tests

* More fixes

* Fixed more tests

* Fixing final test

* Fixed search in old nav

public/app/core/components/AppChrome/AppChrome.tsx

@@ -5,9 +5,12 @@ import { GrafanaTheme2 } from '@grafana/data';
 import { config } from '@grafana/runtime';
 import { useStyles2 } from '@grafana/ui';
 import { useGrafana } from 'app/core/context/GrafanaContext';
+import { CommandPalette } from 'app/features/commandPalette/CommandPalette';
+import { SearchWrapper } from 'app/features/search';
 import { KioskMode } from 'app/types';
 
 import { MegaMenu } from '../MegaMenu/MegaMenu';
+import { NavBar } from '../NavBar/NavBar';
 
 import { NavToolbar } from './NavToolbar';
 import { TopSearchBar } from './TopSearchBar';
@@ -19,9 +22,21 @@ export function AppChrome({ children }: Props) {
   const styles = useStyles2(getStyles);
   const { chrome } = useGrafana();
   const state = chrome.useState();
+  const featureToggles = config.featureToggles;
 
   if (!config.featureToggles.topnav) {
-    return <main className="main-view">{children}</main>;
+    return (
+      <>
+        {!state.chromeless && (
+          <>
+            <NavBar />
+            <SearchWrapper />
+            {featureToggles.commandPalette && <CommandPalette />}
+          </>
+        )}
+        <main className="main-view">{children}</main>
+      </>
+    );
   }
 
   const searchBarHidden = state.searchBarHidden || state.kioskMode === KioskMode.TV;
@@ -32,24 +47,33 @@ export function AppChrome({ children }: Props) {
     [styles.contentChromeless]: state.chromeless,
   });
 
+  // Chromeless routes are without topNav, mega menu, search & command palette
+  if (state.chromeless) {
+    return (
+      <main className="main-view">
+        <div className={contentClass}>{children}</div>
+      </main>
+    );
+  }
+
   return (
     <main className="main-view">
-      {!state.chromeless && (
-        <div className={cx(styles.topNav)}>
-          {!searchBarHidden && <TopSearchBar />}
-          <NavToolbar
-            searchBarHidden={searchBarHidden}
-            sectionNav={state.sectionNav}
-            pageNav={state.pageNav}
-            actions={state.actions}
-            onToggleSearchBar={chrome.onToggleSearchBar}
-            onToggleMegaMenu={chrome.onToggleMegaMenu}
-            onToggleKioskMode={chrome.onToggleKioskMode}
-          />
-        </div>
-      )}
+      <div className={cx(styles.topNav)}>
+        {!searchBarHidden && <TopSearchBar />}
+        <NavToolbar
+          searchBarHidden={searchBarHidden}
+          sectionNav={state.sectionNav}
+          pageNav={state.pageNav}
+          actions={state.actions}
+          onToggleSearchBar={chrome.onToggleSearchBar}
+          onToggleMegaMenu={chrome.onToggleMegaMenu}
+          onToggleKioskMode={chrome.onToggleKioskMode}
+        />
+      </div>
       <div className={contentClass}>{children}</div>
-      {!state.chromeless && <MegaMenu searchBarHidden={searchBarHidden} onClose={() => chrome.setMegaMenu(false)} />}
+      <MegaMenu searchBarHidden={searchBarHidden} onClose={() => chrome.setMegaMenu(false)} />
+      {featureToggles.commandPalette && <CommandPalette />}
+      {!featureToggles.topNavCommandPalette && <SearchWrapper />}
     </main>
   );
 }

public/app/core/components/AppChrome/NavLandingPage.test.tsx

@@ -1,9 +1,8 @@
 import { render, screen } from '@testing-library/react';
 import React from 'react';
-import { Provider } from 'react-redux';
+import { TestProvider } from 'test/helpers/TestProvider';
 
 import { config } from '@grafana/runtime';
-import { configureStore } from 'app/store/configureStore';
 
 import { NavLandingPage } from './NavLandingPage';
 
@@ -51,11 +50,10 @@ describe('NavLandingPage', () => {
       },
     ];
 
-    const store = configureStore();
     return render(
-      <Provider store={store}>
+      <TestProvider>
         <NavLandingPage navId={mockId} />
-      </Provider>
+      </TestProvider>
     );
   };
 

public/app/core/components/AppChrome/Organization/OrganizationSwitcher.test.tsx

@@ -1,11 +1,10 @@
 import { render, screen } from '@testing-library/react';
 import React from 'react';
-import { Provider } from 'react-redux';
+import { TestProvider } from 'test/helpers/TestProvider';
 
 import { OrgRole } from '@grafana/data';
 import { ContextSrv, setContextSrv } from 'app/core/services/context_srv';
 import { getUserOrganizations } from 'app/features/org/state/actions';
-import { configureStore } from 'app/store/configureStore';
 import * as appTypes from 'app/types';
 
 import { OrganizationSwitcher } from './OrganizationSwitcher';
@@ -22,12 +21,10 @@ jest.mock('app/types', () => ({
 }));
 
 const renderWithProvider = ({ initialState }: { initialState?: Partial<appTypes.StoreState> }) => {
-  const store = configureStore(initialState);
-
   render(
-    <Provider store={store}>
+    <TestProvider storeState={initialState}>
       <OrganizationSwitcher />
-    </Provider>
+    </TestProvider>
   );
 };
 

public/app/core/components/AppChrome/QuickAdd/QuickAdd.test.tsx

@@ -1,11 +1,10 @@
 import { render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import React from 'react';
-import { Provider } from 'react-redux';
+import { TestProvider } from 'test/helpers/TestProvider';
 
 import { NavModelItem, NavSection } from '@grafana/data';
 import { reportInteraction } from '@grafana/runtime';
-import { configureStore } from 'app/store/configureStore';
 
 import { QuickAdd } from './QuickAdd';
 
@@ -37,12 +36,10 @@ const setup = () => {
     },
   ];
 
-  const store = configureStore({ navBarTree });
-
   return render(
-    <Provider store={store}>
+    <TestProvider storeState={{ navBarTree }}>
       <QuickAdd />
-    </Provider>
+    </TestProvider>
   );
 };
 

public/app/core/components/MegaMenu/MegaMenu.test.tsx

@@ -1,15 +1,12 @@
 import { render, screen } from '@testing-library/react';
 import React from 'react';
-import { Provider } from 'react-redux';
 import { Router } from 'react-router-dom';
 import { getGrafanaContextMock } from 'test/mocks/getGrafanaContextMock';
 
 import { NavModelItem, NavSection } from '@grafana/data';
 import { locationService } from '@grafana/runtime';
-import { GrafanaContext } from 'app/core/context/GrafanaContext';
-import { configureStore } from 'app/store/configureStore';
 
-import TestProvider from '../../../../test/helpers/TestProvider';
+import { TestProvider } from '../../../../test/helpers/TestProvider';
 
 import { MegaMenu } from './MegaMenu';
 
@@ -33,21 +30,15 @@ const setup = () => {
     },
   ];
 
-  const context = getGrafanaContextMock();
-  const store = configureStore({ navBarTree });
-
-  context.chrome.onToggleMegaMenu();
+  const grafanaContext = getGrafanaContextMock();
+  grafanaContext.chrome.onToggleMegaMenu();
 
   return render(
-    <Provider store={store}>
-      <GrafanaContext.Provider value={context}>
-        <TestProvider>
-          <Router history={locationService.getHistory()}>
-            <MegaMenu onClose={() => {}} />
-          </Router>
-        </TestProvider>
-      </GrafanaContext.Provider>
-    </Provider>
+    <TestProvider storeState={{ navBarTree }} grafanaContext={grafanaContext}>
+      <Router history={locationService.getHistory()}>
+        <MegaMenu onClose={() => {}} />
+      </Router>
+    </TestProvider>
   );
 };
 

public/app/core/components/NavBar/NavBar.test.tsx

@@ -1,12 +1,9 @@
 import { render, screen } from '@testing-library/react';
 import React from 'react';
-import { Provider } from 'react-redux';
-import { Router } from 'react-router-dom';
 
 import { locationService } from '@grafana/runtime';
-import { configureStore } from 'app/store/configureStore';
 
-import TestProvider from '../../../../test/helpers/TestProvider';
+import { TestProvider } from '../../../../test/helpers/TestProvider';
 
 import { NavBar } from './NavBar';
 
@@ -22,16 +19,10 @@ jest.mock('app/core/services/context_srv', () => ({
 }));
 
 const setup = () => {
-  const store = configureStore();
-
   return render(
-    <Provider store={store}>
-      <TestProvider>
-        <Router history={locationService.getHistory()}>
-          <NavBar />
-        </Router>
-      </TestProvider>
-    </Provider>
+    <TestProvider>
+      <NavBar />
+    </TestProvider>
   );
 };
 

public/app/core/components/NavBar/NavBar.tsx

@@ -192,10 +192,6 @@ const getStyles = (theme: GrafanaTheme2) => ({
   navWrapper: css({
     position: 'relative',
     display: 'flex',
-
-    '.sidemenu-hidden &': {
-      display: 'none',
-    },
   }),
   sidemenu: css({
     label: 'sidemenu',

public/app/core/components/NavBar/NavBarItem.test.tsx

@@ -6,8 +6,6 @@ import { BrowserRouter } from 'react-router-dom';
 import { locationUtil } from '@grafana/data';
 import { config, setLocationService } from '@grafana/runtime';
 
-import TestProvider from '../../../../test/helpers/TestProvider';
-
 // Need to mock createBrowserHistory here to avoid errors
 jest.mock('history', () => ({
   ...jest.requireActual('history'),
@@ -45,18 +43,16 @@ async function getTestContext(overrides: Partial<Props> = {}, subUrl = '', isMen
   const props = { ...defaults, ...overrides };
 
   const { rerender } = render(
-    <TestProvider>
-      <BrowserRouter>
-        <NavBarContext.Provider
-          value={{
-            menuIdOpen: isMenuOpen ? props.link.id : undefined,
-            setMenuIdOpen: setMenuIdOpenMock,
-          }}
-        >
-          <NavBarItem {...props} />
-        </NavBarContext.Provider>
-      </BrowserRouter>
-    </TestProvider>
+    <BrowserRouter>
+      <NavBarContext.Provider
+        value={{
+          menuIdOpen: isMenuOpen ? props.link.id : undefined,
+          setMenuIdOpen: setMenuIdOpenMock,
+        }}
+      >
+        <NavBarItem {...props} />
+      </NavBarContext.Provider>
+    </BrowserRouter>
   );
 
   // Need to click this first to set the correct selection range

public/app/core/components/Page/Page.test.tsx

@@ -1,10 +1,9 @@
 import { render, screen } from '@testing-library/react';
 import React from 'react';
-import { Provider } from 'react-redux';
+import { TestProvider } from 'test/helpers/TestProvider';
 
 import { NavModelItem } from '@grafana/data';
 import { config } from '@grafana/runtime';
-import { configureStore } from 'app/store/configureStore';
 
 import { Page } from './Page';
 import { PageProps } from './types';
@@ -30,14 +29,12 @@ const setup = (props: Partial<PageProps>) => {
     },
   ];
 
-  const store = configureStore();
-
   return render(
-    <Provider store={store}>
+    <TestProvider>
       <Page {...props}>
         <div data-testid="page-children">Children</div>
       </Page>
-    </Provider>
+    </TestProvider>
   );
 };
 

public/app/core/components/Page/Page.tsx

@@ -1,10 +1,11 @@
 // Libraries
 import { css, cx } from '@emotion/css';
-import React from 'react';
+import React, { useEffect } from 'react';
 
 import { GrafanaTheme2, PageLayoutType } from '@grafana/data';
 import { config } from '@grafana/runtime';
 import { CustomScrollbar, useStyles2 } from '@grafana/ui';
+import { useGrafana } from 'app/core/context/GrafanaContext';
 
 import { Footer } from '../Footer/Footer';
 import { PageHeader } from '../PageHeader/PageHeader';
@@ -34,11 +35,21 @@ export const OldPage: PageType = ({
 }) => {
   const styles = useStyles2(getStyles);
   const navModel = usePageNav(navId, oldNavProp);
+  const { chrome } = useGrafana();
 
   usePageTitle(navModel, pageNav);
 
   const pageHeaderNav = pageNav ?? navModel?.main;
 
+  useEffect(() => {
+    if (navModel) {
+      // This is needed for chrome to update it's chromeless state
+      chrome.update({
+        sectionNav: navModel.node,
+      });
+    }
+  }, [navModel, chrome]);
+
   return (
     <div className={cx(styles.wrapper, className)} {...otherProps}>
       {layout === PageLayoutType.Standard && (

public/app/core/components/PageNew/Page.test.tsx

@@ -1,13 +1,11 @@
 import { render, screen } from '@testing-library/react';
 import React from 'react';
-import { Provider } from 'react-redux';
+import { TestProvider } from 'test/helpers/TestProvider';
 import { getGrafanaContextMock } from 'test/mocks/getGrafanaContextMock';
 
 import { NavModelItem } from '@grafana/data';
 import { config } from '@grafana/runtime';
-import { GrafanaContext } from 'app/core/context/GrafanaContext';
 import { HOME_NAV_ID } from 'app/core/reducers/navModel';
-import { configureStore } from 'app/store/configureStore';
 
 import { PageProps } from '../Page/types';
 
@@ -38,16 +36,13 @@ const setup = (props: Partial<PageProps>) => {
   ];
 
   const context = getGrafanaContextMock();
-  const store = configureStore();
 
   const renderResult = render(
-    <Provider store={store}>
-      <GrafanaContext.Provider value={context}>
-        <Page {...props}>
-          <div data-testid="page-children">Children</div>
-        </Page>
-      </GrafanaContext.Provider>
-    </Provider>
+    <TestProvider grafanaContext={context}>
+      <Page {...props}>
+        <div data-testid="page-children">Children</div>
+      </Page>
+    </TestProvider>
   );
 
   return { renderResult, context };

public/app/core/components/SharedPreferences/SharedPreferences.test.tsx

@@ -1,7 +1,6 @@
 import { render, screen, waitFor } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import React from 'react';
-import TestProvider from 'test/helpers/TestProvider';
 import { assertInstanceOf } from 'test/helpers/asserts';
 import { getSelectParent, selectOptionInTest } from 'test/helpers/selectOptionInTest';
 
@@ -124,11 +123,7 @@ describe('SharedPreferences', () => {
     mockReload.mockReset();
     mockPrefsUpdate.mockReset();
 
-    render(
-      <TestProvider>
-        <SharedPreferences {...props} />
-      </TestProvider>
-    );
+    render(<SharedPreferences {...props} />);
 
     await waitFor(() => expect(mockPrefsLoad).toHaveBeenCalled());
   });

public/app/core/navigation/GrafanaRoute.test.tsx

@@ -1,19 +1,14 @@
 import { render, screen } from '@testing-library/react';
 import React, { ComponentType } from 'react';
-import { Provider } from 'react-redux';
-import { BrowserRouter } from 'react-router-dom';
-import { getGrafanaContextMock } from 'test/mocks/getGrafanaContextMock';
+import { TestProvider } from 'test/helpers/TestProvider';
 
 import { setEchoSrv } from '@grafana/runtime';
 
-import { configureStore } from '../../store/configureStore';
-import { GrafanaContext } from '../context/GrafanaContext';
 import { Echo } from '../services/echo/Echo';
 
 import { GrafanaRoute, Props } from './GrafanaRoute';
 
 function setup(overrides: Partial<Props>) {
-  const store = configureStore();
   const props: Props = {
     location: { search: '?query=hello&test=asd' } as any,
     history: {} as any,
@@ -26,13 +21,9 @@ function setup(overrides: Partial<Props>) {
   };
 
   render(
-    <BrowserRouter>
-      <GrafanaContext.Provider value={getGrafanaContextMock()}>
-        <Provider store={store}>
-          <GrafanaRoute {...props} />
-        </Provider>
-      </GrafanaContext.Provider>
-    </BrowserRouter>
+    <TestProvider>
+      <GrafanaRoute {...props} />
+    </TestProvider>
   );
 }
 

public/app/core/navigation/GrafanaRoute.tsx

@@ -19,8 +19,8 @@ export function GrafanaRoute(props: Props) {
   chrome.setMatchedRoute(props.route);
 
   useLayoutEffect(() => {
-    keybindings.clearAndInitGlobalBindings();
-  }, [keybindings]);
+    keybindings.clearAndInitGlobalBindings(props.route);
+  }, [keybindings, props.route]);
 
   useEffect(() => {
     updateBodyClassNames(props.route);

public/app/core/services/keybindingSrv.ts

@@ -23,6 +23,7 @@ import {
 import { AppChromeService } from '../components/AppChrome/AppChromeService';
 import { HelpModal } from '../components/help/HelpModal';
 import { contextSrv } from '../core';
+import { RouteDescriptor } from '../navigation/types';
 
 import { toggleTheme } from './theme';
 import { withFocusedPanel } from './withFocusedPanelId';
@@ -30,10 +31,11 @@ import { withFocusedPanel } from './withFocusedPanelId';
 export class KeybindingSrv {
   constructor(private locationService: LocationService, private chromeService: AppChromeService) {}
 
-  clearAndInitGlobalBindings() {
+  clearAndInitGlobalBindings(route: RouteDescriptor) {
     Mousetrap.reset();
 
-    if (this.locationService.getLocation().pathname !== '/login') {
+    // Chromeless pages like login and signup page don't get any global bindings
+    if (!route.chromeless) {
       this.bind(['?', 'h'], this.showHelpModal);
       this.bind('g h', this.goToHome);
       this.bind('g a', this.openAlerting);