Remove wheel-scroll tab/pane navigation (rollback #80/#83)#104
Conversation
zellij delivers ScrollUp/ScrollDown with no device identity and no physical detent, so a notched wheel and a stepless device (Magic Mouse, trackpad) are indistinguishable at the event level. A stepless device emits a burst of scroll events per flick, and no single rate-limiter setting reconciles both classes: a short cooldown still races through many tabs/panes, while a long one locks out deliberate input and lets momentum tails add stray steps. The successive attempts -- leading-edge cooldown (#83), debounce-to-last-event (#96), reopen-on-timer throttle (#100) -- each only traded one failure mode for another. Roll the feature back entirely: delete src/scroll.rs, the scroll / scroll_cooldown_ms config keys, the ScrollUp/ScrollDown/Timer handlers and the EventType::Timer subscription, the cooldown state, the scroll-only pane traversal helper (projection::pane_ids_in_reading_order), and the README docs. The wheel over the bar returns to inert (pre-#80). Closes #103. BREAKING CHANGE: the `scroll` and `scroll_cooldown_ms` config keys are removed and the mouse wheel no longer navigates tabs/panes. Existing layouts keep loading -- config parsing is total, so the now-unknown keys are simply ignored -- but they have no effect. No permission change.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR removes mouse-wheel tab/pane navigation and drag-to-reorder features entirely. ChangesGesture feature removal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR reverts and removes the previously added mouse-wheel tab/pane navigation feature, returning wheel behavior over the tab bar to inert (pre-#80) and eliminating the associated config surface and traversal logic.
Changes:
- Deletes the wheel traversal + cooldown limiter implementation and its unit tests (
src/scroll.rs). - Removes wheel event handling/state and all scroll-specific helpers/tests from the plugin state machine (
src/lib.rs). - Removes
scroll/scroll_cooldown_msconfig keys (and their docs/tests), and drops the now-scroll-only reading-order helper from projection (src/config.rs,src/projection.rs,README.md).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/scroll.rs | Removes the pure traversal + cooldown-gate module and its tests. |
| src/lib.rs | Removes wheel event handling, cooldown state, and pane/tab traversal helpers/tests. |
| src/config.rs | Removes scroll-related config keys, defaults, parsing, and tests. |
| src/projection.rs | Removes the scroll-only pane reading-order helper and its tests; keeps the tiled-pane filter. |
| README.md | Removes documentation for scroll and scroll_cooldown_ms. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
zellij never delivers the tab-drag Hold/Release events to a non-selectable `default_tab_template` bar, so drag-to-reorder was inert in normal use (#102) — the same "mouse gesture on a pinned bar" dead-end that sank wheel-scroll (#103). Rather than keep dead code and a dormant permission behind a config flag, remove the feature entirely. - config: drop the `reorder` key (parsing is total — existing layouts keep loading, the key is simply ignored). - permissions: the bar now always requests exactly two — `ReadApplicationState` + `ChangeApplicationState`; `RunActionsAsUser` is no longer requested. Dropping a permission is freeze-safe (zellij#4982 only bites when *adding* one). - remove `DragState`, the press/Hold/Release drag handling, and the `line::Shift` / `drag_steps` / drop-resolution layout math + tests. - a left click on a tab is now purely switch / focus / close / new-tab. `docs/design.md` keeps its original v2 D&D design notes as a historical record (internal Japanese planning doc). BREAKING CHANGE: the `reorder` config key is removed and the bar no longer requests the `RunActionsAsUser` permission. Layouts that set `reorder "true"` keep working (the key is ignored) and may drop it.
…108) (#109) #104 deleted wheel-scroll navigation wholesale when it rolled back the buggy timing-based cooldown (#83/#96/#100). Restore the navigation on its own, dropping the rate limiter that caused the trouble. `scroll` is a ScrollMode enum {tab (default), pane, off}: `tab` switches tabs (wrapping), `pane` walks the focused pane in reading order across tab boundaries (wrapping globally), `off` opts out. One wheel event maps to exactly one step — no Gate, no cooldown window, no Timer subscription. A stepless device (Magic Mouse, trackpad) bursts per flick and steps several at once; users who dislike that set `scroll = off` rather than living with a limiter that never reconciled both device classes. The pure traversal math (next_tab / next_pane / step) lives in a new dependency-free `scroll` module, mirroring the renderer's off-wasm testable discipline; lib.rs holds only the thin host-call dispatch. Rides the existing ChangeApplicationState grant — no new permission, so existing installs gain it on update without a re-grant (freeze-safe, zellij#4982).
Rolls back and completely removes the mouse-wheel tab/pane navigation
feature (
scroll+scroll_cooldown_ms, #80/#83). Closes #103.Why
zellij delivers
ScrollUp/ScrollDownwith no device identity and nophysical detent, so a notched wheel and a stepless device (Magic Mouse,
trackpad) are indistinguishable at the event level. A stepless device emits a
burst of events per flick, and no single rate-limiter value reconciles both
classes — a short cooldown still races through many tabs/panes, a long one locks
out deliberate input and lets momentum tails add stray steps. The successive
attempts each only moved the failure mode:
On a Magic Mouse / trackpad the bar still receives a flood of scroll events up
front and then stops responding. There is no
scroll_cooldown_msvalue thatbehaves correctly for both device classes, so the feature is removed rather than
tuned further. Full rationale in #103.
What's removed
src/scroll.rs— deleted in full (traversal +gaterate-limiter).src/lib.rs—scroll_coolingstate, theScrollUp/ScrollDown/Timerhandlers, the
EventType::Timersubscription,scroll/arm_scroll_cooldown/
scroll_tabs/scroll_panes/focused_pane_id/pane_focus_order, andthe wheel test suite + helpers.
src/config.rs— thescrollandscroll_cooldown_mskeys, their defaults,parsing, and tests.
src/projection.rs—pane_ids_in_reading_order(scroll-only) and its tests;is_tiled_terminalstays (still used byproject).README.md— thescroll/scroll_cooldown_msdocumentation.Compatibility / breaking
Config parsing is total, so existing layouts that still set
scroll/scroll_cooldown_mskeep loading — the now-unknown keys are simply ignored andhave no effect. The wheel over the bar returns to inert (pre-#80). No permission
change: wheel navigation needed nothing beyond the default set
(
ReadApplicationState+ChangeApplicationState).Verification
cargo build --target wasm32-wasip1— ✓cargo test --lib(host triple) — 268 passed, 0 failed ✓cargo clippy --lib— clean ✓cargo fmt --check— clean ✓Related
#80 (feature), #83 (cooldown), #96 / #97, #100 / #101 (fix attempts),
#77 (mouse-interaction tracking).
Summary by CodeRabbit