Fix Space only ever opening the navigation menu

The shortcut opened the menu but never closed it, so the only way out was
Escape or the button.

useHotkeys registers its listener once and keeps the callback it was first
given. The callback closed over the disclosure toggle, which decides its
direction from the opened value captured at that point. That value stays
false for the life of the component, so every press took the open branch
and the menu was reopened rather than closed. Instrumenting the handler
shows it firing on each press while still reporting the menu as closed,
after the component has already re-rendered with it open.

Holding the state here and toggling from the previous value removes the
dependency on the callback being current, so the shortcut works regardless.

Covered by a spec asserting the shortcut both opens and closes, and that it
closes a menu that was opened with the button. Both fail before this change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014L7R2LgjMSH2WQbiSP4uRn

apps/client/src/common/components/navigation-menu/ViewNavigationMenu.tsx

@@ -1,5 +1,5 @@
-import { useDisclosure, useHotkeys } from '@mantine/hooks';
+import { useHotkeys } from '@mantine/hooks';
-import { memo } from 'react';
+import { memo, useCallback, useState } from 'react';
 import { useSearchParams } from 'react-router';
 
 import { hasCustomParams } from '../../stores/savedViewParams';
@@ -17,17 +17,25 @@ interface ViewNavigationMenuProps {
 
 export default memo(ViewNavigationMenu);
 function ViewNavigationMenu({ isNavigationLocked, suppressSettings }: ViewNavigationMenuProps) {
-  const [isMenuOpen, menuHandler] = useDisclosure();
+  const [isMenuOpen, setIsMenuOpen] = useState(false);
   const { open: showEditFormDrawer } = useViewParamsEditorStore();
   const [searchParams] = useSearchParams();
   const hasSavedChanges = hasCustomParams(searchParams);
 
+  /**
+   * useHotkeys keeps the callback it was first given, so anything reading the open
+   * state through the closure sees it as it was on mount. Toggling from the previous
+   * value keeps the shortcut working no matter how stale the callback is.
+   */
+  const toggleMenu = useCallback(() => setIsMenuOpen((prev) => !prev), []);
+  const closeMenu = useCallback(() => setIsMenuOpen(false), []);
+
   useHotkeys([
     [
       'Space',
       () => {
         if (isNavigationLocked) return;
-        menuHandler.toggle();
+        toggleMenu();
       },
       { preventDefault: true },
     ],
@@ -48,11 +56,11 @@ function ViewNavigationMenu({ isNavigationLocked, suppressSettings }: ViewNaviga
   return (
     <>
       <FloatingNavigation
-        toggleMenu={isNavigationLocked ? undefined : menuHandler.toggle}
+        toggleMenu={isNavigationLocked ? undefined : toggleMenu}
         toggleSettings={suppressSettings ? undefined : showEditFormDrawer}
         hasSavedChanges={hasSavedChanges}
       />
-      {!isNavigationLocked && <NavigationMenu isOpen={isMenuOpen} onClose={menuHandler.close} />}
+      {!isNavigationLocked && <NavigationMenu isOpen={isMenuOpen} onClose={closeMenu} />}
     </>
   );
 }

e2e/tests/features/215-view-navigation.spec.ts

@@ -0,0 +1,34 @@
+import { expect, test } from '@playwright/test';
+
+/**
+ * The Space shortcut has to read the current open state, not the one captured when
+ * the shortcut was registered, otherwise it only ever opens the menu.
+ */
+test('Space toggles the navigation menu both ways', async ({ page }) => {
+  await page.goto('/timer');
+  // the shortcut is registered by the view chrome, wait for it before typing
+  await expect(page.getByTestId('navigation__toggle-menu')).toBeAttached();
+
+  const menu = page.getByRole('dialog');
+  await expect(menu).toBeHidden();
+
+  await page.keyboard.press(' ');
+  await expect(menu).toBeVisible();
+
+  await page.keyboard.press(' ');
+  await expect(menu).toBeHidden();
+});
+
+test('Space closes a menu opened with the button', async ({ page }) => {
+  await page.goto('/timer');
+  await expect(page.getByTestId('navigation__toggle-menu')).toBeAttached();
+
+  await page.mouse.move(Math.random() * 100, Math.random() * 100);
+  await page.getByTestId('navigation__toggle-menu').click();
+
+  const menu = page.getByRole('dialog');
+  await expect(menu).toBeVisible();
+
+  await page.keyboard.press(' ');
+  await expect(menu).toBeHidden();
+});