Skip to content

#546: ShellApp: migrate dialog + context-menu rendering and dismissal (right-click & tab-close broken) - #559

Merged
JDonaghy merged 2 commits into
developfrom
issue-546-shellapp-migrate-dialog-context-menu-ren
Jul 5, 2026
Merged

#546: ShellApp: migrate dialog + context-menu rendering and dismissal (right-click & tab-close broken)#559
JDonaghy merged 2 commits into
developfrom
issue-546-shellapp-migrate-dialog-context-menu-ren

Conversation

@JDonaghy

@JDonaghy JDonaghy commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Closes #546

Automated merge from the coordinator for assignment f2a1fb4c8969 on issue #546.

Worker branch: issue-546-shellapp-migrate-dialog-context-menu-rendevelop.

JDonaghy and others added 2 commits July 5, 2026 18:08
GTK's ShellApp render path never painted `screen.dialog` / `screen.context_menu`
at all — their draw + click-geometry caches were only ever populated by the
dead legacy `draw_editor` Cairo path, which has zero live callers under
ShellApp. Right-click menus were invisible/unclickable, and dialogs (:about,
Unsaved-Changes on tab close, etc.) were invisible and undismissable by mouse,
freezing the editor to further clicks.

- Add `render::dialog_generic_layout` / `context_menu_generic_layout`: shared,
  backend-agnostic layout functions (char-cell approximation from just
  char_width/line_height) used by BOTH GTK and TUI, replacing four
  independently-drifting reimplementations (quadraui's own example, TUI's
  render step, TUI's click-time recompute, GTK's dead Pango version) with
  two.
- GTK `render_content` now draws the dialog/context-menu popup and caches the
  resolved `DialogLayout`/`ContextMenuLayout` into the same fields
  `handle_mouse_click_msg` already reads — no click-handler logic changed,
  it was just starved of input.
- GTK's dialog click handling now resolves buttons via
  `DialogLayout::hit_test` instead of a hand-rolled rect scan; the
  `dialog_btn_rects`/`dialog_popup_rect` cache pair collapses into one
  `dialog_layout` field, mirroring `context_menu_layout`.
- TUI's `mouse.rs` now consumes the render-cached `DialogLayout` via
  `hit_test` instead of independently recomputing dialog geometry at click
  time (the two copies had already drifted: `.len()` byte count vs
  `.chars().count()`). The now-dead `core::engine::{DialogLayout,
  DialogClickResult, resolve_dialog_click}` and their test are removed.
- Fix a second, independent gap required for the explorer acceptance
  criterion: GTK's `try_route_sidebar_mouse_event` forwards all MouseDown
  (incl. right-clicks) into `TreeController`, whose `right_click()` already
  resolves `ContextMenuRequested{path, position}` correctly — but
  `dispatch_explorer_tree_event`'s catch-all silently dropped that variant,
  so explorer right-click did nothing. Wire it to
  `engine.open_explorer_context_menu`, converting position via the same
  cached explorer metrics used for the TreeController's own hit-test.
  Editor/tab-bar right-click and TUI were unaffected.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ingle-group tab-close dialog

Fix iteration 1 addressing the failed smoke test on the ShellApp dialog/
context-menu migration.

FAILED-1 — tab-bar right-click showed the editor's menu: `UiEvent::MouseDown`
had exactly one `MouseButton::Right` branch that unconditionally built
`Msg::EditorRightClick`, regardless of where the click landed. Added
`click::resolve_tab_right_click` — a read-only mirror of
`pixel_to_click_target`'s zone resolution (no engine mutation, unlike the
left-click path) — so a right-click over a tab now resolves the actual
(group_id, tab_idx) and dispatches the existing `Msg::TabRightClick`
instead.

FAILED-2 — explorer context menu rendered but was non-interactive: opening
it sets the same `engine.context_menu` the editor/tab-bar menus use, and
render_content now draws it generically (this PR's earlier commit), but
`try_route_sidebar_mouse_event` forwarded every MouseDown in the sidebar's
bounds straight to `TreeController` with no awareness that a menu might be
open on top — so item clicks acted on the tree row underneath and outside
clicks never dismissed it. Extracted the modal-stack-aware hit-test/dismiss
block out of `handle_mouse_click_msg` into `dispatch_context_menu_click` and
call it first from `try_route_sidebar_mouse_event` whenever a menu is open,
before any click reaches the tree. Esc/Enter already closed it via the
generic `context_menu.is_some()` check in `handle_key_press` — that symptom
was a side effect of the passthrough clicks, not a separate bug.

FAILED-3 — tab-close confirm dialog silently no-op'd with a single group
(but worked with a split open): `screen_zone_hit_test`'s single-group
branch hardcoded `y >= 0.0` as the tab row's top. That was equivalent to
the multi-group branch's derivation (`window.rect.y - tab_bar_height`) only
by coincidence, when the editor content's top-left happened to be (0, 0).
#552 gave GTK's ShellApp a persistent menu/title-bar chrome band, so
`main_content_bounds.y` (and thus every window rect GTK builds) is now
routinely > 0 — the hardcoded origin stopped matching any real click, so a
click on the tab's × was misread as a buffer click (no confirm dialog ever
opened) instead of `CloseTab`. Fixed by deriving the bar's bounds from the
actual window rects, exactly like the multi-group branch already does.
Added `test_single_group_tab_bar_hit_test_with_editor_offset` (src/render.rs)
to pin this against a chrome-offset editor origin — TUI's content-relative
layout produces the same result as before, so this is a no-op there.

Verified: `cargo build`, `cargo clippy -- -D warnings`, and
`cargo test --no-default-features` are all clean (2129 vcd-binary tests +
2011 lib tests, 0 failures).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.

ShellApp: migrate dialog + context-menu rendering and dismissal (right-click & tab-close broken)

1 participant