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

[JDonaghy]

Summary

In the GTK ShellApp path (the Relm4→ShellApp flip, #540), dialogs and
context menus are never rendered and cannot be dismissed by clicking. Their
draw code and click-region caches are populated only by the now-dead legacy
draw_editor Cairo path, so in the live ShellApp build they are effectively
broken.

This was uncovered while finishing #515 (editor-group drag-and-drop). It is
ShellApp-port fallout, not a #515 regression — filing separately.

User-visible symptoms (GTK)

• Right-click does nothing — the editor/tab/explorer context menus never
  appear.
• Closing a tab (×) on a modified buffer hangs the UI — it fires
  Msg::ShowCloseTabConfirm → engine.dialog = Some(...), but the
  "Unsaved Changes" dialog is invisible (never drawn) and undismissable by
  click. dialog.is_some() stays true and the dialog block in
  handle_mouse_click_msg then swallows all subsequent clicks, so the editor
  appears frozen to the mouse. Only Esc (keyboard) clears it.
• Any other show_dialog(...) caller (:about, extension-remove confirm,
  buffer prompts, etc.) is likewise invisible/stuck.

Root cause

The rendering + hit-region caches live in the dead path:

• src/gtk/draw.rs — draw_dialog_popup(...) and the context-menu draw set
  dialog_btn_rects, dialog_popup_rect, and the context-menu layout. These run
  only inside draw_editor, which is not called under the ShellApp runner.
• render_content (the live path) draws neither dialogs nor context menus.
• handle_mouse_click_msg still has the dialog block (and the context-menu
  block) that read those now-empty caches:
  • dialog: dialog_btn_rects empty → no button is ever hit; dialog_popup_rect
None → never pushed to the modal stack → dispatch_mouse_down reports
    outside == false → the dismiss branch never runs → the dialog is stuck.
  • context menu: context_menu is set by right-click but never drawn, and the
    inner ContextMenuLayout hit-test has no geometry.

Fix

Migrate both surfaces into the ShellApp render + event path, mirroring the
explorer/tab-bar work already done for #515/#540:

1. Render in render_content — draw the dialog popup and the context-menu
   popup via quadraui primitives (Dialog/DialogTable, ContextMenu) on top of
   the frame, and cache their resolved click geometry from the same layout the
   renderer uses (no per-backend pixel maps).
2. Wire clicks — route dialog-button / context-menu-item clicks (and
   outside-click dismissal) through that cached layout in handle_mouse_click_msg,
   the same way tab-bar/tree hit-testing now flows through shared hit_regions /
   TreeController.
3. Verify right-click menus appear and act; tab-close-on-dirty shows a
   visible, clickable confirm; :about and other dialogs render and dismiss.

Acceptance

• Right-click in editor / tab bar / explorer opens a working context menu.
• Closing a modified tab shows a visible Unsaved-Changes dialog whose
  Save/Discard/Cancel buttons work by click (and Esc/Enter still work).
• No stuck-invisible-dialog state; clicks are not swallowed after a dialog opens.

Notes

• Platform-neutral: reuse the shared quadraui Dialog/ContextMenu primitives; the
  GTK side should be thin event→engine wiring only.
• Keyboard handling already works (engine handles dialog/menu keys) — this is
  specifically the render + mouse gap.

[JDonaghy]

Coordinator: Assignment Complete

Machine: dellserver
Status: done
Exit code: 0
Duration: —

Summary

Fixed #546: GTK ShellApp render_content never drew screen.dialog/screen.context_menu (dead legacy draw_editor path was the only writer of the click-geometry caches). Implemented larger de-dup scope per user request: added shared render::dialog_generic_layout/context_menu_generic_layout in src/render.rs used by BOTH GTK and TUI (replacing 4 independently-drifting reimplementations with 2), collapsed GTK's dialog_btn_rects+dialog_popup_rect into one dialog_layout field using DialogLayout::hit_test (mirrors context_menu_layout), made TUI's mouse.rs consume the render-cached DialogLayout instead of recomputing it (removed now-dead core::engine::{DialogLayout,DialogClickResult,resolve_dialog_click} + test), and fixed a second discovered gap: GTK explorer right-click was completely unwired (TreeController's ContextMenuRequested event was silently dropped) - now wired to open_explorer_context_menu. Build/clippy/fmt/tests all clean. Recorded a follow-up quadraui-issue suggestion in coord context (lift the generic-measure formula into the library) - not blocking, filed as a note for the coordinator since I can't run gh myself.

[JDonaghy]

Coordinator: Assignment Complete

Machine: elitebook
Status: done
Exit code: 0
Duration: —

Summary

Smoke test FAILED: editor right-click + :about work, but tab-bar menu shows wrong (editor) content, explorer menu renders but click-through/no dismiss, and single-tab-group tab-close still reproduces original stuck-invisible-dialog bug (works only with 2 tab groups).

[JDonaghy]

Coordinator: Assignment Complete

Machine: dellserver
Status: done
Exit code: 0
Duration: —

Summary

Fixed all 3 smoke-test failures. FAILED-1 (tab-bar right-click showed editor menu): added click::resolve_tab_right_click, a read-only zone-resolution mirror, so right-click now dispatches Msg::TabRightClick when over a tab. FAILED-2 (explorer ctx-menu non-interactive): extracted the modal-stack hit-test/dismiss logic into dispatch_context_menu_click and call it from try_route_sidebar_mouse_event before forwarding clicks to TreeController, so an open menu now takes priority over sidebar row clicks (Esc/Enter already worked generically once passthrough stopped). FAILED-3 (single-group tab-close dialog silently no-op'd): root cause was screen_zone_hit_test's single-group branch hardcoding y>=0.0 as the tab bar's origin, which broke once #552 gave GTK a persistent nonzero main_content_bounds.y chrome offset -- fixed by deriving bar bounds from actual window rects like the multi-group branch does, added a regression test (test_single_group_tab_bar_hit_test_with_editor_offset) in render.rs. cargo build, cargo clippy -- -D warnings, and cargo test --no-default-features (2129 vcd + 2011 lib tests) all clean. Pushed to issue-546-shellapp-migrate-dialog-context-menu-ren.

[JDonaghy]

Coordinator: Assignment Complete

Machine: elitebook
Status: done
Exit code: 0
Duration: —

Summary

Smoke re-test of #546 passed all 7 checks: dialogs + context menus (editor/tab-bar/explorer) render and dismiss correctly under ShellApp; prior failures (tab-bar menu content, explorer right-click) confirmed fixed.

[JDonaghy]

Coordinator: Assignment Complete

Machine: dellserver
Status: done
Exit code: 0
Duration: —

Summary

Review verdict recovered from the session transcript (agent could not run coord report-result).

Verdict: approve

Review findings

Re-review (iteration 1) — #546 fix commits

Scope: the two fix commits pushed since the failed smoke test — e441207 (render dialog/context-menu popups) and 713a05d (route tab-bar right-click, explorer ctx-menu clicks, single-group tab-close dialog). Concentrated on 713a05d, which directly targets the three FAILED items from the smoke test.

FAILED-1 — tab-bar right-click (fixed, verified)

src/gtk/click.rs: resolve_tab_right_click correctly mirrors pixel_to_click_target's zone resolution as a read-only lookup (no &mut Engine, no side effects), reusing screen_zone_hit_test + resolve_pixel_tab_click/resolve_charcell_tab_click — the same functions the left-click path already relies on. In src/gtk/mod.rs's MouseButton::Right arm, it's called with self.cached_screen_layout / cached_line_height / cached_char_width / cached_tab_pixel_hits — the identical cached fields the MouseDown left-click arm feeds into pixel_to_click_target a few lines above, so the coordinate space matches. Falls back to EditorRightClick when the zone isn't TabBar. Msg::TabRightClick{group_id, tab_idx, x, y} was already a real message with an existing handler (open_tab_context_menu) — this just wires the dispatch that used to never fire. Looks correct.

FAILED-2 — explorer context-menu interactivity (fixed, verified)

dispatch_context_menu_click is a faithful extraction of the block that used to live inline in handle_mouse_click_msg (diff confirms it's a pure move, not a rewrite — modal-stack push/dispatch/hit-test logic is byte-for-byte the same). It's now called from both handle_mouse_click_msg and, new, from try_route_sidebar_mouse_event, which checks context_menu.is_some() before falling through to the explorer's TreeController routing — correctly gates on MouseDown{button: Left} only, and swallows (returns true, sets draw_needed) other event kinds (DoubleClick, Scroll, non-left MouseDown) while a menu is open rather than leaking them to the tree underneath. This matches the stated fix and plugs the gap described in the issue.

FAILED-3 — single-group tab-close dialog no-op (fixed, verified)

screen_zone_hit_test's single-group branch now derives min_x/min_y/max_x from layout.windows and computes tab_y = min_y - tab_bar_height, mirroring the multi-group branch's GroupTabBar::bounds-derived logic immediately above it (same pattern: local_x: x - min_x, bar_width: max_x - min_x). This is consistent with how downstream consumers (pixel_to_click_target → tab_bar_inner_hit_test, and the new resolve_tab_right_click) already treat TabBar::local_x as bar-relative for the multi-group case, so the single-group case now matches instead of leaking an absolute x/y that only worked by coincidence when main_content_bounds was (0,0).

The new test test_single_group_tab_bar_hit_test_with_editor_offset correctly exercises the regression: it builds a WindowRect offset at (50, 100) (matching the #552 chrome-band scenario described in the commit message), confirms a click on the offset tab row resolves to ScreenZone::TabBar with local_x relative to the bar's left edge, and confirms a click just below the bar still resolves to Window (i.e. the derived band doesn't over-widen). WindowRect::new field order and build_screen_layout signature both match their real definitions.

Other notes

• The panels.rs / tests.rs modifications visible in this machine's working tree (Tab/ISO_Left_Tab/BackTab dialog-hotkey guard, tests referencing GTK: pressing Shift alone in a dialog activates the default button #207) are uncommitted, unrelated local changes — not part of the issue-546-... branch and not touched by either fix commit. Flagging only so they aren't accidentally swept into this PR's branch/commit later; they are out of scope for this review and I did not evaluate them.
• No regressions spotted in the reviewed diff. Borrow scoping in dispatch_context_menu_click (sequential borrow()/borrow_mut() calls, each dropped before the next) looks safe against RefCell double-borrow panics.
• Non-blocking nit: resolve_tab_right_click's Some(T::Tab(idx)) | Some(T::CloseTab(idx)) => Some((group_id, idx)) treats a click on a tab's close (×) button the same as a click on the tab body for right-click purposes — reasonable (right-clicking either should open the tab's context menu), just calling it out as a deliberate choice worth a one-line comment if not already obvious to future readers.

Verdict

The three previously-FAILED smoke-test items are each addressed at the root cause identified in the issue, consistent with existing patterns elsewhere in the file (multi-group tab-bar branch, existing pixel_to_click_target zone resolution), and backed by a regression test for FAILED-3. Approving.