Skip to content

fix(menu): consume outside presses when dismissing popup menus - #3372

Closed
luan wants to merge 1 commit into
longbridge:mainfrom
luan:luan/fix-popup-menu-outside-click
Closed

luan wants to merge 1 commit into
longbridge:mainfrom
luan:luan/fix-popup-menu-outside-click

Conversation

@luan

@luan luan commented Oct 5, 2026 •

Copy link
Copy Markdown

Description

An outside press dismissed a PopupMenu and then reached the surface beneath it. A canvas could therefore arm a command on the same click used to close the menu.

Stop propagation when the menu actually dismisses. Keep the parent-menu bounds and active-submenu exemptions so parent and child actions still work. No public API, visual styling or GPUI version changes.

The implementation change and new regression tests were generated with an AI coding agent and reviewed by that agent. Human review remains pending; the PR remains a draft.

Interaction evidence

The visuals are unchanged. Real GPUI pointer dispatch tests exercise the production window entry point and menu components:

Outside dismissal Before After
Underlying surface mouse-down count 1 0
Next fresh click — Executes once
Parent/child action with submenu open Executes once Executes once

Verified against the original main baseline 8d8cc671 (Kit 0.7.1 / GPUI 0.3.8): the unchanged menu source gives 4 passing tests and this one failure; the fix gives 5/5 passing tests in 0.418s. Scoped strict Clippy passed. The same red/green behavior was independently checked against the exact published 0.7.0 source (GPUI 0.3.7).

Before, the actual outside-press test reports:

outside_press_dismisses_without_arming_the_surface: FAIL
Left, nested=false: underlying presses = 1, expected 0

After, both new cases and the three existing menu tests pass. No static screenshot could demonstrate whether the same click also reached the underlying surface; this table and the public pointer-dispatch regression tests record that behavior directly.

How to Test

cargo nextest run --locked --no-fail-fast -j 2 -p gpui-kit --features test-support --test menu
cargo clippy --locked -j 2 -p gpui-kit --features test-support --test menu -- -D warnings

The new cases cover left/right outside dismissal, an open submenu, no underlying down/up command, focus restoration, the next fresh click, and parent/child actions executing once. The existing disabled-command, keyboard and scrollable-submenu tests remain in the same target.

Checklist

  • Read CONTRIBUTING and the canonical Design/Coding Guides.
  • Human contributor review of the AI-generated change and its public behavior tests. Agent review is complete.
  • Passed the affected native Menu Story interactions on macOS using the required signed app and accessibility-driven controls.
  • Tested macOS, Windows and Linux native platform behavior.

CI run 37291392055 tested the exact PR head 68497c7d149aa5ffe7fa2b8b26aede11cf1ebef2. Both new menu tests passed on macOS, Linux and Windows; the macOS rendering target also passed its ten Metal cases. Those automated results are separate from the native Story checks below.

All eleven check conclusions report success, but the Windows GPUI Shell Standard Runtime log contains three failing tests (llrt_pure_modules_execute_inside_the_shell_runtime, every_fs_call_settles_through_a_promise, and websocket_reads_and_writes_text_and_binary_messages). Later commands succeeded, masking those failures in the job conclusion. The same three tests fail on upstream-base run 37277918742 at 8d8cc671; they predate this PR. They are not claimed as passing.

Native Story verification

Built the exact PR source with cargo build --locked -p gpui-component-story on macOS arm64 and Linux x86_64. On macOS, launched ./script/run-story-macos Menu; the signed bundle passed codesign --verify --deep --strict. Used its accessibility tree to locate buttons and menu items and refreshed the tree after every state change.

  • Edit → click the underlying Scrollable Menu (100 items) button: the first outside click only dismisses; the next fresh click opens that menu.
  • Right-click outside the scrollable popup: it dismisses.
  • Edit → Links submenu → parent Handle Click: the complete chain dismisses and its action result appears.
  • First context region → Settings → child Item 1: the complete chain dismisses and the visible result is You have clicked info: 1.
  • With Settings open, right-click the second context region: the first press only dismisses; the next fresh right-click opens the second region's menu.
  • Real Down/Down/Return selects and activates Handle Click; Escape dismisses a context menu.

Context regions and displayed result text are absent from the native accessibility tree, so those used screenshot/coordinate fallback. Native AX reports the focused element as the window; this does not prove native semantic focus restoration. The automated tests cover focus restoration separately. The Story exited normally after verification.

On Linux, the exact built Story opened and rendered its real Menu gallery in Xvfb; its complete screenshot was opened and inspected. Linux native pointer/keyboard interactions and a Windows interactive Story session were not run. The change has no platform-specific implementation or rendering changes; the three-platform public menu tests are the automated coverage for those paths.

The focused local menu, root, and overlays targets passed all 17 tests with zero skipped, and scoped Clippy passed with -D warnings. Human contributor review remains pending, so this stays a draft.

@luan luan closed this Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant