Unified sidebar rendering via ScreenLayout

[JDonaghy]

Context

The editor area, find/replace panel, tab bar, and status bar all render from shared ScreenLayout data — backends just iterate and draw. This is the pattern that makes "test once, works everywhere" possible. The sidebar does NOT follow this pattern: each backend (GTK, TUI, Win-GUI) builds its own sidebar UI independently.

Current state:

Component	Renders from ScreenLayout?	Clicks centralized?
Editor content	✅ all backends	✅
Find/replace	✅ all backends	✅
Tab bar	✅ all backends	TUI ✅, GTK/Win-GUI ❌
Status bar	✅ all backends	TUI ✅, GTK/Win-GUI ❌
Sidebar	❌ each backend builds own	❌ each backend has own logic

GTK is the biggest outlier — it uses native GTK widgets (buttons, tree views) for the sidebar while TUI and Win-GUI do coordinate-based rendering and click detection.

Goal

Move sidebar panel content into ScreenLayout so backends become thin drawing loops:

1. Activity bar — ScreenLayout provides a list of ActivityBarItem { icon, label, panel_id, is_active } entries. Backends iterate and draw icons. Click → resolve_activity_bar_click() (already done in Hit regions for sidebar #42).

2. Explorer tree — ScreenLayout provides pre-built rows (ExplorerRow { indent, icon, name, is_dir, is_expanded, is_selected, ... }). Backends iterate and draw. Currently TUI has TuiSidebar.rows and GTK uses a TreeView — both should consume the same data.

3. Git/debug/extensions/AI panels — ScreenLayout already has SourceControlData, DebugSidebarData, ExtSidebarData, AiPanelData. These just need scroll-aware row computation so backends don't recompute positions.

4. Tab bar + status bar click migration — GTK and Win-GUI still use their own pixel-based hit caches for tab bar (Hit regions for tab bar #40) and status bar (Hit regions for status bar #41) clicks instead of the shared hit regions. Migrate them.

Scope

• Add sidebar rendering data to ScreenLayout (activity bar items, panel content rows)
• Migrate GTK sidebar from native widgets to DrawingArea rendering from ScreenLayout data (or keep widgets but drive them from shared data)
• Migrate GTK/Win-GUI tab bar and status bar clicks to use shared hit regions
• Ensure all 3 backends produce visually consistent sidebars

Dependencies

• After Hit regions for sidebar #42 (sidebar activity bar hit regions)
• After Hit regions for context menus + dialogs #43 (context menus + dialogs hit regions)
• Before Abstract event layer (UiEvent) #44 (abstract event layer)

Acceptance

A change to sidebar panel rendering or click logic in render.rs / engine/ should work in all backends without per-backend fixes. Testing in one backend gives high confidence for the others.

[JDonaghy]

Coordinator: Assignment Complete

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

Summary

Implemented #133: consolidated the duplicated activity-bar builder into a single shared render::build_activity_bar() in render.rs. GTK's build_gtk_activity_bar_primitive (~80 lines) and TUI's build_activity_bar_primitive (~80 lines) both deleted; both backends now call the shared adapter with include_hamburger and active_ext_panel params. Icons now use icon.s() (respects use_nerd_fonts setting) fixing a latent GTK bug. Build clean, clippy clean, 2005 tests pass. Branch pushed, no PR yet (awaiting smoke-test approval).

[JDonaghy]

Coordinator: Assignment Complete

Machine: dellserver
Status: done
Exit code: 0
Duration: 30m 37s

[JDonaghy]

Coordinator: Assignment Complete

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

Summary

All 6 checks passed: GTK and TUI activity bars render correctly from shared render::build_activity_bar(); hamburger present in TUI only; panel switching, keyboard focus, search icon, and nerd fonts toggle all good. GTK nerd fonts requiring restart is pre-existing.

[JDonaghy]

Coordinator: Assignment Complete

Machine: elitebook
Status: done
Exit code: 0
Duration: 12m 20s

[JDonaghy]

Coordinator: Assignment Complete

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

Summary

Clean unification refactor; one silent icon change (SEARCH_COD→SEARCH for GTK) and no new tests, neither blocking

Verdict: approve

Review findings

Review: feat(#133) — unify activity bar builder into shared render::build_activity_bar

Branch: issue-133-unified-sidebar-rendering-via-screenlayo
Files changed: src/gtk/mod.rs, src/render.rs, src/tui_main/panels.rs
Commit: 2089981

---

Summary

This PR removes the duplicate per-backend activity bar builder functions (build_gtk_activity_bar_primitive in src/gtk/mod.rs and build_activity_bar_primitive in src/tui_main/panels.rs) and replaces them with a single shared render::build_activity_bar(engine, theme, include_hamburger, active_ext_panel) function in src/render.rs. Both call-sites are wired to the shared function with a two-liner delegation.

The change is architecturally sound and moves in exactly the right direction per the Platform-Neutrality rule.

---

Findings

1. Silent icon change for GTK search button (informational — should confirm intentional)

src/gtk/mod.rs (deleted function) vs src/render.rs:7466

The old GTK builder used:

icons::SEARCH_COD.nerd   // \u{ea6d} — nf-cod-search (VS Code codicon)

The new shared builder uses:

icons::SEARCH.s()        // \u{f002} when nerd fonts enabled — font-awesome magnifier

The comment in src/icons.rs:56 explicitly labels SEARCH_COD as "GTK only", which indicates the distinction was a deliberate prior choice. The commit message does not call out this visual change.

In practice: both are magnifier-glass glyphs, so the UX impact is minimal. And the issue goal is unification — having GTK use a different icon from TUI is an existing inconsistency that the PR rightly erases. However, since icons.rs still carries the // (GTK only) annotation on SEARCH_COD, it would be good for the commit description or a follow-up comment to confirm this change is intentional and that the annotation can be removed (or changed to "unused").

2. No unit tests added

Review checklist: "Did the worker add or update tests for the change?"

render::build_activity_bar is a new pub function with no tests. Because it requires an Engine instance and a Theme, writing a minimal test is non-trivial, but the existing test infra (Engine::new() + Theme::onedark()) does support it. A test verifying the item count, ordering, keyboard-index assignments, and the include_hamburger branching would add meaningful regression coverage for this shared adapter.

Not blocking on its own (it's a refactoring, not new behavior), but worth addressing in a follow-up.

3. TUI tooltip content changed (cosmetic)

src/tui_main/panels.rs (old build_activity_bar_primitive)

Old TUI tooltips: "Explorer", "Search", etc.
New shared tooltips: "Explorer (Ctrl+Shift+E)", "Search (Ctrl+Shift+F)", etc.

TUI tooltips are unlikely to be visible to users in the terminal backend, so this is cosmetic. Nonetheless it is a behavioral difference that could surface if quadraui ever renders TUI tooltips.

---

Positive observations

• Platform neutrality ✅ — New code lives in src/render.rs (shared), imports only core::engine::sidebar constants, quadraui, and icons. Zero GTK/TUI types. Exactly the right layer.
• USE_NERD_FONTS now respected for GTK ✅ — The old GTK builder hardcoded .nerd (nerd glyphs regardless of settings); the new builder uses .s() which honours settings.use_nerd_fonts. This is a correctness improvement.
• Doc-comment fix ✅ — A pre-existing confusion in src/gtk/mod.rs where map_gtk_key_name's doc-comment was accidentally prepended to build_gtk_activity_bar_primitive is silently fixed by the refactor.
• active_ext_panel parameter is correct ✅ — GTK passes engine.ext_panel_active.as_deref() (same source as before) and TUI passes sidebar.ext_panel_name.as_deref() (same source as before). Logic is equivalent.
• Scope appropriate ✅ — The PR is an honest incremental step of issue Unified sidebar rendering via ScreenLayout #133; the commit message is accurate.

---

Verdict

The SEARCH icon swap is the only substantive behavioral change and it is consistent with the stated unification goal (both backends should render identically). The lack of tests is a minor gap. Neither is blocking.

Approve with the request to either update the icons.rs annotation on SEARCH_COD (now unused) or add a note in the commit/PR description confirming the icon change is intentional.

[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

Review: feat(#133) — unify activity bar builder into shared render::build_activity_bar

Branch: issue-133-unified-sidebar-rendering-via-screenlayo
Files changed: src/gtk/mod.rs, src/render.rs, src/tui_main/panels.rs
Commit: 2089981

---

Summary

This PR removes the duplicate per-backend activity bar builder functions (build_gtk_activity_bar_primitive in src/gtk/mod.rs and build_activity_bar_primitive in src/tui_main/panels.rs) and replaces them with a single shared render::build_activity_bar(engine, theme, include_hamburger, active_ext_panel) function in src/render.rs. Both call-sites are wired to the shared function with a two-liner delegation.

The change is architecturally sound and moves in exactly the right direction per the Platform-Neutrality rule.

---

Findings

1. Silent icon change for GTK search button (informational — should confirm intentional)

src/gtk/mod.rs (deleted function) vs src/render.rs:7466

The old GTK builder used:

icons::SEARCH_COD.nerd   // \u{ea6d} — nf-cod-search (VS Code codicon)

The new shared builder uses:

icons::SEARCH.s()        // \u{f002} when nerd fonts enabled — font-awesome magnifier

The comment in src/icons.rs:56 explicitly labels SEARCH_COD as "GTK only", which indicates the distinction was a deliberate prior choice. The commit message does not call out this visual change.

In practice: both are magnifier-glass glyphs, so the UX impact is minimal. And the issue goal is unification — having GTK use a different icon from TUI is an existing inconsistency that the PR rightly erases. However, since icons.rs still carries the // (GTK only) annotation on SEARCH_COD, it would be good for the commit description or a follow-up comment to confirm this change is intentional and that the annotation can be removed (or changed to "unused").

2. No unit tests added

Review checklist: "Did the worker add or update tests for the change?"

render::build_activity_bar is a new pub function with no tests. Because it requires an Engine instance and a Theme, writing a minimal test is non-trivial, but the existing test infra (Engine::new() + Theme::onedark()) does support it. A test verifying the item count, ordering, keyboard-index assignments, and the include_hamburger branching would add meaningful regression coverage for this shared adapter.

Not blocking on its own (it's a refactoring, not new behavior), but worth addressing in a follow-up.

3. TUI tooltip content changed (cosmetic)

src/tui_main/panels.rs (old build_activity_bar_primitive)

Old TUI tooltips: "Explorer", "Search", etc.
New shared tooltips: "Explorer (Ctrl+Shift+E)", "Search (Ctrl+Shift+F)", etc.

TUI tooltips are unlikely to be visible to users in the terminal backend, so this is cosmetic. Nonetheless it is a behavioral difference that could surface if quadraui ever renders TUI tooltips.

---

Positive observations

• Platform neutrality ✅ — New code lives in src/render.rs (shared), imports only core::engine::sidebar constants, quadraui, and icons. Zero GTK/TUI types. Exactly the right layer.
• USE_NERD_FONTS now respected for GTK ✅ — The old GTK builder hardcoded .nerd (nerd glyphs regardless of settings); the new builder uses .s() which honours settings.use_nerd_fonts. This is a correctness improvement.
• Doc-comment fix ✅ — A pre-existing confusion in src/gtk/mod.rs where map_gtk_key_name's doc-comment was accidentally prepended to build_gtk_activity_bar_primitive is silently fixed by the refactor.
• active_ext_panel parameter is correct ✅ — GTK passes engine.ext_panel_active.as_deref() (same source as before) and TUI passes sidebar.ext_panel_name.as_deref() (same source as before). Logic is equivalent.
• Scope appropriate ✅ — The PR is an honest incremental step of issue Unified sidebar rendering via ScreenLayout #133; the commit message is accurate.

---

Verdict

The SEARCH icon swap is the only substantive behavioral change and it is consistent with the stated unification goal (both backends should render identically). The lack of tests is a minor gap. Neither is blocking.

Approve with the request to either update the icons.rs annotation on SEARCH_COD (now unused) or add a note confirming the icon change is intentional.