#555: [retry] #448-H: GTK breadcrumb bar non-functional after ShellApp migration (clicks dead / dropdown hidden) - #664
Merged
JDonaghy merged 2 commits intoAug 26, 2026
Conversation
#448-H. Four independent defects, all regressions from the #540 Relm4→ShellApp migration, stacked between a breadcrumb click and a working dropdown. Each one alone was enough to make the bar look dead. 1. **Clicks resolved against the wrong group.** `render::resolve_breadcrumb_click` scans every group's bar but returned a bare segment index, which `Engine::handle_breadcrumb_click` then resolved against the *active* group's segments. With two groups on files of different path depth, clicking the deeper group's third segment while the shallower group held focus produced an out-of-range index and `breadcrumb_open_scoped` silently bailed. `Hit` now carries the `GroupId`, and the engine focuses that group first. Both backends updated (TUI routes through the same entry point). 2. **The sidebar ate presses on the dropdown's left half.** `try_route_sidebar_mouse_event` intercepted any press inside `sidebar_content_bounds`, but the picker is centred on the *window*, so with the sidebar open its left column sits on top of the explorer tree. Those presses drove the tree underneath instead of the popup. An open picker now owns the press — which also makes outside-click dismissal work, since the fall-through reaches the picker's own modal-stack dispatch. 3. **The popup's hit rect was in a different place than its pixels.** `render_content` centres the popup in `backend.viewport()` at `gtk_picker_sizing(line_height)`; `compute_picker_popup_bounds` re-derived it from `main_content_bounds` at `(0, 0)` with a `line_h: 1.0` sizing. The painted rect is now published and reused. 4. **Click hit-tests measured painted geometry with the wrong line height.** `render_content` takes `&self` and cannot write the plain `cached_line_height` field, which is seeded once in `setup()` from the runner's default metrics — 16px against a painted 18px here. The picker resolved clicks two rows off; every single-row band, including the breadcrumb row itself, lost its bottom 2px. The painted value is now published in a `Cell` and read by the hit-tests. Also replaces the breadcrumb *double*-click handler's hand-rolled pre-#540 geometry (`y >= lh && y < lh * 2.0` plus a per-char_width walk over the active group's segments) with the same shared `resolve_breadcrumb_click` the single-click path uses. That band no longer contains the breadcrumb row under ShellApp, so double-click was dead while still able to match chrome rows and fire the wrong segment. Tests (black-box, via the #646 headless GTK harness): - `breadcrumb_segment_click_opens_the_dropdown_and_selection_dispatches` — the acceptance test: click a segment, assert the dropdown paints, click an entry, confirm, assert it navigated. Its entry click lands in the sidebar-overlap band, so it covers (2) as well. - `breadcrumb_click_acts_on_the_clicked_group_not_the_focused_one` — the split-group regression from (1). - `test_breadcrumb_click_resolves_against_the_clicked_group` — backend-agnostic guard for (1), covering the TUI path too. - New `Harness::breadcrumb_segment_center` locates segments from the rasteriser's own `StatusBarLayout`, never hardcoded coordinates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…nted text The GTK harness test added with the #555 fix asserted the dropdown had painted with `screen_contains("Find Files")` and located a result row with `driver.find("SUMMARIES/core_modules.md")`. Both can only ever fail at the quadraui rev `quadraui-pin.txt` pins: paint-time text recording (quadraui's `gtk/painted_text.rs`, which routes every rasteriser's `show_layout` into the painted-text map) landed *after* f6d27c2, so at the pin `GtkBackend::draw_palette` contributes nothing to the map that `find`/`screen_contains` scan. The popup was painting the whole time — this module's header already warns that a `None` from `find` means "not recorded", not "not drawn", and the assertion walked straight into it. Replace both with rev-independent checks on what the frame actually painted: - `Harness::picker_popup()` exposes the popup rect `render_content` publishes *inside* its picker draw branch (and clears on any frame without one), so `Some` means drawn, not `picker_open == true`. - `picker_row_center()` / `picker_row_probe()` locate a result row from that painted rect plus the painted line height, mirroring `draw_palette`'s row layout — the same arithmetic the click path hit-tests with, so test and user resolve to the same row. - The dropdown's selection highlight is read back with `driver.pixel`: row 0 differs from row 3 on open, and after clicking row 3 the two colours swap. That is a stronger check than the text match it replaces — it pins paint geometry and click geometry to each other, which is the "cache at paint, hit-test at click" invariant #555 broke. Also drops the hardcoded `SUMMARIES/core_modules.md` expectation: the Enter assertion now compares against whichever item the clicked row resolved to, so the test no longer depends on the repo's directory listing. No production code changes.
7 tasks
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 #555
Automated PR opened by coordinator for review of issue #555.