Skip to content

fix(compact-bar): forward wheel to exact focused pane - #20

Open
m-szymanska wants to merge 8 commits into
developfrom
agent/fix-compact-bar-scroll
Open

fix(compact-bar): forward wheel to exact focused pane#20
m-szymanska wants to merge 8 commits into
developfrom
agent/fix-compact-bar-scroll

Conversation

@m-szymanska

@m-szymanska m-szymanska commented Aug 21, 2026

Copy link
Copy Markdown
Member

Summary

  • stop using wheel events over compact-bar to switch tabs
  • add a pane-id-bound mouse-scroll plugin contract carrying a content-relative position and line count
  • route the event synchronously to the tab containing that exact pane, without screen-coordinate hit testing or late active-tab selection
  • require WriteToStdin for third-party callers because the contract can synthesize terminal input
  • clamp compact-bar trackpad deltas to the shared 100-line limit, reject larger third-party requests at the host boundary, and stream alternate-screen arrow fallback writes without a line-count-sized allocation
  • target the focused terminal cursor from compact-bar, with a content-center fallback when the cursor is unavailable, and revalidate stale positions against current pane dimensions after resize
  • preserve native mouse reporting for TUIs, alternate-screen arrow fallback, and ordinary scrollback
  • leave tab clicks, classic tab-bar behavior, and the existing scrollback-only pane-id API unchanged
  • restore rustfmt compliance for the source-manifest-dir expression already present at the develop tip

Why

The product compact-bar is a non-selectable one-row chrome surface, but it consumed wheel input to switch tabs. Moving the pointer across the top row therefore interrupted scrolling in the active Codex or shell pane.

The initial coordinate-only implementation reused ScrollUpAt and ScrollDownAt. Review correctly identified that this could hit an overlapping pinned or floating pane, race with a tab switch because run_action dispatches on another thread, and use an arbitrary point in a mouse-aware TUI. The final implementation binds the event to the exact PaneId and local cursor position before it enters the screen queue.

Verification

  • cargo test -p compact-bar — 19 passed, including 3/100/usize::MAX wheel-delta bounds
  • cargo test -p zellij-utils mouse_scroll_in_pane_id — 3 passed, including invalid-range and excessive-count rejection
  • cargo test -p zellij-server mouse_scroll_in_pane_id — 2 passed, covering exact local position and stale-position clamping after resize
  • cargo test -p zellij-server pane_faux_scrolling_in_alternate_mode — passed for coordinate and pane-ID paths
  • cargo clippy -p compact-bar -p zellij-utils -p zellij-tile -p zellij-server --all-targets -- -D warnings — passed
  • make plugins-assets — passed; rebuilt the 14 tracked plugins because the shared plugin API changed and refreshed SHA256SUMS
  • make plugins-parity — passed, 14/14 assets match SHA256SUMS
  • cargo test -p zellij-utils asset_map_matches_bundled_plugin_files — passed
  • cargo xtask format --check — passed
  • git diff --check — passed

Runtime note

The locally built binary and WASM were not projected into the installed Vibecrafted runtime, so the active user session remained untouched. Runtime mouse behavior should be smoke-tested in an isolated session before release.

Known develop CI debt

The current develop tip contains stale snapshots from the earlier Quick cmd key change from Super+k to Shift+Super+period. The compact-bar tests pass in the No Web run, but the shared zellij-utils snapshot suites fail afterward:

  • No Web: 2 stale KDL snapshots
  • Full zellij-utils test jobs on Windows, macOS, and Ubuntu: the same 2 KDL snapshots plus 4 stale setup snapshots
  • E2E: 10 stale chrome snapshots showing the already-landed failed-pane marker, terminal-theme control, and spacing changes (9a07c5938, fb979323f) against snapshots last synchronized by 31dfa9353

Updating the six zellij-utils generated snapshots alone is roughly 3,200 lines of churn; the E2E snapshots are a separate existing chrome drift. Both are deliberately kept out of this focused behavior PR. These failures are inherited from base configuration/chrome versus snapshot mismatches rather than introduced by this diff.

Additional CI flake observed

The final pre-hardening SHA Ubuntu build and runtime triage passed. On two earlier attempts, the Ubuntu build itself also passed. Its post-build runtime process-boundary triage failed twice for different timing-sensitive reasons: attempt 1 received empty/non-JSON pane inventory for Needs attention; attempt 2 timed out waiting for a NewTab acknowledgement in a headless transcript scenario. Cleanup passed in both receipts, and the same Ubuntu build+triage job passed on the preceding PR SHA 6294ccb26. No mouse-scroll or plugin-API test failed in these attempts. This is recorded as CI instability rather than patched into this focused PR.

Replace compact-bar wheel-to-switch-tabs with coordinate-aware scroll actions aimed at the focused terminal pane. Resolve focus per client, use the current pane geometry, and leave plugin or empty targets untouched so terminal mouse protocol, alternate-screen fallback, and normal scrollback continue through the existing core path.

Add focused unit coverage, record the behavior change in the changelog, and keep the classic tab-bar unchanged.

Authored-By: codex <agents@vetcoders.io>
session_id: 01a01c6a-1eac-7761-b669-d95c024963cd
time: 2026-08-21T21:28:06+02:00
runtime: codex
Copilot AI lite review requested due to automatic review settings August 21, 2026 19:28

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Code review skipped — your organization has reached its monthly code review spending cap.

An organization admin can view or raise the cap at claude.ai/admin-settings/claude-code. The cap resets at the start of the next billing period.

Once the cap resets or is raised, reopen this pull request to trigger a review.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Hi @m-szymanska, I've received your request, and I'm working on it now! You can track my progress in the logs for more details.

Apply rustfmt to the source-manifest-dir expression introduced at the current develop tip. The formatting and Ubuntu test jobs both stop at this shared precheck before exercising the compact-bar change.

Authored-By: codex <agents@vetcoders.io>
session_id: 01a01c6a-1eac-7761-b669-d95c024963cd
time: 2026-08-21T21:31:29+02:00
runtime: codex

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the compact-bar plugin so mouse wheel events over the one-row chrome no longer switch tabs, and instead forward a coordinate-aware scroll action into the currently focused terminal pane for the current client. This aligns wheel behavior with user expectations (scroll continues uninterrupted) while relying on Zellij’s existing scrollwheel handling to preserve mouse-reporting TUIs, alternate-screen fallback, and normal scrollback behavior.

Changes:

  • Replace compact-bar wheel-to-tab-switching with wheel forwarding to the focused terminal pane via Action::Scroll{Up,Down}At.
  • Track the latest PaneManifest in plugin state to compute a safe “center of focused terminal content” target coordinate.
  • Add unit tests covering coordinate selection and edge cases (plugin-only focus / empty content surfaces), and add the needed zellij-utils dependency.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.

File Description
default-plugins/compact-bar/src/main.rs Stores PaneManifest and forwards wheel events to the focused terminal using coordinate-aware scroll actions; adds tests for the new behavior.
default-plugins/compact-bar/Cargo.toml Adds zellij-utils dependency for Position.
CHANGELOG.md Documents the compact-bar wheel behavior fix.
Cargo.lock Locks the new dependency edge for zellij-utils.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI review requested due to automatic review settings August 21, 2026 19:33

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8227a06134

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread default-plugins/compact-bar/src/main.rs Outdated
Comment thread default-plugins/compact-bar/src/main.rs Outdated
Comment thread default-plugins/compact-bar/src/main.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

default-plugins/compact-bar/src/main.rs:655

  • forward_scroll_to_focused_pane drops wheel events when pane_manifest doesn't yet contain the focused pane (eg early startup / stale manifest). That can make scrolling over the compact bar intermittently do nothing. Consider falling back to get_pane_info(focused_pane_id) to compute a content-center position when the manifest lookup fails, so wheel forwarding remains reliable.
    fn forward_scroll_to_focused_pane(&self, scroll_up: bool) {
        let Ok((_, focused_pane_id)) = get_focused_pane_info() else {
            return;
        };
        let Some(position) = focused_terminal_content_center(focused_pane_id, &self.pane_manifest)

Comment thread zellij-utils/build.rs
@m-szymanska
m-szymanska marked this pull request as draft August 21, 2026 19:42
Add a pane-id-bound mouse-scroll plugin contract carrying a content-relative position and line count. Route it synchronously to the tab containing that exact pane, preserving mouse-aware TUI events, alternate-screen arrows, and ordinary scrollback without coordinate hit testing or active-tab races.

Use the focused terminal cursor from compact-bar, with a content-center fallback, and cover plugin API roundtrips plus exact local mouse coordinates.

Authored-By: codex <agents@vetcoders.io>
session_id: 01a01c6a-1eac-7761-b669-d95c024963cd
time: 2026-08-21T20:47:03Z
runtime: codex
@m-szymanska m-szymanska changed the title fix(compact-bar): forward wheel to focused pane fix(compact-bar): forward wheel to exact focused pane Aug 21, 2026
Query PaneInfo for the exact focused PaneId at wheel time instead of depending on the latest PaneUpdate snapshot. This removes the early-startup and stale-manifest no-op window while retaining the pane-id-bound routing contract.

Authored-By: codex <agents@vetcoders.io>
session_id: 01a01c6a-1eac-7761-b669-d95c024963cd
time: 2026-08-21T20:52:38Z
runtime: codex
@m-szymanska

Copy link
Copy Markdown
Member Author

Follow-up 6294ccb26 also removes the stale PaneManifest dependency from wheel forwarding. After resolving the focused PaneId, compact-bar now synchronously queries fresh PaneInfo before deriving the cursor/content fallback, closing the early-startup and delayed-PaneUpdate no-op window noted in the suppressed review suggestion.

@m-szymanska
m-szymanska marked this pull request as ready for review August 21, 2026 20:57
Copilot AI review requested due to automatic review settings August 21, 2026 20:57

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

zellij-utils/src/plugin_api/plugin_command.rs:1483

  • Same as MouseScrollUpInPaneId: decoding position via try_into() will silently wrap/truncate negative or oversized int64 values coming from a plugin. This new plugin->host API should reject out-of-range positions explicitly before creating the internal Position.
                    let position = payload
                        .position
                        .ok_or("MouseScrollDownInPaneId requires a position")?
                        .try_into()?;

Comment thread zellij-utils/src/plugin_api/plugin_command.rs
Reject negative or unrepresentable content-relative mouse coordinates before constructing Position from the plugin protobuf payload. Cover both scroll directions and all line/column range boundaries.

Authored-By: codex <agents@vetcoders.io>

session_id: 01a01c6a-1eac-7761-b669-d95c024963cd

time: 2026-08-21T21:10:00Z

runtime: codex
Copilot AI review requested due to automatic review settings August 21, 2026 21:08

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d64396a4de

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread zellij-server/src/plugins/zellij_exports.rs Outdated
Comment thread zellij-server/src/tab/mouse_handler.rs Outdated
Require WriteToStdin for pane-targeted mouse scroll commands and stream alternate-screen key writes instead of allocating one vector per requested line. Extend the faux-scroll integration test to cover the pane-ID path.

Authored-By: codex <agents@vetcoders.io>

session_id: 01a01c6a-1eac-7761-b669-d95c024963cd

time: 2026-08-21T21:48:13Z

runtime: codex
Copilot AI review requested due to automatic review settings August 21, 2026 21:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fd2e051b93

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread zellij-server/src/tab/mouse_handler.rs

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Comment thread zellij-tile/src/shim.rs
Reject pane-targeted mouse-scroll requests above 100 lines at both serialization and host decoding boundaries. Document the limit and cover both directions plus the accepted boundary.

Authored-By: codex <agents@vetcoders.io>
Copilot AI review requested due to automatic review settings August 21, 2026 22:43
@m-szymanska
m-szymanska force-pushed the agent/fix-compact-bar-scroll branch from 77b1718 to bdc031e Compare August 21, 2026 22:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Comment thread default-plugins/compact-bar/src/main.rs

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bdc031ed69

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread zellij-server/src/tab/mouse_handler.rs Outdated
Comment thread default-plugins/compact-bar/src/main.rs
Clamp large compact-bar wheel deltas to the shared host limit, revalidate stale cursor positions against current pane dimensions, and refresh the bundled plugin artifacts and checksum receipt.

Authored-By: codex <agents@vetcoders.io>
Copilot AI review requested due to automatic review settings August 21, 2026 23:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 30 changed files in this pull request and generated no new comments.

@m-szymanska

Copy link
Copy Markdown
Member Author

Final verification for 7d2972213:

  • formatting: passed
  • platform builds: Ubuntu, macOS, and Windows passed; Ubuntu/macOS runtime process-boundary triage passed
  • focused local suites: compact-bar 19/19, plugin API 3/3, server pane-targeted scrolling 2/2, alternate-screen fallback 1/1, Clippy clean
  • bundled assets: canonical make plugins-assets, make plugins-parity 14/14, asset-map test passed
  • E2E: 32 passed / 10 failed, matching the documented inherited chrome snapshot drift; both scrolling_inside_a_pane and scrolling_inside_a_pane_with_mouse passed
  • shared test lanes fail only on the same documented 2 KDL + 4 setup snapshots

All review threads are resolved. The installed Vibecrafted runtime and active sessions were not modified.

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.

2 participants