Splitpane mouse drag feature#2322
Conversation
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesSplitPane divider dragging
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/widgets/src/layout/SplitPane.ts (2)
104-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
MouseEventat the top of the file instead of using an inline import.The file already imports
KeyEvent,Screen, and other shared types from@termuijs/core. Usingimport('@termuijs/core').MouseEventinline is non-idiomatic and inconsistent. As per coding guidelines, shared types should be imported from@termuijs/core.♻️ Proposed refactor
Add
MouseEventto the existing import from@termuijs/coreat the top of the file:- handleMouse(event: import('`@termuijs/core`').MouseEvent): void { + handleMouse(event: MouseEvent): void {And update the top-level import (adjust to match the existing import statement):
-import { type KeyEvent, type Screen, ... } from '`@termuijs/core`'; +import { type KeyEvent, type MouseEvent, type Screen, ... } from '`@termuijs/core`';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widgets/src/layout/SplitPane.ts` at line 104, Import MouseEvent alongside the existing KeyEvent, Screen, and other symbols from `@termuijs/core`, then update SplitPane.handleMouse to use the imported MouseEvent type instead of the inline import.Source: Coding guidelines
107-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeduplicate the horizontal and vertical branches.
The two branches are structurally identical, differing only in axis (
x/widthvsy/height). Consolidating them reduces maintenance burden and the risk of the branches diverging.♻️ Proposed refactor
if (width <= 0 || height <= 0) return; - if (this._direction === 'horizontal') { - const dividerX = x + Math.floor(this._ratio * width); - - if (event.type === 'mousedown' && event.button === 'left') { - if (event.x === dividerX) { - this._dragging = true; - } - } else if ((event.type === 'mousemove' || event.type === 'drag') && this._dragging) { - const newRatio = (event.x - x) / width; - this.setRatio(newRatio); - } else if (event.type === 'mouseup' || event.type === 'dragend') { - this._dragging = false; - } - } else { - const dividerY = y + Math.floor(this._ratio * height); - - if (event.type === 'mousedown' && event.button === 'left') { - if (event.y === dividerY) { - this._dragging = true; - } - } else if ((event.type === 'mousemove' || event.type === 'drag') && this._dragging) { - const newRatio = (event.y - y) / height; - this.setRatio(newRatio); - } else if (event.type === 'mouseup' || event.type === 'dragend') { - this._dragging = false; - } - } + const isHorizontal = this._direction === 'horizontal'; + const pos = isHorizontal ? event.x : event.y; + const origin = isHorizontal ? x : y; + const size = isHorizontal ? width : height; + const dividerPos = origin + Math.floor(this._ratio * size); + + if (event.type === 'mousedown' && event.button === 'left') { + if (pos === dividerPos) { + this._dragging = true; + } + } else if ((event.type === 'mousemove' || event.type === 'drag') && this._dragging) { + this.setRatio((pos - origin) / size); + } else if (event.type === 'mouseup' || event.type === 'dragend') { + this._dragging = false; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widgets/src/layout/SplitPane.ts` around lines 107 - 133, Deduplicate the axis-specific logic in the SplitPane event handler by selecting the relevant coordinate and dimension based on this._direction, then sharing one mousedown, move/drag, and mouseup/dragend flow. Preserve divider detection, ratio calculation, setRatio, and dragging state behavior while referencing the unified axis values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/widgets/src/layout/SplitPane.ts`:
- Around line 105-116: Prevent invalid ratio updates in SplitPane.handleMouse by
guarding horizontal drag calculations when width is non-positive, matching the
existing safeguards in _renderSelf and _positionChildren. Skip the newRatio
calculation and setRatio call when the content width is zero or less; apply the
equivalent height guard for vertical dragging if it performs the same division.
---
Nitpick comments:
In `@packages/widgets/src/layout/SplitPane.ts`:
- Line 104: Import MouseEvent alongside the existing KeyEvent, Screen, and other
symbols from `@termuijs/core`, then update SplitPane.handleMouse to use the
imported MouseEvent type instead of the inline import.
- Around line 107-133: Deduplicate the axis-specific logic in the SplitPane
event handler by selecting the relevant coordinate and dimension based on
this._direction, then sharing one mousedown, move/drag, and mouseup/dragend
flow. Preserve divider detection, ratio calculation, setRatio, and dragging
state behavior while referencing the unified axis values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 944a350e-80d5-410c-847f-bfec2f2b3837
📒 Files selected for processing (1)
packages/widgets/src/layout/SplitPane.ts
1 participant
Description
This PR fully implements interactive mouse dragging for the
SplitPanelayout widget in@termuijs/widgets. Previously, users could only resize the panes if the widget had terminal focus by holding Shift+Arrows. This commit introduces ahandleMouselistener that dynamically tracks the divider's cell coordinates and manages a_draggingstate machine acrossmousedown,drag, andmouseupevents, allowing fluid, continuous mouse adjustments to the layout ratio.Related Issue
Closes #2261
Which package(s)?
@termuijs/widgetsType of Change
type:bug)type:feature)type:docs)type:testing)type:refactor)type:design)type:accessibility)type:performance)type:devops)type:security)Checklist
needs-starcheck blocks your merge otherwise.bun vitest runbun run buildbun run typecheckCONTRIBUTING.md.type: short description.markDirty()(if your change affects rendering).anytypes without an inline comment explaining why.GSSoC 2026 Participation
https://gssoc.girlscript.org/profile/YOUR_PROFILE_ID_HEREScreenshots / Recordings (UI changes)
Notes for the Reviewer
The coordinates are dynamically derived inside
handleMouserather than caching absolute values, ensuring that the drag interaction area remains perfectly accurate even if the terminal window is resized mid-drag!Summary by CodeRabbit