You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by as...@apache.org on 2026/09/07 08:37:15 UTC

(maka) branch main updated: fix(ui): focus sidebar rename dialogs

This is an automated email from the ASF dual-hosted git repository.

Astro-Han pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/maka.git


The following commit(s) were added to refs/heads/main by this push:
     new 11575bfe1b fix(ui): focus sidebar rename dialogs
11575bfe1b is described below

commit 11575bfe1b3da5be3041c3d55d49eb011efe923b
Author: Sun-GLiang <81...@users.noreply.github.com>
AuthorDate: Mon Sep 7 16:37:11 2026 +0800

    fix(ui): focus sidebar rename dialogs
    
    Open sidebar task and project rename dialogs synchronously from their menu actions. Focus and select the shared Astryx input when the dialog mounts, with one guarded frame retry for the menu/native dialog focus handoff.
    
    The native Electron regression covers menu and double-click entry, replacing the selected old name, and Escape focus restoration. No protocol or storage migration.
    
    Fixes #4828
    
    Generated-by: Codex
---
 apps/desktop/e2e-budget.json              |  4 ++
 apps/desktop/e2e/fixtures.ts              | 12 ++++
 apps/desktop/e2e/rename-focus.spec.ts     | 97 +++++++++++++++++++++++++++++++
 packages/ui/src/session-history-list.tsx  | 53 +++++------------
 packages/ui/src/session-rename-dialog.tsx | 23 +++++++-
 5 files changed, 149 insertions(+), 40 deletions(-)

diff --git a/apps/desktop/e2e-budget.json b/apps/desktop/e2e-budget.json
index 0f1eeb29a9..4a284abc9b 100644
--- a/apps/desktop/e2e-budget.json
+++ b/apps/desktop/e2e-budget.json
@@ -29,6 +29,10 @@
       "tests": 1,
       "electron": "a pointer capture released outside the window -- there is no window edge to leave in a browser tab"
     },
+    "rename-focus.spec.ts": {
+      "tests": 1,
+      "electron": "native Electron keyboard events must reach the rename field immediately after menu dismissal or a row double-click"
+    },
     "session-draft-focus.spec.ts": {
       "tests": 1,
       "electron": "needs a second real Session (Host round trip) to switch to; the focus and draft-restore halves alone would not earn a window"
diff --git a/apps/desktop/e2e/fixtures.ts b/apps/desktop/e2e/fixtures.ts
index e19d1ec32d..03859a084d 100644
--- a/apps/desktop/e2e/fixtures.ts
+++ b/apps/desktop/e2e/fixtures.ts
@@ -520,6 +520,7 @@ type E2eTestFixtures = {
   gitReviewWindow: { page: Page; projectRoot: string };
   invocableSkillsWindow: Page;
   projectSidebarWindow: Page;
+  renameFocusWindow: { page: Page; app: ElectronApplication };
   parentRemovalWindow: Page;
   railRenderWindow: Page;
   promptRailWindow: Page;
@@ -600,6 +601,17 @@ export const test = base.extend<E2eTestFixtures>({
       locale: 'zh-CN',
     }, use);
   },
+  // Visible because the contract under test is native keyboard delivery into
+  // the focused renderer element, not Playwright's synthetic page keyboard.
+  renameFocusWindow: async ({}, use) => {
+    await withE2eWindow({
+      seed: false,
+      readinessSelector: '[data-maka-contract="search-modal"][open]',
+      e2eFixtureScenario: 'sidebar-search-modal-open',
+      locale: 'zh-CN',
+      showWindow: true,
+    }, async (page, { app }) => use({ page, app }));
+  },
   parentRemovalWindow: async ({}, use) => {
     await withE2eWindow(
       {
diff --git a/apps/desktop/e2e/rename-focus.spec.ts b/apps/desktop/e2e/rename-focus.spec.ts
new file mode 100644
index 0000000000..e9a0605561
--- /dev/null
+++ b/apps/desktop/e2e/rename-focus.spec.ts
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import type { ElectronApplication, Locator } from '@playwright/test';
+import { expect, test } from './fixtures';
+
+async function focusWindow(app: ElectronApplication): Promise<void> {
+  await app.evaluate(({ BrowserWindow }) => {
+    const window = BrowserWindow.getAllWindows()[0];
+    if (!window) throw new Error('Maka window is missing');
+    window.show();
+    window.focus();
+    window.webContents.focus();
+  });
+}
+
+async function sendNativeText(app: ElectronApplication, value: string): Promise<void> {
+  await app.evaluate(({ BrowserWindow }, text) => {
+    const window = BrowserWindow.getAllWindows()[0];
+    if (!window) throw new Error('Maka window is missing');
+    for (const character of text) {
+      window.webContents.sendInputEvent({ type: 'char', keyCode: character });
+    }
+  }, value);
+}
+
+async function expectSelected(input: Locator): Promise<void> {
+  await expect
+    .poll(() => input.evaluate((element: HTMLInputElement) => ({
+      start: element.selectionStart,
+      end: element.selectionEnd,
+      length: element.value.length,
+    })))
+    .toEqual(expect.objectContaining({ start: 0 }));
+  await expect
+    .poll(() => input.evaluate((element: HTMLInputElement) => element.selectionEnd))
+    .toBe(await input.inputValue().then((value) => value.length));
+}
+
+test('rename inputs own native focus for menu and double-click entry', async ({
+  renameFocusWindow: { page, app },
+}) => {
+  await focusWindow(app);
+  await page.keyboard.press('Escape');
+  await expect(page.locator('[data-maka-contract="search-modal"]')).not.toBeVisible();
+
+  const sidebar = page.getByRole('navigation', { name: '任务列表' });
+  const rowButton = sidebar.getByRole('button', { name: /^任务 00 / }).first();
+  const row = rowButton.locator('..');
+  const taskActions = sidebar.getByRole('button', { name: /^任务 00 .*任务操作$/ });
+
+  await row.hover();
+  await taskActions.click();
+  await page.getByRole('menuitem', { name: '重命名', exact: true }).click();
+  await sendNativeText(app, 'MENU');
+  const taskInput = page.getByRole('textbox', { name: '重命名任务' });
+  await expect(taskInput).toHaveValue('MENU');
+  await page.keyboard.press('Escape');
+  await expect(taskActions).toBeFocused();
+
+  await rowButton.dblclick();
+  const doubleClickInput = page.getByRole('textbox', { name: '重命名任务' });
+  await expectSelected(doubleClickInput);
+  await sendNativeText(app, 'DOUBLE');
+  await expect(doubleClickInput).toHaveValue('DOUBLE');
+  await page.keyboard.press('Escape');
+  await expect(rowButton).toBeFocused();
+
+  await sidebar.getByRole('radio', { name: '按项目', exact: true }).click();
+  const projectActions = page.getByRole('button', {
+    name: '示例项目 项目操作',
+    exact: true,
+  });
+  await projectActions.click();
+  await page.getByRole('menuitem', { name: '重命名', exact: true }).click();
+  await sendNativeText(app, 'PROJECT');
+  const projectInput = page.getByRole('textbox', { name: '重命名项目' });
+  await expect(projectInput).toHaveValue('PROJECT');
+  await page.keyboard.press('Escape');
+  await expect(projectActions).toBeFocused();
+});
diff --git a/packages/ui/src/session-history-list.tsx b/packages/ui/src/session-history-list.tsx
index 9a22a1f797..02e53643a3 100644
--- a/packages/ui/src/session-history-list.tsx
+++ b/packages/ui/src/session-history-list.tsx
@@ -360,13 +360,9 @@ function SessionListGroups(props: {
    * The control the rename was started from, so focus can go back to it.
    *
    * Astryx's Dialog restores focus on its own — to whatever was focused when it
-   * opened — and that is exactly what fails here. A menu-launched dialog opens
-   * one frame AFTER the menu closed, and the two race: measured frame by frame,
-   * the dialog's capture lands on the menu item (frames 1-3) while the menu
-   * hands focus back to the trigger on frame 4. Restoring to a node that has
-   * since been unmounted is a no-op, so the edit ended on <body> and the next
-   * Tab started at the top of the window — while the delete confirm one item
-   * below in the same menu returns the trigger.
+   * opened. For a menu-launched dialog that is the menu item, which is removed
+   * as the menu closes. Restoring to that node is a no-op, so remember the
+   * stable trigger explicitly instead.
    *
    * The opener is passed in rather than captured here, because the component
    * that renders the menu is the only one that can name it without racing.
@@ -1106,7 +1102,6 @@ function ProjectItemActions(props: {
   const [pendingAction, setPendingAction] = useState<ProjectRowActionId | null>(null);
   const mountedRef = useMountedRef();
   const pendingActionRef = useRef<ProjectRowActionId | null>(null);
-  const pendingMenuIntentRef = useRef<(() => void) | null>(null);
   const project = props.project;
   const actions = props.actions;
 
@@ -1166,11 +1161,8 @@ function ProjectItemActions(props: {
           label: copy.projectRename,
           icon: Pencil,
           onClick: () => {
-            // Read now, while the trigger is still the thing the user is on:
-            // by the time the intent runs the menu has closed and focus is
-            // mid-handover.
             const opener = trailingRef.current?.querySelector<HTMLElement>('button') ?? null;
-            pendingMenuIntentRef.current = () => props.onStartRename(opener);
+            props.onStartRename(opener);
           },
         },
         {
@@ -1187,13 +1179,7 @@ function ProjectItemActions(props: {
         label={copy.projectActionsAriaLabel(project.name)}
         isDisabled={pendingAction !== null}
         isMenuOpen={menuOpen}
-        onOpenChange={(open) => {
-          setMenuOpen(open);
-          if (open) return;
-          const intent = pendingMenuIntentRef.current;
-          pendingMenuIntentRef.current = null;
-          if (intent) window.requestAnimationFrame(intent);
-        }}
+        onOpenChange={setMenuOpen}
         items={menuItems}
       />
     </span>
@@ -1233,7 +1219,6 @@ function SessionItemActions(props: {
   const [pendingAction, setPendingAction] = useState<SessionRowActionId | null>(null);
   const mountedRef = useMountedRef();
   const pendingActionRef = useRef<SessionRowActionId | null>(null);
-  const pendingMenuIntentRef = useRef<(() => void) | null>(null);
   const actions = props.actions;
 
   useEffect(
@@ -1271,13 +1256,7 @@ function SessionItemActions(props: {
         label={copy.actionsAriaLabel(actionContext)}
         isDisabled={pendingAction !== null}
         isMenuOpen={menuOpen}
-        onOpenChange={(open) => {
-          setMenuOpen(open);
-          if (open) return;
-          const intent = pendingMenuIntentRef.current;
-          pendingMenuIntentRef.current = null;
-          if (intent) window.requestAnimationFrame(intent);
-        }}
+        onOpenChange={setMenuOpen}
         items={
           props.bulkCount > 1 && props.selectionCommands
             ? [
@@ -1310,20 +1289,16 @@ function SessionItemActions(props: {
                   label: copy.rename,
                   icon: Pencil,
                   onClick: () => {
-                    // Read now, while the trigger is still the thing the user
-                    // is on: by the time the intent runs the menu has closed
-                    // and focus is mid-handover.
                     const opener =
                       trailingRef.current?.querySelector<HTMLElement>('button') ?? null;
-                    pendingMenuIntentRef.current = () =>
-                      props.onStartRename(
-                        {
-                          kind: 'session',
-                          id: props.session.id,
-                          name: props.session.name,
-                        },
-                        opener,
-                      );
+                    props.onStartRename(
+                      {
+                        kind: 'session',
+                        id: props.session.id,
+                        name: props.session.name,
+                      },
+                      opener,
+                    );
                   },
                 },
                 // Archive is where the rail stops. Deleting is the one row
diff --git a/packages/ui/src/session-rename-dialog.tsx b/packages/ui/src/session-rename-dialog.tsx
index 334ac3d553..4a8f3476bd 100644
--- a/packages/ui/src/session-rename-dialog.tsx
+++ b/packages/ui/src/session-rename-dialog.tsx
@@ -17,7 +17,7 @@
  * under the License.
  */
 
-import { useState, type FormEvent } from 'react';
+import { useEffect, useRef, useState, type FormEvent } from 'react';
 import { Button, HStack, TextInput } from '@astryxdesign/core';
 import { Dialog, DialogHeader } from '@astryxdesign/core/Dialog';
 import { Layout, LayoutContent, LayoutFooter } from '@astryxdesign/core/Layout';
@@ -62,6 +62,26 @@ export function SessionRenameDialog(props: {
   // `key` at the call site), so the seed is the name as it was when the menu
   // item was chosen, and nothing has to be synchronised while it is open.
   const [name, setName] = useState(target.name);
+  const inputRef = useRef<HTMLInputElement>(null);
+
+  useEffect(() => {
+    const input = inputRef.current;
+    if (!input) return;
+
+    const focusAndSelect = () => {
+      input.focus({ preventScroll: true });
+      input.select();
+    };
+
+    focusAndSelect();
+    const frame = window.requestAnimationFrame(() => {
+      // Closing a menu and opening a native dialog both manage focus. If
+      // either handoff wins after this effect, take ownership back once the
+      // dialog has settled.
+      if (document.activeElement !== input) focusAndSelect();
+    });
+    return () => window.cancelAnimationFrame(frame);
+  }, []);
 
   const trimmed = name.trim();
   // The row's own vocabulary: a session is 任务 everywhere the user can see
@@ -104,6 +124,7 @@ export function SessionRenameDialog(props: {
           <LayoutContent>
             <form id="maka-session-rename-form" onSubmit={submit}>
               <TextInput
+                ref={inputRef}
                 label={title}
                 // Hidden: the dialog's own header already says what is being
                 // renamed, and Astryx clips the whole label block — a


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org