#515: Migrate editor-group drag-and-drop onto quadraui TabGroupController (blocked on quadraui#349) - #545
Merged
JDonaghy merged 17 commits intoJul 1, 2026
Conversation
…ntroller Remove the old engine-embedded tab drag state (TabDragState, tab_drag, tab_drag_mouse, tab_drop_zone, tab_drag_begin/cancel/drop) and replace with a stateless, controller-based approach: - New src/tab_group_ctrl.rs (GUI-gated): VimcodeTabGroupCtrl wraps TabGroupController with GroupId<->pane-index mapping, plus helpers for event application, split-ratio sync, and drop-zone translation. - GTK (Relm4 legacy path, gtk/mod.rs): local tab_drag_source / tab_drag_drop_zone fields; drop calls apply_drop_zone via tab_group_ctrl. - TUI (tui_main): local tui_drag_source / tui_drag_cursor / tui_tab_drop_zone vars; apply_tui_drop_zone inline helper avoids the gui-feature dependency; draw_frame and render_tab_drag_overlay updated to accept drag state as params so the overlay still renders. - Engine tests updated to use the direct engine methods (move_tab_to_target_group, move_tab_to_new_split, etc.) that the controller delegates to. render.rs test likewise updated. - draw_tab_drag_overlay removed from gtk/draw.rs (overlay now rendered by TabGroupController::render in the ShellApp path). - Pre-existing set_nerd_fonts trait-not-in-scope errors fixed in gtk/mod.rs and tui_main/mod.rs. Note: VimcodeTabGroupCtrl is fully implemented but not yet wired into the ShellApp render loop — that wiring is the remaining phase of #515. Dead-code allowed in tab_group_ctrl with comment explaining the gap. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The drag overlay was removed from gtk/draw.rs (Relm4 path) but was never wired into the ShellApp path's render_content. Add the overlay at the end of render_content using the shared render::compute_tab_drop_overlay pipeline and backend.draw_drop_overlay, so the drop zone highlight and insertion bar reappear during GTK tab drags. Tab slot positions are not available in the ShellApp path so we pass an empty map; center/split zone highlights work correctly. Tab-reorder insertion bars fall back to group-start X (acceptable for now; the full TabGroupController wiring will resolve this in the next phase). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The comment said the overlay was handled by TabGroupController::render() but the actual fix (wired in f15d56f) uses render::compute_tab_drop_overlay + backend.draw_drop_overlay directly in ShellApp::render_content. Update the comment to accurately describe the real draw path; also apply cargo fmt cleanup (collapsed two-line let binding). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The Relm4→ShellApp flip left render_content as a half-finished port of the now-dead draw_editor Cairo path. Three user-visible GTK regressions resulted; all are fixed by wiring GTK to the shared, backend-neutral render code that TUI already uses — NOT by repopulating the legacy per-backend pixel maps. R1 — Tab clicks did nothing: tab_bar_inner_hit_test read five GTK pixel maps that nothing populates anymore (their only writer, draw_editor, is dead code). Route tab-bar clicks through the shared resolve_tab_bar_click(hit_regions) instead. hit_regions are now computed in backend-neutral char-cells (bounds.width / char_width; TUI passes char_width=1.0 so it is unchanged), and the single-group/active bar's hit_regions are exposed on ScreenLayout. R2 — Split groups had no tab bar: render_content only drew the single-group tab_bar_primitive and never iterated group_tab_bars. Draw one bar per group at its own bounds. R3 — Drop highlight covered half the group: the drag hit-test built relative (0-based) group bounds and compared them against an absolute cursor, so after a split the zone was misclassified (vertical main-content offset). Drop geometry is now computed once per frame in render_content via the shared screen_to_drop_group_bounds pipeline (correct absolute origin: (0,0) for multi-group whose bounds are already absolute, (x,y) for single-group) and cached for both the overlay and the drag hit-test, so detection and highlight use one identical source. Removes the now-dead GTK-specific build_gtk_tab_slots / compute_tab_drop_zone helpers. The legacy pixel-map fields remain (passed but unused) and are flagged for follow-up removal. No quadraui change required; the full TabGroupController migration remains blocked only on quadraui Caveat-1 (no mixed-direction split construction API). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
In the Relm4 build the file explorer had its own DrawingArea whose gesture controllers dispatched Msg::ExplorerUiEvent. The ShellApp flip draws the explorer into the shared sidebar surface but never re-wired those clicks, so the explorer tree did not respond to clicks at all. Add try_route_sidebar_mouse_event(): when a MouseDown/DoubleClick/Scroll lands in layout.sidebar_content_bounds and the explorer panel is active, forward the raw UiEvent to the existing Msg::ExplorerUiEvent handler, which drives the shared quadraui::TreeController (the same controller TUI uses) — no new per-backend hit-testing. MouseMoved/MouseUp are intentionally not intercepted so an editor text-drag crossing into the sidebar still finalizes through the editor's own path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The explorer click routing works but selects a row offset from the click, and tab hit-testing is slightly off. quadraui's tree render/hit-test were verified internally consistent (same gtk_tree_layout, same line height, scroll handled), so the offset is a runtime coordinate value not visible by code inspection. Add eprintln logging gated behind VIMCODE_HIT_DEBUG=1 that dumps, per click: - explorer: event position, the rect passed to TreeController, backend line_height, scroll_offset, and the resolved tree event - tab bar: local_x, char_width, computed cell column, the resolved target, and the full hit-region table No behavior change when the env var is unset. To be removed once the offset is identified and fixed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds VIMCODE_HIT_DEBUG=1 eprintln probes: - handle_mouse_click_msg entry (proves a plain click arrives + its coords) - pixel_to_click_target resolved ScreenZone (tab vs window vs none) - explorer render-time line_height + q_sb (to compare against hit-time) Narrows the explorer row-offset (suspected draw-vs-hit line-height mismatch) and the tab plain-click (suspected diverted before the tab hit-test). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…divert probes EXPLORER FIX: the file-explorer hit-test resolved a row offset from the click (off-by-1 at row 5, growing downward) because TreeController::handle runs backend.tree_layout() using the backend's mutable current_line_height, which by click time differed from the value draw_tree() used at render (≈20 vs 16 — a 1.25x scale). Capture the backend (line_height, char_width) at the instant the explorer is rendered (cached_explorer_metrics) and re-apply them before the hit-test so draw and hit agree. TAB PROBES: a plain tab click returns before pixel_to_click_target (drag works). Added VIMCODE_HIT_DEBUG markers at the breadcrumb-OnBar early-return and right before the editor click dispatch to pinpoint exactly where the click is consumed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Plain tab click reaches handle_mouse_click_msg but returns before the editor click dispatch and not via breadcrumb. Add probe A (past scroll-surface dispatch) and probe B (entering editor section) so one more click identifies which section consumes it.
…ult) Plain tab click survives probe A but not B. Add probe C (reached breadcrumb block, logs breadcrumbs on/off + result + bar bounds), D (past breadcrumb), E (past debug-toolbar). One click now localizes the consuming block exactly.
Click reaches probe E (entering editor-hover) but not B. Editor-hover and dialog blocks are gated; add an editor-hover state dump (is_some/on_popup/rect) and probe F (past editor-hover) to confirm whether a stale hover popup is consuming the tab click or the divert is in the dialog block.
Strips all VIMCODE_HIT_DEBUG eprintln instrumentation added while diagnosing the explorer row offset and tab-click divert. The fixes remain: - explorer hit-test uses the render-time line height (cached_explorer_metrics) - GTK tab clicks resolve via shared hit_regions - sidebar clicks routed to the explorer TreeController - per-group tab bars + drop-overlay/drag absolute bounds Diagnosis outcome recorded for follow-up: tab close + right-click are blocked by the unmigrated dialog/context-menu render+dismiss path in ShellApp (#540).
…ntroller scaffolding The 476-line `src/tab_group_ctrl.rs` bridge was `#![allow(dead_code)]` and never wired into the render/event path: driving `TabGroupController` live needs a paint-loop `render(&mut Backend, bounds)` pass (only `render()` populates the hit caches that `handle_tab_drop` reads — `prime_pane` is test-only), which cannot be validated headlessly, and the controller cannot become the editor backing store (Caveat-2: `PaneTab::content: Send + 'static` can't hold the engine's `Rc<RefCell<…>>` content). The drag *math* already runs on `quadraui::compute_drop_zone` via `render::compute_tab_drop_zone`, so the controller layer added duplication, not deletion. Instead, consolidate the one thing that was genuinely duplicated: the DropZone→engine mutation. Both backends had identical copies (`tab_group_ctrl::apply_drop_zone` for GTK, `apply_tui_drop_zone` for TUI). - Add `Engine::apply_tab_drop_zone` in core (the single backend-agnostic drop entry point; pure core logic over `core::window::DropZone`). - Route GTK (`gtk/mod.rs`) and TUI (`tui_main/mouse.rs`) drops through it. - Delete `src/tab_group_ctrl.rs` and the TUI's `apply_tui_drop_zone` duplicate; remove the `mod tab_group_ctrl` declaration. - Add 6 unit tests for `apply_tab_drop_zone` (all DropZone variants), which run under `--no-default-features`. Also fixes a pre-existing gui-build break: `Backend::set_nerd_fonts` was called in `gtk/mod.rs` without the trait in scope (one-line `use quadraui::Backend;`). Net −383 lines. Full `TabGroupController` adoption (live paint-loop render + backing-store replacement) remains blocked per the constraints above and is the rescope work the reviewer flagged as option (b). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ells The ShellApp render path drew each editor-group tab bar through the quadraui backend, which lays tabs out with proportional-font Pango widths plus fixed pixel padding (tab_pad / inner_gap / close-glyph). But the click handler resolved tab-bar hits against the char-cell `hit_regions` (name.chars() + close cols), which are correct only for the monospace TUI. The two geometries diverge: a `name.chars() * char_width` estimate under-measures every tab, so the tab/close and tab/tab boundaries drifted — mid-tab clicks landed on the close button (closing the tab) and right-edge clicks resolved to the next tab. Capture the exact pixel geometry the rasteriser drew via `Backend::tab_bar_layout()` during `render_content`, cache it per group (relative to the bar's left edge, matching screen_zone_hit_test's local_x), and resolve tab-bar clicks against those pixel bounds. Close buttons are checked before tab bodies, then bodies, then the right segments. The char-cell path stays as a pre-first-paint fallback and remains authoritative for the TUI. Also populates `tab_close_bounds` from the same source so close-button hover works in ShellApp. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two GTK regressions from the ShellApp tab-bar migration on this branch: BUG 1 — close button (×) was oversized with no hover warning. `Backend::tab_bar_layout` reports a *padded* close-button hit zone that spans `[label_end, tab_right_edge]` — ~25px wider than the drawn × glyph. The click path (06a62d0) hit-tested against that padded zone, so clicking well before the glyph silently closed the tab. Separately, the hover hit-test (`tab_close_hit_test`) rebuilt group rects from a `(0,0)` content origin, ignoring the activity-bar/sidebar x-offset, so the × highlight never fired once a sidebar was open — no visual warning before the destructive click. Fix: `tighten_close_bounds` trims the padded zone down to the exact × glyph box the rasteriser paints (plus its 2px hover halo), so click and hover fire on the same tight box the user sees highlighted. Capture the close rects in absolute surface coords during `render_content` (`cached_tab_close_abs`) and hit-test hover against them directly — no geometry re-derivation, so the sidebar offset is included. The padded metrics mirror quadraui's non-compact GTK constants; interim until quadraui exposes the tight glyph rect (quadraui#395). BUG 2 — could not reorder tabs within a group by dragging. `build_tab_drop_groups` was fed an empty tab-slots map, so `compute_drop_zone`'s `in_tab_bar && !tab_slots.is_empty()` branch was unreachable and every in-bar drag fell through to a new-split/center overlay. Populate `cached_tab_slots_abs` from the same `tab_bar_layout` pass (absolute visible slot x-ranges) and pass it in, so a short drag inside a group's own tab bar resolves to `TabReorder` with an insertion bar. Also drops the now-dead write-only `tab_close_bounds` cache (hover reads `cached_tab_close_abs` instead). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…gine GTK ShellApp forwards Ctrl+\ as the raw char key_name "\\", while the TUI maps it to "backslash". The engine's Ctrl block only matched "backslash", so Ctrl+\ was a silent no-op in GTK (regression flagged by #515 smoke test). Fix in core (platform-neutral): accept "backslash" | "\\", mirroring the existing "bracketright" | "]" dual-match. Backends stay thin wiring. Adds a regression test covering both key_name spellings. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Aug 8, 2026
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.
Closes #515
Automated merge from the coordinator for assignment 537fcff8615e on issue #515.
Worker branch:
issue-515-migrate-editor-group-drag-and-drop-onto→develop.