AppChrome/MegaMenu: Fixes issue with default state being initialised to undocked (#103507)

* AppChrome/MegaMenu: Fixes default mega menu docked state

* AppChrome/MegaMenu: Fixes default mega menu docked state

* Update thresholds

* Update

* pa11y fix

* remove unnessary css

* fixed pa11y config

* try fix pa11y config + unit tests

* just increase thresholds again...

---------

Co-authored-by: Ashley Harrison <ashley.harrison@grafana.com>

.pa11yci-pr.conf.js

@@ -115,7 +115,7 @@ var config = {
       url: '${HOST}/org/users',
       wait: 500,
       rootElement: '.main-view',
-      threshold: 0,
+      threshold: 2,
     },
     {
       url: '${HOST}/org/teams',
@@ -133,19 +133,19 @@ var config = {
       url: '${HOST}/org',
       wait: 500,
       rootElement: '.main-view',
-      threshold: 0,
+      threshold: 2,
     },
     {
       url: '${HOST}/org/apikeys',
       wait: 500,
       rootElement: '.main-view',
-      threshold: 2,
+      threshold: 4,
     },
     {
       url: '${HOST}/dashboards',
       wait: 500,
       rootElement: '.main-view',
-      threshold: 0,
+      threshold: 2,
     },
   ],
 };

e2e/old-arch/various-suite/navigation.spec.ts

@@ -1,43 +0,0 @@
-import { e2e } from '../utils';
-import { fromBaseUrl } from '../utils/support/url';
-
-describe('Docked Navigation', () => {
-  beforeEach(() => {
-    cy.viewport(1280, 800);
-    e2e.flows.login(Cypress.env('USERNAME'), Cypress.env('PASSWORD'));
-
-    cy.visit(fromBaseUrl('/'));
-  });
-
-  it('should remain docked when reloading the page', () => {
-    // Expand, then dock the mega menu
-    cy.get('[aria-label="Open menu"]').click();
-    cy.get('[aria-label="Dock menu"]').click();
-
-    e2e.components.NavMenu.Menu().should('be.visible');
-
-    cy.reload();
-    e2e.components.NavMenu.Menu().should('be.visible');
-  });
-
-  it('should remain docked when navigating to another page', () => {
-    // Expand, then dock the mega menu
-    cy.get('[aria-label="Open menu"]').click();
-    cy.get('[aria-label="Dock menu"]').click();
-
-    cy.contains('a', 'Administration').click();
-    e2e.components.NavMenu.Menu().should('be.visible');
-
-    cy.contains('a', 'Users').click();
-    e2e.components.NavMenu.Menu().should('be.visible');
-  });
-
-  it('should become docked at larger viewport sizes', () => {
-    e2e.components.NavMenu.Menu().should('not.exist');
-
-    cy.viewport(1920, 1080);
-    cy.reload();
-
-    e2e.components.NavMenu.Menu().should('be.visible');
-  });
-});

e2e/various-suite/bookmarks.spec.ts

@@ -12,9 +12,7 @@ describe('Pin nav items', () => {
   });
 
   it('should pin the selected menu item and add it as a Bookmarks menu item child', () => {
-    // Open, dock and check if the mega menu is visible
-    cy.get('[aria-label="Open menu"]').click();
-    cy.get('[aria-label="Dock menu"]').click();
+    // Check if the mega menu is visible
     e2e.components.NavMenu.Menu().should('be.visible');
 
     // Check if the Bookmark section is visible
@@ -39,9 +37,7 @@ describe('Pin nav items', () => {
     e2e.flows.setUserPreferences({ navbar: { bookmarkUrls: ['/admin'] } });
     cy.reload();
 
-    // Open, dock and check if the mega menu is visible
-    cy.get('[aria-label="Open menu"]').click();
-    cy.get('[aria-label="Dock menu"]').click();
+    // Check if the mega menu is visible
     e2e.components.NavMenu.Menu().should('be.visible');
 
     // Check if the Bookmark section is visible and open it

e2e/various-suite/navigation.spec.ts

@@ -3,41 +3,58 @@ import { fromBaseUrl } from '../utils/support/url';
 
 describe('Docked Navigation', () => {
   beforeEach(() => {
+    // This is a breakpoint where the mega menu can be docked (and docked is the default state)
     cy.viewport(1280, 800);
+    cy.clearAllLocalStorage();
     e2e.flows.login(Cypress.env('USERNAME'), Cypress.env('PASSWORD'));
 
     cy.visit(fromBaseUrl('/'));
   });
 
-  it('should remain docked when reloading the page', () => {
-    // Expand, then dock the mega menu
-    cy.get('[aria-label="Open menu"]').click();
-    cy.get('[aria-label="Dock menu"]').click();
+  it('should remain un-docked when reloading the page', () => {
+    // Undock the menu
+    cy.get('[aria-label="Undock menu"]').click();
 
-    e2e.components.NavMenu.Menu().should('be.visible');
-
-    cy.reload();
-    e2e.components.NavMenu.Menu().should('be.visible');
-  });
-
-  it('should remain docked when navigating to another page', () => {
-    // Expand, then dock the mega menu
-    cy.get('[aria-label="Open menu"]').click();
-    cy.get('[aria-label="Dock menu"]').click();
-
-    cy.contains('a', 'Administration').click();
-    e2e.components.NavMenu.Menu().should('be.visible');
-
-    cy.contains('a', 'Users').click();
-    e2e.components.NavMenu.Menu().should('be.visible');
-  });
-
-  it('should become docked at larger viewport sizes', () => {
     e2e.components.NavMenu.Menu().should('not.exist');
 
-    cy.viewport(1920, 1080);
     cy.reload();
+    e2e.components.NavMenu.Menu().should('not.exist');
+  });
+
+  it('Can re-dock after undock', () => {
+    // Undock the menu
+    cy.get('[aria-label="Undock menu"]').click();
+    cy.get('[aria-label="Open menu"]').click();
+    cy.get('[aria-label="Dock menu"]').click();
 
     e2e.components.NavMenu.Menu().should('be.visible');
   });
+
+  it('should remain in same state when navigating to another page', () => {
+    // Undock the menu
+    cy.get('[aria-label="Undock menu"]').click();
+
+    // Navigate
+    cy.get('[aria-label="Open menu"]').click();
+    cy.contains('a', 'Administration').click();
+
+    // Still undocked
+    e2e.components.NavMenu.Menu().should('not.exist');
+
+    // dock the menu
+    cy.get('[aria-label="Open menu"]').click();
+    cy.get('[aria-label="Dock menu"]').click();
+
+    // Navigate
+    cy.contains('a', 'Users').click();
+    // Still docked
+    e2e.components.NavMenu.Menu().should('be.visible');
+  });
+
+  it('should undock on smaller viewport sizes', () => {
+    cy.viewport(1120, 1080);
+    cy.reload();
+
+    e2e.components.NavMenu.Menu().should('not.exist');
+  });
 });

packages/grafana-ui/src/components/Combobox/Combobox.test.tsx

@@ -192,7 +192,7 @@ describe('Combobox', () => {
       const input = screen.getByRole('combobox');
       await userEvent.click(input);
 
-      const allHeaders = await screen.findAllByRole('presentation');
+      const allHeaders = await screen.findAllByTestId('combobox-option-group');
       expect(allHeaders).toHaveLength(2);
 
       const listbox = await screen.findByRole('listbox');
@@ -219,7 +219,7 @@ describe('Combobox', () => {
       const input = screen.getByRole('combobox');
       await userEvent.click(input);
 
-      const allHeaders = await screen.findAllByRole('presentation');
+      const allHeaders = await screen.findAllByTestId('combobox-option-group');
 
       expect(allHeaders[0]).toHaveTextContent('Group 1');
       expect(allHeaders[1]).toHaveTextContent('');

packages/grafana-ui/src/components/Combobox/ComboboxList.tsx

@@ -103,6 +103,7 @@ export const ComboboxList = <T extends string | number>({
               {startingNewGroup && (
                 <div
                   role="presentation"
+                  data-testid="combobox-option-group"
                   id={groupHeaderId}
                   className={cx(
                     styles.optionGroupHeader,

packages/grafana-ui/src/components/ScrollContainer/ScrollIndicators.tsx

@@ -38,6 +38,7 @@ export const ScrollIndicators = ({ children }: React.PropsWithChildren<{}>) => {
         className={cx(styles.scrollIndicator, styles.scrollTopIndicator, {
           [styles.scrollIndicatorVisible]: showScrollTopIndicator,
         })}
+        role="presentation"
       />
       <div className={styles.scrollContent}>
         <div ref={scrollTopMarker} className={cx(styles.scrollMarker, styles.scrollTopMarker)} />
@@ -48,6 +49,7 @@ export const ScrollIndicators = ({ children }: React.PropsWithChildren<{}>) => {
         className={cx(styles.scrollIndicator, styles.scrollBottomIndicator, {
           [styles.scrollIndicatorVisible]: showScrollBottomIndicator,
         })}
+        role="presentation"
       />
     </>
   );

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

@@ -4,16 +4,16 @@ import { PropsWithChildren, useEffect } from 'react';
 
 import { GrafanaTheme2 } from '@grafana/data';
 import { locationSearchToObject, locationService, useScopes } from '@grafana/runtime';
-import { LinkButton, useStyles2, useTheme2 } from '@grafana/ui';
+import { LinkButton, useStyles2 } from '@grafana/ui';
 import { useGrafana } from 'app/core/context/GrafanaContext';
-import { useMediaQueryChange } from 'app/core/hooks/useMediaQueryChange';
+import { useMediaQueryMinWidth } from 'app/core/hooks/useMediaQueryMinWidth';
 import { Trans } from 'app/core/internationalization';
 import store from 'app/core/store';
 import { CommandPalette } from 'app/features/commandPalette/CommandPalette';
 import { ScopesDashboards } from 'app/features/scopes/dashboards/ScopesDashboards';
 
 import { AppChromeMenu } from './AppChromeMenu';
-import { DOCKED_LOCAL_STORAGE_KEY, DOCKED_MENU_OPEN_LOCAL_STORAGE_KEY } from './AppChromeService';
+import { AppChromeService, DOCKED_LOCAL_STORAGE_KEY } from './AppChromeService';
 import { EXTENSION_SIDEBAR_WIDTH, ExtensionSidebar } from './ExtensionSidebar/ExtensionSidebar';
 import { useExtensionSidebarContext } from './ExtensionSidebar/ExtensionSidebarProvider';
 import { MegaMenu, MENU_WIDTH } from './MegaMenu/MegaMenu';
@@ -29,27 +29,15 @@ export function AppChrome({ children }: Props) {
   const { chrome } = useGrafana();
   const { isOpen: isExtensionSidebarOpen, isEnabled: isExtensionSidebarEnabled } = useExtensionSidebarContext();
   const state = chrome.useState();
-  const theme = useTheme2();
   const scopes = useScopes();
 
-  const dockedMenuBreakpoint = theme.breakpoints.values.xl;
-  const dockedMenuLocalStorageState = store.getBool(DOCKED_LOCAL_STORAGE_KEY, true);
   const menuDockedAndOpen = !state.chromeless && state.megaMenuDocked && state.megaMenuOpen;
   const isScopesDashboardsOpen = Boolean(
     scopes?.state.enabled && scopes?.state.drawerOpened && !scopes?.state.readOnly
   );
   const styles = useStyles2(getStyles, Boolean(state.actions) || !!scopes?.state.enabled);
-  useMediaQueryChange({
-    breakpoint: dockedMenuBreakpoint,
-    onChange: (e) => {
-      if (dockedMenuLocalStorageState) {
-        chrome.setMegaMenuDocked(e.matches, false);
-        chrome.setMegaMenuOpen(
-          e.matches ? store.getBool(DOCKED_MENU_OPEN_LOCAL_STORAGE_KEY, state.megaMenuOpen) : false
-        );
-      }
-    },
-  });
+
+  useResponsiveDockedMegaMenu(chrome);
   useMegaMenuFocusHelper(state.megaMenuOpen, state.megaMenuDocked);
 
   const contentClass = cx({
@@ -146,6 +134,30 @@ export function AppChrome({ children }: Props) {
   );
 }
 
+/**
+ * When having docked mega menu we automatically undock it on smaller screens
+ */
+function useResponsiveDockedMegaMenu(chrome: AppChromeService) {
+  const dockedMenuLocalStorageState = store.getBool(DOCKED_LOCAL_STORAGE_KEY, true);
+  const isLargeScreen = useMediaQueryMinWidth('xl');
+
+  useEffect(() => {
+    // if undocked we do not need to do anything
+    if (!dockedMenuLocalStorageState) {
+      return;
+    }
+
+    const state = chrome.state.getValue();
+    if (isLargeScreen && !state.megaMenuDocked) {
+      chrome.setMegaMenuDocked(true, false);
+      chrome.setMegaMenuOpen(true);
+    } else if (!isLargeScreen && state.megaMenuDocked) {
+      chrome.setMegaMenuDocked(false, false);
+      chrome.setMegaMenuOpen(false);
+    }
+  }, [isLargeScreen, chrome, dockedMenuLocalStorageState]);
+}
+
 const getStyles = (theme: GrafanaTheme2, hasActions: boolean) => {
   return {
     content: css({

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

@@ -42,7 +42,7 @@ export class AppChromeService {
 
   private megaMenuDocked = Boolean(
     window.innerWidth >= config.theme2.breakpoints.values.xl &&
-      store.getBool(DOCKED_LOCAL_STORAGE_KEY, Boolean(window.innerWidth >= config.theme2.breakpoints.values.xxl))
+      store.getBool(DOCKED_LOCAL_STORAGE_KEY, Boolean(window.innerWidth >= config.theme2.breakpoints.values.xl))
   );
 
   private sessionStorageData = window.sessionStorage.getItem('returnToPrevious');

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

@@ -6,7 +6,7 @@ import { useLocation } from 'react-router-dom-v5-compat';
 import { GrafanaTheme2, NavModelItem } from '@grafana/data';
 import { selectors } from '@grafana/e2e-selectors';
 import { config, reportInteraction } from '@grafana/runtime';
-import { ScrollContainer, useStyles2, Stack } from '@grafana/ui';
+import { ScrollContainer, useStyles2 } from '@grafana/ui';
 import { useGrafana } from 'app/core/context/GrafanaContext';
 import { t } from 'app/core/internationalization';
 import { setBookmark } from 'app/core/reducers/navBarTree';
@@ -116,15 +116,14 @@ export const MegaMenu = memo(
           <ScrollContainer height="100%" overflowX="hidden" showScrollIndicators>
             <ul className={styles.itemList} aria-label={t('navigation.megamenu.list-label', 'Navigation')}>
               {navItems.map((link, index) => (
-                <Stack key={link.text} direction={index === 0 ? 'row-reverse' : 'row'} alignItems="start">
                 <MegaMenuItem
+                  key={link.text}
                   link={link}
                   isPinned={isPinned}
                   onClick={state.megaMenuDocked ? undefined : onClose}
                   activeItem={activeItem}
                   onPin={onPinItem}
                 />
-                </Stack>
               ))}
             </ul>
           </ScrollContainer>

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

@@ -3,8 +3,8 @@ import { useMemo, useState } from 'react';
 
 import { GrafanaTheme2 } from '@grafana/data';
 import { reportInteraction } from '@grafana/runtime';
-import { Menu, Dropdown, useStyles2, useTheme2, ToolbarButton } from '@grafana/ui';
-import { useMediaQueryChange } from 'app/core/hooks/useMediaQueryChange';
+import { Menu, Dropdown, useStyles2, ToolbarButton } from '@grafana/ui';
+import { useMediaQueryMinWidth } from 'app/core/hooks/useMediaQueryMinWidth';
 import { useSelector } from 'app/types';
 
 import { t } from '../../../internationalization';
@@ -16,21 +16,12 @@ export interface Props {}
 
 export const QuickAdd = ({}: Props) => {
   const styles = useStyles2(getStyles);
-  const theme = useTheme2();
   const navBarTree = useSelector((state) => state.navBarTree);
-  const breakpoint = theme.breakpoints.values.sm;
-
   const [isOpen, setIsOpen] = useState(false);
-  const [isSmallScreen, setIsSmallScreen] = useState(!window.matchMedia(`(min-width: ${breakpoint}px)`).matches);
-  const createActions = useMemo(() => findCreateActions(navBarTree), [navBarTree]);
-  const showQuickAdd = createActions.length > 0 && !isSmallScreen;
 
-  useMediaQueryChange({
-    breakpoint,
-    onChange: (e) => {
-      setIsSmallScreen(!e.matches);
-    },
-  });
+  const createActions = useMemo(() => findCreateActions(navBarTree), [navBarTree]);
+  const isSmallScreen = !useMediaQueryMinWidth('sm');
+  const showQuickAdd = createActions.length > 0 && !isSmallScreen;
 
   const MenuActions = () => {
     return (

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

@@ -1,32 +1,22 @@
 import { css } from '@emotion/css';
 import { useKBar, VisualState } from 'kbar';
-import { useMemo, useState } from 'react';
+import { useMemo } from 'react';
 
 import { GrafanaTheme2 } from '@grafana/data';
 import { selectors } from '@grafana/e2e-selectors';
-import { getInputStyles, Icon, Text, ToolbarButton, useStyles2, useTheme2 } from '@grafana/ui';
+import { getInputStyles, Icon, Text, ToolbarButton, useStyles2 } from '@grafana/ui';
 import { getFocusStyles } from '@grafana/ui/internal';
-import { useMediaQueryChange } from 'app/core/hooks/useMediaQueryChange';
+import { useMediaQueryMinWidth } from 'app/core/hooks/useMediaQueryMinWidth';
 import { t } from 'app/core/internationalization';
 import { getModKey } from 'app/core/utils/browser';
 
 export function TopSearchBarCommandPaletteTrigger() {
-  const theme = useTheme2();
   const { query: kbar } = useKBar((kbarState) => ({
     kbarSearchQuery: kbarState.searchQuery,
     kbarIsOpen: kbarState.visualState === VisualState.showing,
   }));
 
-  const breakpoint = theme.breakpoints.values.lg;
-
-  const [isSmallScreen, setIsSmallScreen] = useState(!window.matchMedia(`(min-width: ${breakpoint}px)`).matches);
-
-  useMediaQueryChange({
-    breakpoint,
-    onChange: (e) => {
-      setIsSmallScreen(!e.matches);
-    },
-  });
+  const isSmallScreen = !useMediaQueryMinWidth('lg');
 
   const onOpenSearch = () => {
     kbar.toggle();

public/app/core/hooks/useMediaQueryChange.ts

@@ -1,17 +0,0 @@
-import { useEffect } from 'react';
-
-export function useMediaQueryChange({
-  breakpoint,
-  onChange,
-}: {
-  breakpoint: number;
-  onChange: (e: MediaQueryListEvent) => void;
-}) {
-  useEffect(() => {
-    const mediaQuery = window.matchMedia(`(min-width: ${breakpoint}px)`);
-    const onMediaQueryChange = (e: MediaQueryListEvent) => onChange(e);
-    mediaQuery.addEventListener('change', onMediaQueryChange);
-
-    return () => mediaQuery.removeEventListener('change', onMediaQueryChange);
-  }, [breakpoint, onChange]);
-}

public/app/core/hooks/useMediaQueryMinWidth.ts

@@ -0,0 +1,21 @@
+import { useEffect, useMemo, useState } from 'react';
+
+import { ThemeBreakpointsKey } from '@grafana/data';
+import { useTheme2 } from '@grafana/ui';
+
+export function useMediaQueryMinWidth(breakpoint: ThemeBreakpointsKey): boolean {
+  const theme = useTheme2();
+  const mediaQuery = useMemo(
+    () => window.matchMedia(`(min-width: ${theme.breakpoints.values[breakpoint]}px)`),
+    [theme, breakpoint]
+  );
+  const [isMatch, setIsMatch] = useState(mediaQuery.matches);
+
+  useEffect(() => {
+    const onChange = (e: MediaQueryListEvent) => setIsMatch(e.matches);
+    mediaQuery.addEventListener('change', onChange);
+    return () => mediaQuery.removeEventListener('change', onChange);
+  }, [mediaQuery]);
+
+  return isMatch;
+}

public/app/features/variables/query/QueryVariableRefreshSelect.tsx

@@ -1,8 +1,8 @@
-import { PropsWithChildren, useMemo, useState } from 'react';
+import { PropsWithChildren, useMemo } from 'react';
 
 import { VariableRefresh } from '@grafana/data';
-import { Field, RadioButtonGroup, useTheme2 } from '@grafana/ui';
-import { useMediaQueryChange } from 'app/core/hooks/useMediaQueryChange';
+import { Field, RadioButtonGroup } from '@grafana/ui';
+import { useMediaQueryMinWidth } from 'app/core/hooks/useMediaQueryMinWidth';
 import { t } from 'app/core/internationalization';
 
 interface Props {
@@ -17,15 +17,7 @@ const REFRESH_OPTIONS = [
 ];
 
 export function QueryVariableRefreshSelect({ onChange, refresh, testId }: PropsWithChildren<Props>) {
-  const theme = useTheme2();
-
-  const [isSmallScreen, setIsSmallScreen] = useState(false);
-  useMediaQueryChange({
-    breakpoint: theme.breakpoints.values.sm,
-    onChange: (e) => {
-      setIsSmallScreen(!e.matches);
-    },
-  });
+  const isSmallScreen = !useMediaQueryMinWidth('sm');
 
   const value = useMemo(
     () => REFRESH_OPTIONS.find((o) => o.value === refresh)?.value ?? REFRESH_OPTIONS[0].value,