fix(ui): dismiss stale topbar menus and restore focus (#103508)

2026-07-19 08:01:40 +00:00 · 2026-07-10 08:33:14 +01:00
parent d133f28cfb
commit 66bcfeac6b
2 changed files with 168 additions and 8 deletions
--- a/ui/src/components/app-topbar.ts
+++ b/ui/src/components/app-topbar.ts
@@ -134,8 +134,7 @@ class AppTopbar extends OpenClawLightDomContentsElement {
      x: Math.max(8, Math.min(rect.left, window.innerWidth - menuWidth - 8)),
      y: rect.bottom + 6,
    };
-    document.addEventListener("pointerdown", this.handleDocumentPointerDown, true);
-    document.addEventListener("keydown", this.handleDocumentKeydown, true);
+    this.listenForDismissal();
    void this.updateComplete.then(() => {
      this.querySelector<HTMLElement>(".topbar-menu .topbar-menu__item")?.focus();
    });
@@ -145,7 +144,7 @@ class AppTopbar extends OpenClawLightDomContentsElement {
    const trigger = this.moreMenuTrigger;
    this.moreMenuTrigger = null;
    this.moreMenuPosition = null;
-    this.syncDocumentListeners();
+    this.syncDismissListeners();
    if (options.restoreFocus) {
      trigger?.focus();
    }
@@ -160,8 +159,7 @@ class AppTopbar extends OpenClawLightDomContentsElement {
      x: Math.max(8, Math.min(x, window.innerWidth - menuWidth - 8)),
      y: Math.max(8, Math.min(y, window.innerHeight - menuMaxHeight - 8)),
    };
-    document.addEventListener("pointerdown", this.handleDocumentPointerDown, true);
-    document.addEventListener("keydown", this.handleDocumentKeydown, true);
+    this.listenForDismissal();
    void this.updateComplete.then(() => {
      this.querySelector<HTMLElement>(".sidebar-customize-menu__item")?.focus();
    });
@@ -171,20 +169,34 @@ class AppTopbar extends OpenClawLightDomContentsElement {
    const trigger = this.customizeMenuTrigger;
    this.customizeMenuTrigger = null;
    this.customizeMenuPosition = null;
-    this.syncDocumentListeners();
+    this.syncDismissListeners();
    if (options.restoreFocus) {
      trigger?.focus();
    }
  }

-  private syncDocumentListeners() {
+  private listenForDismissal() {
+    document.addEventListener("pointerdown", this.handleDocumentPointerDown, true);
+    document.addEventListener("keydown", this.handleDocumentKeydown, true);
+    window.addEventListener("resize", this.handleWindowResize);
+  }
+
+  private syncDismissListeners() {
    if (this.moreMenuPosition || this.customizeMenuPosition) {
      return;
    }
    document.removeEventListener("pointerdown", this.handleDocumentPointerDown, true);
    document.removeEventListener("keydown", this.handleDocumentKeydown, true);
+    window.removeEventListener("resize", this.handleWindowResize);
  }

+  private readonly handleWindowResize = () => {
+    // Fixed-position menus are anchored to desktop-only controls. Close them
+    // whenever that geometry changes so an overlay cannot outlive its anchor.
+    this.closeMoreMenu();
+    this.closeCustomizeMenu();
+  };
+
  private readonly handleDocumentPointerDown = (event: PointerEvent) => {
    const path = event.composedPath();
    if (this.moreMenuTrigger && path.includes(this.moreMenuTrigger)) {
@@ -200,12 +212,51 @@ class AppTopbar extends OpenClawLightDomContentsElement {

  private readonly handleDocumentKeydown = (event: KeyboardEvent) => {
    if (event.key === "Escape") {
+      event.preventDefault();
      event.stopPropagation();
      this.closeMoreMenu({ restoreFocus: true });
      this.closeCustomizeMenu({ restoreFocus: true });
+      return;
    }
+    if (event.key === "Tab") {
+      // Menu items stay outside the page tab order. Restore the durable trigger
+      // before the browser performs its normal forward/backward Tab movement.
+      this.closeMoreMenu({ restoreFocus: true });
+      this.closeCustomizeMenu({ restoreFocus: true });
+      return;
+    }
+    this.moveMenuFocus(event);
  };

+  private moveMenuFocus(event: KeyboardEvent) {
+    const menu = this.querySelector<HTMLElement>(".topbar-menu, .sidebar-customize-menu");
+    if (!menu) {
+      return;
+    }
+    const items = Array.from(
+      menu.querySelectorAll<HTMLElement>('[role="menuitem"], [role="menuitemcheckbox"]'),
+    );
+    if (items.length === 0) {
+      return;
+    }
+    const activeIndex = items.indexOf(document.activeElement as HTMLElement);
+    let nextIndex: number;
+    if (event.key === "ArrowDown") {
+      nextIndex = (activeIndex + 1) % items.length;
+    } else if (event.key === "ArrowUp") {
+      nextIndex = activeIndex <= 0 ? items.length - 1 : activeIndex - 1;
+    } else if (event.key === "Home") {
+      nextIndex = 0;
+    } else if (event.key === "End") {
+      nextIndex = items.length - 1;
+    } else {
+      return;
+    }
+    event.preventDefault();
+    event.stopPropagation();
+    items[nextIndex]?.focus();
+  }
+
  private readonly openCustomizeMenuFromContext = (event: MouseEvent) => {
    event.preventDefault();
    this.openCustomizeMenu(event.clientX, event.clientY);
@@ -237,6 +288,7 @@ class AppTopbar extends OpenClawLightDomContentsElement {
        href=${href}
        class=${classes}
        role=${menuItem ? "menuitem" : nothing}
+        tabindex=${menuItem ? "-1" : nothing}
        aria-current=${active ? "page" : nothing}
        @focus=${(event: Event) => this.preloadRoute(routeId, event)}
        @blur=${this.cancelPreload}
@@ -273,6 +325,7 @@ class AppTopbar extends OpenClawLightDomContentsElement {
        href=${href}
        class="topbar-menu__item ${active ? "topbar-menu__item--active" : ""}"
        role="menuitem"
+        tabindex="-1"
        @click=${(event: MouseEvent) => {
          if (!shouldHandleNavigationClick(event)) {
            return;
@@ -306,6 +359,7 @@ class AppTopbar extends OpenClawLightDomContentsElement {
        <a
          class="topbar-menu__item"
          role="menuitem"
+          tabindex="-1"
          href="https://docs.openclaw.ai"
          target=${EXTERNAL_LINK_TARGET}
          rel=${buildExternalLinkRel()}
@@ -318,10 +372,11 @@ class AppTopbar extends OpenClawLightDomContentsElement {
          type="button"
          class="topbar-menu__item"
          role="menuitem"
+          tabindex="-1"
          @click=${(event: MouseEvent) => {
            const trigger = event.currentTarget as HTMLElement;
            const rect = trigger.getBoundingClientRect();
-            this.openCustomizeMenu(rect.left, rect.top);
+            this.openCustomizeMenu(rect.left, rect.top, this.moreMenuTrigger);
          }}
        >
          <span class="nav-item__icon" aria-hidden="true">${icons.penLine}</span>
@@ -351,6 +406,7 @@ class AppTopbar extends OpenClawLightDomContentsElement {
              type="button"
              class="sidebar-customize-menu__item"
              role="menuitemcheckbox"
+              tabindex="-1"
              aria-checked=${String(pinned)}
              @click=${() => this.togglePinnedRoute(routeId)}
            >
@@ -369,6 +425,7 @@ class AppTopbar extends OpenClawLightDomContentsElement {
          type="button"
          class="sidebar-customize-menu__item"
          role="menuitem"
+          tabindex="-1"
          @click=${() => {
            this.onUpdatePinnedRoutes?.([...DEFAULT_SIDEBAR_PINNED_ROUTES]);
            this.closeCustomizeMenu({ restoreFocus: true });
--- a/ui/src/e2e/sidebar-customization.e2e.test.ts
+++ b/ui/src/e2e/sidebar-customization.e2e.test.ts
@@ -63,6 +63,18 @@ async function holdUiProof(page: Page, durationMs = 600) {
  }
 }

+async function openTopbarTestPage() {
+  const context = await browser.newContext({
+    locale: "en-US",
+    serviceWorkers: "block",
+    viewport: { height: 900, width: 1440 },
+  });
+  const page = await context.newPage();
+  await installMockGateway(page);
+  await page.goto(`${server.baseUrl}overview`);
+  return { context, page };
+}
+
 describeControlUiE2e("Control UI sidebar customization mocked Gateway E2E", () => {
  beforeAll(async () => {
    if (!chromiumAvailable) {
@@ -446,4 +458,95 @@ describeControlUiE2e("Control UI sidebar customization mocked Gateway E2E", () =
      await context.close();
    }
  });
+
+  it("dismisses the desktop More menu when the viewport enters drawer layout", async () => {
+    const { context, page } = await openTopbarTestPage();
+
+    try {
+      const moreButton = page.locator(".topbar-nav").getByRole("button", { name: "More" });
+      await moreButton.click();
+      await expect.poll(() => page.locator(".topbar-menu").count()).toBe(1);
+      await captureUiProof(page, "07-topbar-menu-open.png");
+
+      await page.setViewportSize({ height: 900, width: 900 });
+      await expect.poll(() => page.locator(".topbar-menu").count()).toBe(0);
+      await captureUiProof(page, "08-drawer-with-menu-dismissed.png");
+
+      await page.setViewportSize({ height: 900, width: 1440 });
+      await expect.poll(() => moreButton.isVisible()).toBe(true);
+      await moreButton.click();
+      await page.getByRole("menuitem", { name: "Edit pinned items" }).click();
+      await expect.poll(() => page.locator(".sidebar-customize-menu").count()).toBe(1);
+      await page.setViewportSize({ height: 900, width: 900 });
+      await expect.poll(() => page.locator(".sidebar-customize-menu").count()).toBe(0);
+    } finally {
+      await context.close();
+    }
+  });
+
+  it("restores focus to More after closing the pin editor with Escape", async () => {
+    const { context, page } = await openTopbarTestPage();
+
+    try {
+      const moreButton = page.locator(".topbar-nav").getByRole("button", { name: "More" });
+      await moreButton.click();
+      await page.getByRole("menuitem", { name: "Edit pinned items" }).click();
+      const pinItems = page
+        .getByRole("menu", { name: "Edit pinned items" })
+        .locator('[role="menuitem"], [role="menuitemcheckbox"]');
+      await page.keyboard.press("End");
+      await expect
+        .poll(() => pinItems.last().evaluate((element) => element === document.activeElement))
+        .toBe(true);
+      await page.keyboard.press("Home");
+      await expect
+        .poll(() => pinItems.first().evaluate((element) => element === document.activeElement))
+        .toBe(true);
+      await page.keyboard.press("Escape");
+
+      await expect.poll(() => page.locator(".sidebar-customize-menu").count()).toBe(0);
+      await expect
+        .poll(() => moreButton.evaluate((element) => element === document.activeElement))
+        .toBe(true);
+    } finally {
+      await context.close();
+    }
+  });
+
+  it("moves focus through the More menu with arrow keys", async () => {
+    const { context, page } = await openTopbarTestPage();
+
+    try {
+      await page.locator(".topbar-nav").getByRole("button", { name: "More" }).click();
+      const menuItems = page.locator(".topbar-menu").getByRole("menuitem");
+      await expect
+        .poll(() => menuItems.evaluateAll((items) => items.every((item) => item.tabIndex === -1)))
+        .toBe(true);
+      await expect
+        .poll(() => menuItems.first().evaluate((element) => element === document.activeElement))
+        .toBe(true);
+
+      await page.keyboard.press("ArrowDown");
+      await expect
+        .poll(() => menuItems.nth(1).evaluate((element) => element === document.activeElement))
+        .toBe(true);
+      await page.keyboard.press("End");
+      await expect
+        .poll(() => menuItems.last().evaluate((element) => element === document.activeElement))
+        .toBe(true);
+      await page.keyboard.press("ArrowDown");
+      await expect
+        .poll(() => menuItems.first().evaluate((element) => element === document.activeElement))
+        .toBe(true);
+      await page.keyboard.press("Tab");
+      await expect.poll(() => page.locator(".topbar-menu").count()).toBe(0);
+      await expect
+        .poll(() =>
+          page.locator(".topbar-search").evaluate((element) => element === document.activeElement),
+        )
+        .toBe(true);
+    } finally {
+      await context.close();
+    }
+  });
 });