refactor(ui): polish navigation menu and view params editor

e2e/tests/002-view-navigation.spec.ts

@@ -48,6 +48,47 @@ test.describe('test view navigation feature', () => {
     await expect(page).toHaveURL('/timer');
   });
 
+  /**
+   * The shortcut has to read the current open state, not the one captured when it was
+   * registered, otherwise it only ever opens the menu.
+   */
+  test('Space closes the menu as well as opening it', async ({ page }) => {
+    const menu = page.getByRole('dialog');
+    await expect(menu).toBeHidden();
+
+    await openNavigationMenu(page);
+    await expect(menu).toBeVisible();
+
+    await page.keyboard.press('Space');
+    await expect(menu).toBeHidden();
+  });
+
+  test('Space closes a menu opened with the button', async ({ page }) => {
+    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('Space');
+    await expect(menu).toBeHidden();
+  });
+
+  test('Space toggles a focused menu switch without closing the menu', async ({ page }) => {
+    await openNavigationMenu(page);
+
+    const menu = page.getByRole('dialog');
+    const flipScreen = page.getByRole('switch', { name: 'Flip Screen' });
+    await expect(menu).toBeVisible();
+
+    const initiallyChecked = await flipScreen.getAttribute('aria-checked');
+    await flipScreen.focus();
+    await page.keyboard.press('Space');
+
+    await expect(flipScreen).toHaveAttribute('aria-checked', initiallyChecked === 'true' ? 'false' : 'true');
+    await expect(menu).toBeVisible();
+  });
+
   test('not-found', async ({ page }) => {
     await page.goto('/not-found');
 

e2e/tests/features/206-url-preset.spec.ts

@@ -44,7 +44,7 @@ test.describe('URL Preset', () => {
       .filter({ hasText: /^testingApply$/ })
       .getByRole('button')
       .click();
-    await expect(page.getByRole('button', { name: 'Applied' })).toBeVisible();
+    await expect(page.getByText('Current')).toBeVisible();
 
     // 2. the URL contains the preset
     expect(page.url().includes('hideTimerSeconds=true')).toBeTruthy();

e2e/tests/features/207-view-params.spec.ts

@@ -13,3 +13,28 @@ test('View params configures timer view', async ({ page }) => {
   await expect(page.getByText('TIME NOW', { exact: true })).not.toBeInViewport();
   await expect(page).toHaveURL(/.*hideClock=true/);
 });
+
+/**
+ * The form gathers its values from the DOM, so a collapsed section has to stay mounted.
+ * Unmounting it would quietly drop everything the user set in it on the next apply.
+ */
+test('View params keeps the values of collapsed sections', async ({ page }) => {
+  // hideClock lives in the section we are about to collapse
+  await page.goto('/timer?hideClock=true');
+  await expect(page.getByText('TIME NOW', { exact: true })).not.toBeInViewport();
+
+  await page.mouse.move(Math.random() * 100, Math.random() * 100);
+  await page.getByTestId('navigation__toggle-settings').click();
+
+  const section = page.getByRole('button', { name: /Element visibility/ });
+  await expect(section).toHaveAttribute('aria-expanded', 'true');
+  await section.focus();
+  await page.keyboard.press('Space');
+  await expect(section).toHaveAttribute('aria-expanded', 'false');
+  await expect(page.getByRole('dialog', { name: 'Ontime' })).toBeHidden();
+
+  await page.getByTestId('apply-view-params').click();
+
+  await expect(page).toHaveURL(/.*hideClock=true/);
+  await expect(page.getByText('TIME NOW', { exact: true })).not.toBeInViewport();
+});