menu: Draw a context menu only from the trigger that was pressed - #3135
Merged
Merged
Conversation
Triggers without an `ElementId` fall back to their code location, so rows rendered from one call site share the `ContextMenu` element state. Once one row was right-clicked every row drew the same `PopupMenu`; the stacked copies shared an item's pending-click state and the covered copies cleared it on mouse up before the visible one fired, so `PopupMenuItem::on_click` never ran (#3134). The deferred menu is now wrapped in a `DeferredMenu` element that is laid out by every sharing trigger but prepainted and painted only by the one whose bounds contain the press position. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
huacnlee
enabled auto-merge (squash)
September 19, 2026 14:35
feigeCode
added a commit
to feigeCode/gpui-kit
that referenced
this pull request
Sep 20, 2026
…main Brings in the 45 upstream commits between 0.6.1 and 0.6.4 (longbridge#3071..longbridge#3135), including the QuickJS JIT bump to 0.12.9 (longbridge#3116), which overlaps our 32-bit Windows work. Conflicts and how they were resolved: - crates/shell/Cargo.toml, crates/shell/rquickjs-compat/Cargo.toml: keep the per-target split (64-bit targets get the `compiler` feature, 32-bit targets run the interpreter) and point the deps at feigeCode/quickjs-jit fix/i686-interpreter-only-0.12.9: the i686 patch rebased onto 0.12.9. - crates/base/src/input/base/element.rs: keep our gutter-lane bounds and inline widget painting alongside upstream's token_element, taking both sides of the layout and paint hunks. - crates/base/src/input/editor/display_map/*: upstream replaced the wrapped `ShapedLine` rows with `InputLine`, which carries `len` as a field and has no `split_at`. Our inline-widget layout splits the first row to reserve room for widgets, so `InputLine` gained a `split_at` (delegating to the inner `ShapedLine`, and passing a token row through whole) and the call sites read the field. - crates/component/src/input/{input,editor}.rs, button/button.rs: keep both sides' additions (`editor_style` plus `paste_handler`/`token_*`; `glyph_size` plus `content_style`/`icon_size`). - crates/component/src/dialog/dialog.rs: upstream now merges `DialogButtonProps` instead of replacing them, so `show_cancel` is an `Option<bool>` and the handlers sit behind accessors; `alert()`/`confirm()` go through the `show_cancel(..)` builder and the footer reads `is_cancel_shown()`. - Cargo.lock: taken from upstream, then re-resolved for our fork revisions. `cargo check -p gpui-base -p gpui-component -p gpui-shell -p gpui-component-shell` compiles everything except crates/component/src/inspector.rs, which is untouched by this merge: it targets gpui-pre's factory-style inspector registration for the gpui-pre fork navop patches in, while this workspace still resolves crates.io gpui-pre 0.3.5, whose registration API predates that factory.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3134.
PopupMenuItem::on_clicknever fired for aContextMenuon rows rendered from one call site without anElementId— the issue's shape (toggle_favorite(&id)per row). A single trigger, or rows with.id(..), were unaffected, which is why the gallery never showed it.Root cause
ContextMenuExt::context_menufalls back toElementId::CodeLocation(caller)when the element has no id (since #2614; before that the id was a memory address that changed every render). Sibling rows from one call site therefore share oneGlobalElementId, and GPUI'swith_element_statehands them the sameContextMenuState. After one row is right-clicked every row seesopen == trueand draws the samePopupMenuentity, stacked. The stacked copies of a menu item share itspending_mouse_downstate too; on mouse up, GPUI's capture-phase handler of a covered copy (hitboxnot hovered) takes the pending press without firing, so by the time the visible copy runs it is gone and no click is dispatched.on_mouse_downdoes not depend on cross-event state, which is why the issue's workaround works. Verified on screen against thev0.6.4tag and againstdf3b940e(before #3096): not a recent regression.The fix
The deferred menu is wrapped in a private
DeferredMenuelement. Every sharing trigger still lays it out (a deferred child must be registered during layout), but only the trigger whose bounds contain the press position prepaints and paints it —ContextMenu::prepaintsets a per-frame flag once it knows its bounds. The originaldeferred(anchored(..))construction is untouched; a trigger with its own id behaves exactly as before, since its bounds always contain the press.Verification
menu::context_menu::tests::item_click_fires_once_from_rows_without_an_id: three id-less rows, right-click the second, click the item, asserton_clickfires exactly once. Without the fix the test never settles (the stacked copies keep refreshing each other); with itmenu::context_menupasses (3 tests).Favorite clicked 0/1/2now print on every row; onv0.6.4anddf3b940eonly theon_mouse_downworkaround printed.cargo clippy -p gpui-component --all-targets -- --deny warnings,cargo fmt --check,typosclean.No public API changes:
DeferredMenuis private and the newContextMenuStatefield is private.🤖 Generated with Claude Code