Skip to content

Expose main content focus to tab chrome - #242

Closed
lawrencecchen wants to merge 1 commit into
mainfrom
feat-sidebar-focus-tab-state
Closed

lawrencecchen wants to merge 1 commit into
mainfrom
feat-sidebar-focus-tab-state

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Sep 15, 2026 •

Copy link
Copy Markdown

Problem

Hosts can move keyboard focus to an adjacent surface while keeping Bonsplit selection alive, but the tab chrome has no way to de-emphasize its selected indicator.

Change

Add an optional isMainContentFocused input to BonsplitView and thread it through nested pane containers. The selected tab remains selected, while focused tab chrome is desaturated when the host reports that another surface owns input.

Validation

swift test --package-path vendor/bonsplit builds successfully. One existing translated geometry test was flaky in this run.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Adds an isMainContentFocused input to BonsplitView so hosts can desaturate the selected tab chrome when keyboard focus moves to an adjacent surface.

  • The selected tab remains selected; only the tab chrome de-emphasizes when the host reports another surface owns input.
  • isMainContentFocused defaults to true, so existing callers are unaffected.

Written for commit de63f80. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Added an isMainContentFocused option to control whether the primary content receives keyboard focus while Bonsplit continues managing pane selection.
    • The option is available through both BonsplitView initializers and defaults to enabled.
    • Focus behavior is consistently applied across panes and nested split views.

@coderabbitai

coderabbitai Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

Warning

Review limit reached

Next included review available in 42 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 31569d05-fad3-44eb-87ac-ee2d579f46db

📥 Commits

Reviewing files that changed from the base of the PR and between a708f09 and de63f80.

📒 Files selected for processing (1)
  • Sources/Bonsplit/Internal/Views/SplitContainerView.swift

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: bed5f78f-0113-458c-984c-db4a461f695c

📥 Commits

Reviewing files that changed from the base of the PR and between 4679197 and a708f09.

📒 Files selected for processing (5)
  • Sources/Bonsplit/Internal/Views/PaneContainerView.swift
  • Sources/Bonsplit/Internal/Views/SplitContainerView.swift
  • Sources/Bonsplit/Internal/Views/SplitNodeView.swift
  • Sources/Bonsplit/Internal/Views/SplitViewContainer.swift
  • Sources/Bonsplit/Public/BonsplitView.swift

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Bonsplit adds an isMainContentFocused option to its public view initializers. The value propagates through split containers to pane views. Pane focus now also requires main-content focus.

Changes

Main content focus control

Layer / File(s) Summary
Focus option and view hierarchy wiring
Sources/Bonsplit/Public/BonsplitView.swift, Sources/Bonsplit/Internal/Views/SplitViewContainer.swift, Sources/Bonsplit/Internal/Views/SplitNodeView.swift
BonsplitView exposes isMainContentFocused in both initializers. The value is stored and passed through the split view hierarchy.
Focus state application
Sources/Bonsplit/Internal/Views/SplitContainerView.swift, Sources/Bonsplit/Internal/Views/PaneContainerView.swift
Split containers pass the focus state to panes and nested split containers. A pane is focused only when main-content focus is enabled and the pane matches the controller’s focused pane.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Suggested reviewers: austinywang

Merge Risk: ⚪ Minimal · up to a708f

The new option changes tab chrome when focus leaves the main content while preserving the selected tab.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 5 files. 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 describes the main change: exposing main content focus so tab chrome can reflect focus state.
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 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-sidebar-focus-tab-state

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.

@lawrencecchen

Copy link
Copy Markdown
Author

Superseded by #264, which combines this change with the other right sidebar Bonsplit change for manaflow-ai/cmux#15534.

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