Skip to content

Add embedded split interaction policy - #167

Merged
austinywang merged 3 commits into
mainfrom
issue-7373-native-embedding
Jul 9, 2026
Merged

austinywang merged 3 commits into
mainfrom
issue-7373-native-embedding

Conversation

@austinywang

@austinywang austinywang commented Jul 9, 2026 •

Copy link
Copy Markdown

Supports embedded split trees that need native pane chrome without outer-window-only interaction surfaces.

  • make tab context menus configurable
  • gate shortcut monitor lifecycle when hints are disabled
  • make divider position bounds configurable
  • preserve existing defaults for current hosts

Required by manaflow-ai/cmux#7406.


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Summary by cubic

Adds an embedded split interaction policy with opt-in tab context menus, divider bounds, and shortcut hints, enforces tab-move rules across drag/drop and APIs, and clamps initial split positions to the configured range. Defaults are unchanged; required by manaflow-ai/cmux#7406.

  • New Features

    • BonsplitConfiguration.allowsTabContextMenu to enable/disable tab context menus (default: true).
    • BonsplitConfiguration.dividerPositionRange to constrain divider positions (normalized, default: 0.1...0.9). Clamping now honors this plus minimum pane size; initial split positions are clamped to this range.
    • Shortcut hint monitor only attaches/starts when shortcut hints are enabled; stops when disabled.
    • Tests cover defaults and allow programmatic narrow layouts (e.g., 0...1).
  • Bug Fixes

    • Honor allowTabReordering and allowCrossPaneTabMove in drag/drop validation, drops, and programmatic moves (reorderTab, moveTab). Manual reorder overlay is gated by allowTabReordering. Added tests to confirm behavior.

Written for commit 68158bd. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features

    • Added configurable divider positioning ranges, including support for the full split range.
    • Added a setting to enable or disable tab context menus.
    • Improved controls for tab reordering and moving tabs between panes.
  • Bug Fixes

    • Divider positions now consistently respect configured limits.
    • Tab drag-and-drop actions now correctly honor reordering and cross-pane movement settings.
    • Shortcut monitoring and reordering indicators only appear when enabled.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds configurable divider position ranges and tab interaction controls. Divider constraints propagate from public configuration through split rendering and coordinator calculations. Tab reordering, cross-pane moves, context menus, and shortcut monitoring now follow configuration flags, with coverage added for divider and tab behavior.

Changes

Configuration controls

Layer / File(s) Summary
Configurable divider bounds
Sources/Bonsplit/Public/BonsplitConfiguration.swift, Sources/Bonsplit/Public/BonsplitController.swift, Sources/Bonsplit/Public/BonsplitView.swift, Sources/Bonsplit/Internal/Views/SplitViewContainer.swift, Sources/Bonsplit/Internal/Views/SplitNodeView.swift, Sources/Bonsplit/Internal/Views/SplitContainerView.swift, Sources/Bonsplit/Internal/Controllers/SplitViewController.swift, Tests/BonsplitTests/EmbeddedConfigurationTests.swift
dividerPositionRange is normalized in configuration and propagated through split creation and rendering. Coordinator bounds combine pane-size feasibility with the configured range, while tests cover narrow divider positions.
Configurable tab interactions
Sources/Bonsplit/Public/BonsplitConfiguration.swift, Sources/Bonsplit/Public/BonsplitController.swift, Sources/Bonsplit/Internal/Views/TabBarView.swift, Sources/Bonsplit/Internal/Views/TabItemView.swift, Tests/BonsplitTests/EmbeddedConfigurationTests.swift
Tab movement APIs and drag delegates enforce reordering and cross-pane movement flags. Context-menu presentation, reorder tracking, and shortcut monitoring are conditional, with tests covering rejected tab mutations.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Configuration
  participant Controller
  participant SplitContainerView
  participant Coordinator
  Configuration->>Controller: configure dividerPositionRange
  Controller->>SplitContainerView: pass divider range
  SplitContainerView->>Coordinator: update range
  Coordinator->>Coordinator: normalize and clamp divider position
Loading
sequenceDiagram
  participant TabBarView
  participant TabDropDelegate
  participant BonsplitController
  TabBarView->>TabDropDelegate: validate local or external drop
  TabDropDelegate->>BonsplitController: evaluate movement flags
  BonsplitController->>TabDropDelegate: permit or reject operation
  TabDropDelegate->>TabBarView: perform permitted drop
Loading

Possibly related PRs

Suggested reviewers: azooz2003-bit

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main theme of the PR: adding an embedded split interaction policy.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-7373-native-embedding

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 9, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds embedded split interaction controls. The main changes are:

  • Configurable tab context menu installation.
  • Shortcut hint monitor lifecycle gated by hint availability.
  • Configurable split divider position bounds.
  • Default behavior preserved for existing hosts.
  • Tests for embedded configuration defaults and programmatic divider updates.

Confidence Score: 4/5

The initial split creation path needs a fix before merging.

  • Programmatic divider updates use the configured range.
  • Newly-created splits with initialDividerPosition can still use the old hard-coded bounds.
  • Context menu and shortcut hint lifecycle changes appear contained.

Sources/Bonsplit/Public/BonsplitConfiguration.swift and the split creation normalization path.

Important Files Changed

Filename Overview
Sources/Bonsplit/Public/BonsplitConfiguration.swift Adds public controls for tab context menus and divider bounds.
Sources/Bonsplit/Public/BonsplitController.swift Uses the configured divider range when setting divider positions programmatically.
Sources/Bonsplit/Internal/Views/SplitContainerView.swift Applies configured divider bounds through split view layout and reconciliation.
Sources/Bonsplit/Internal/Views/TabBarView.swift Gates shortcut hint monitor setup and passes context menu configuration to tab items.
Sources/Bonsplit/Internal/Views/TabItemView.swift Installs the context menu presenter only when context menus are enabled.
Tests/BonsplitTests/EmbeddedConfigurationTests.swift Adds tests for default embedded controls and widened divider ranges.

Reviews (1): Last reviewed commit: "Add embedded split interaction policy" | Re-trigger Greptile

Comment thread Sources/Bonsplit/Public/BonsplitConfiguration.swift
@austinywang
austinywang merged commit 8e2e058 into main Jul 9, 2026
5 checks passed
@austinywang
austinywang deleted the issue-7373-native-embedding branch July 9, 2026 23:49

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

🤖 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 `@Sources/Bonsplit/Internal/Views/SplitContainerView.swift`:
- Around line 625-639: The divider-position update logic incorrectly permits an
out-of-range divider because it compares the clamped value in stateBounds. In
the relevant divider adjustment method, compare the raw currentDividerPixels /
availableSize ratio against the configured bounds (or explicitly reject values
outside stateBounds) before the early return, then clamp and apply the corrected
position. Add a regression test covering a reused split view after its divider
range narrows.

In `@Sources/Bonsplit/Public/BonsplitController.swift`:
- Around line 601-609: Update normalizedInitialDividerPosition(_:) so a nil
position resolves to the midpoint of configuration.dividerPositionRange rather
than remaining nil; preserve clamping for explicit positions and return the
midpoint for empty/default splitPane calls, ensuring the result always respects
the configured bounds.
🪄 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

Run ID: 1dea442a-6c7a-4528-b837-40ebbafc99df

📥 Commits

Reviewing files that changed from the base of the PR and between fe53e04 and 68158bd.

📒 Files selected for processing (10)
  • Sources/Bonsplit/Internal/Controllers/SplitViewController.swift
  • Sources/Bonsplit/Internal/Views/SplitContainerView.swift
  • Sources/Bonsplit/Internal/Views/SplitNodeView.swift
  • Sources/Bonsplit/Internal/Views/SplitViewContainer.swift
  • Sources/Bonsplit/Internal/Views/TabBarView.swift
  • Sources/Bonsplit/Internal/Views/TabItemView.swift
  • Sources/Bonsplit/Public/BonsplitConfiguration.swift
  • Sources/Bonsplit/Public/BonsplitController.swift
  • Sources/Bonsplit/Public/BonsplitView.swift
  • Tests/BonsplitTests/EmbeddedConfigurationTests.swift

Comment thread Sources/Bonsplit/Internal/Views/SplitContainerView.swift
Comment thread Sources/Bonsplit/Public/BonsplitController.swift
hhsw2015 pushed a commit to hhsw2015/bonsplit that referenced this pull request Jul 10, 2026
Two follow-ups to manaflow-ai#167's dividerPositionRange contract:

- A nil initialDividerPosition fell through to the internal 0.5 fallback,
  bypassing a configured range that excludes 0.5. The default is now
  clamped exactly like an explicit position.
- syncPosition compared a pre-clamped current ratio, so a divider that
  physically drifted outside the range (non-drag AppKit resize) satisfied
  the early return and stayed out of range. The comparison now uses the
  raw pixel ratio so the re-assert path restores the model position.
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.

1 participant