Repository navigation
Per-pane full-width tab mode with prominent title header - #159
Conversation
When enabled for a pane, the header renders a prominent full-width title of the selected tab instead of the tab strip: same height, live title updates, subtle index/count badge when extra tabs exist, right-click opens the selected tab's context menu (including the exit item), no new-tab button. New TabContextAction.toggleFullWidthTab and per-pane BonsplitController API (set/get/toggle/list), en+ja strings, tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds per-pane full-width tab mode, a dedicated header view, context menu support, drag handling helpers, controller APIs, localization, and tests. ChangesFull Width Tab Mode
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
Consumer PR: manaflow-ai/cmux#7425 |
Greptile SummaryThis PR introduces a per-pane
Confidence Score: 3/5The core view and API are sound, but the advertised right-click exit path is a silent no-op without explicit host-delegate wiring, and full-width mode ignores the tabBarVisibility = .never configuration. Two real defects exist in the changed code. First, requestTabContextAction(.toggleFullWidthTab) has no internal fallback: the PR documented exit path silently does nothing until the consuming host delegate calls toggleFullWidthTabMode(inPane:) — a step not yet done. Second, PaneContainerView checks isFullWidthTabMode before the tabBarVisibility guard, so a pane configured with tabBarVisibility = .never will unexpectedly show the full-width header once the mode is enabled. BonsplitController.swift (missing internal handler for .toggleFullWidthTab) and PaneContainerView.swift (full-width check bypasses tabBarVisibility). Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant FullWidthTabHeaderView
participant TabContextMenuPresenter
participant BonsplitController
participant Delegate as Host Delegate
User->>FullWidthTabHeaderView: right-click
FullWidthTabHeaderView->>TabContextMenuPresenter: show context menu
TabContextMenuPresenter-->>User: menu with "Exit Full Width Tab"
User->>TabContextMenuPresenter: select "Exit Full Width Tab"
TabContextMenuPresenter->>FullWidthTabHeaderView: onContextAction(.toggleFullWidthTab)
FullWidthTabHeaderView->>BonsplitController: requestTabContextAction(.toggleFullWidthTab, ...)
BonsplitController->>Delegate: didRequestTabContextAction(.toggleFullWidthTab, ...)
Note over Delegate: Host must call toggleFullWidthTabMode(inPane:) — no internal fallback
Delegate-->>BonsplitController: toggleFullWidthTabMode(inPane:)
BonsplitController-->>FullWidthTabHeaderView: "pane.isFullWidthTabMode = false"
FullWidthTabHeaderView-->>User: reverts to TabBarView
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant FullWidthTabHeaderView
participant TabContextMenuPresenter
participant BonsplitController
participant Delegate as Host Delegate
User->>FullWidthTabHeaderView: right-click
FullWidthTabHeaderView->>TabContextMenuPresenter: show context menu
TabContextMenuPresenter-->>User: menu with "Exit Full Width Tab"
User->>TabContextMenuPresenter: select "Exit Full Width Tab"
TabContextMenuPresenter->>FullWidthTabHeaderView: onContextAction(.toggleFullWidthTab)
FullWidthTabHeaderView->>BonsplitController: requestTabContextAction(.toggleFullWidthTab, ...)
BonsplitController->>Delegate: didRequestTabContextAction(.toggleFullWidthTab, ...)
Note over Delegate: Host must call toggleFullWidthTabMode(inPane:) — no internal fallback
Delegate-->>BonsplitController: toggleFullWidthTabMode(inPane:)
BonsplitController-->>FullWidthTabHeaderView: "pane.isFullWidthTabMode = false"
FullWidthTabHeaderView-->>User: reverts to TabBarView
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if pane.isFullWidthTabMode, !pane.tabs.isEmpty { | ||
| FullWidthTabHeaderView( | ||
| pane: pane, | ||
| isFocused: isFocused | ||
| ) | ||
| } else if tabBarVisibility.showsTabBar(tabCount: pane.tabs.count) { |
There was a problem hiding this comment.
FullWidthTabHeaderView bypasses tabBarVisibility configuration
The full-width header check is placed before the tabBarVisibility.showsTabBar(...) guard, so it fires regardless of the configured visibility policy. If a host app sets tabBarVisibility = .never (or uses an .auto/threshold variant that would hide the bar for the current tab count), calling setFullWidthTabMode(true, inPane:) will still render a header — overriding the app's explicit "no tab chrome" decision. The guard should respect the same visibility policy: isFullWidthTabMode should only take effect when tabBarVisibility would otherwise allow chrome to be shown, or at a minimum should be gated on tabBarVisibility != .never.
| .contentShape(Rectangle()) | ||
| .onTapGesture { | ||
| withTransaction(Transaction(animation: nil)) { | ||
| controller.focusPane(pane.id) | ||
| } | ||
| } |
There was a problem hiding this comment.
contentShape(Rectangle()) applied twice
.contentShape(Rectangle()) appears both on the inner content view at line 43 and again on the outer Group at line 78. The outer modifier supersedes the inner one, making the inner one a no-op. Consider removing the one at line 43 to keep the modifier hierarchy clear.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![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)
Sources/Bonsplit/Internal/Views/FullWidthTabHeaderView.swift (2)
105-125: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winIndex/count badge is invisible to VoiceOver.
.accessibilityElement(children: .ignore)collapses the whole header to justtab.title, so the visually-shown "index/count" pill (Lines 105-120) is never announced to assistive tech.♿️ Proposed fix
- .accessibilityElement(children: .ignore) - .accessibilityLabel(tab.title) + .accessibilityElement(children: .ignore) + .accessibilityLabel( + pane.tabs.count > 1 + ? "\(tab.title), tab \(index + 1) of \(pane.tabs.count)" + : tab.title + )🤖 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 `@Sources/Bonsplit/Internal/Views/FullWidthTabHeaderView.swift` around lines 105 - 125, The index/count badge in FullWidthTabHeaderView is hidden from VoiceOver because the header is collapsed to only tab.title by accessibilityElement(children: .ignore). Update the accessibility setup for the header HStack so the badge text is exposed as part of the accessible content, either by combining the title and "\(index + 1)/\(pane.tabs.count)" into the accessibility label or by making the badge its own accessible element while keeping the tab header readable.
92-92: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFont sizes derived from
appearance.tabTitleFontSizearen't fully guarded against non-finite values.Line 92 uses
appearance.tabTitleFontSize + 2directly with no clamp at all, and Lines 109/129 usemax(...), which only protects againstNaN(the non-NaNoperand wins) but not against+infinitypropagating through to.font(size:)/NSFontcreation if a host misconfigurestabTitleFontSize.Based on learnings, a prior review of
TabBarTypography.swiftin this repo flagged this exactmax()behavior as insufficient protection against non-finite font sizes reachingNSFont.Also applies to: 109-109, 127-146
🤖 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 `@Sources/Bonsplit/Internal/Views/FullWidthTabHeaderView.swift` at line 92, Guard all font-size calculations in FullWidthTabHeaderView against non-finite values, not just NaN. Update the uses of appearance.tabTitleFontSize in the font-building paths (including the .font call in the title view and the other size computations later in the view) so they sanitize or clamp to a finite default before passing into .font(size:) or NSFont creation. Use the existing font-size code paths in FullWidthTabHeaderView as the fix location and ensure any arithmetic like tabTitleFontSize + 2 cannot propagate infinity.Source: Learnings
🤖 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/FullWidthTabHeaderView.swift`:
- Around line 127-146: The custom image path in tabIcon(for:) applies no
saturation adjustment, so icons loaded from tab.iconImageData stay fully colored
when the view is unfocused. Update the NSImage-based branch to apply the same
saturation behavior used for the SF Symbol branch, using the existing saturation
value and iconTint/appearance state so both image types dim consistently.
---
Nitpick comments:
In `@Sources/Bonsplit/Internal/Views/FullWidthTabHeaderView.swift`:
- Around line 105-125: The index/count badge in FullWidthTabHeaderView is hidden
from VoiceOver because the header is collapsed to only tab.title by
accessibilityElement(children: .ignore). Update the accessibility setup for the
header HStack so the badge text is exposed as part of the accessible content,
either by combining the title and "\(index + 1)/\(pane.tabs.count)" into the
accessibility label or by making the badge its own accessible element while
keeping the tab header readable.
- Line 92: Guard all font-size calculations in FullWidthTabHeaderView against
non-finite values, not just NaN. Update the uses of appearance.tabTitleFontSize
in the font-building paths (including the .font call in the title view and the
other size computations later in the view) so they sanitize or clamp to a finite
default before passing into .font(size:) or NSFont creation. Use the existing
font-size code paths in FullWidthTabHeaderView as the fix location and ensure
any arithmetic like tabTitleFontSize + 2 cannot propagate infinity.
🪄 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: 9bca9f42-e355-4f6f-acf9-d7986ed6dac6
📒 Files selected for processing (10)
Sources/Bonsplit/Internal/Models/PaneState.swiftSources/Bonsplit/Internal/Views/FullWidthTabHeaderView.swiftSources/Bonsplit/Internal/Views/PaneContainerView.swiftSources/Bonsplit/Internal/Views/TabBarView.swiftSources/Bonsplit/Internal/Views/TabItemView.swiftSources/Bonsplit/Public/BonsplitController.swiftSources/Bonsplit/Public/Types/TabContextAction.swiftSources/Bonsplit/Resources/en.lproj/Localizable.stringsSources/Bonsplit/Resources/ja.lproj/Localizable.stringsTests/BonsplitTests/BonsplitTests.swift
Dragging the header starts a drag of the selected tab via a shared SplitViewController+TabDrag helper (same provider, preview, drag-state bookkeeping, and cancelled-drag cleanup as the tab strip; strip and manual reorder now route through it too). The header accepts .tabTransfer/.fileURL drops appended at the end of the pane, with a hover highlight honoring focus dim and saturation. Restores the one-shot monitor's capture-box nil-out so cancelled drags don't leak the monitor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Picks up manaflow-ai/bonsplit#159 round 2: header drag-out via shared drag-state helper, drop-in append with hover highlight, cancelled-drag monitor leak fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Tests/BonsplitTests/BonsplitTests.swift (1)
3917-3956: 📐 Maintainability & Code Quality | 🔵 TrivialConsider extracting shared window/hosting-view setup for render helpers.
renderedFullWidthHeaderIndicatorSaturationandrenderedFullWidthPaneChromeAlphaduplicate the same ~20-lineNSWindow/NSHostingView/layout-settle boilerplate already present inrenderedTabBarIndicatorSaturationandrenderedPaneContainerHasTabBar. Extracting a shared helper (e.g., taking a root SwiftUI view and returning the configuredhostingView) would reduce duplication as more full-width rendering tests are added.Also applies to: 4661-4716
🤖 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 `@Tests/BonsplitTests/BonsplitTests.swift` around lines 3917 - 3956, The render helper in renderedFullWidthHeaderIndicatorSaturation duplicates the same NSWindow/NSHostingView setup used by renderedFullWidthPaneChromeAlpha, renderedTabBarIndicatorSaturation, and renderedPaneContainerHasTabBar. Extract the shared window/hosting-view/layout boilerplate into a common helper that accepts the root SwiftUI view (or similar) and returns the configured hosting view, then update these helpers to call it and keep only their unique sampling logic.
🤖 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.
Nitpick comments:
In `@Tests/BonsplitTests/BonsplitTests.swift`:
- Around line 3917-3956: The render helper in
renderedFullWidthHeaderIndicatorSaturation duplicates the same
NSWindow/NSHostingView setup used by renderedFullWidthPaneChromeAlpha,
renderedTabBarIndicatorSaturation, and renderedPaneContainerHasTabBar. Extract
the shared window/hosting-view/layout boilerplate into a common helper that
accepts the root SwiftUI view (or similar) and returns the configured hosting
view, then update these helpers to call it and keep only their unique sampling
logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f487c4ed-8797-4fc3-8973-7bd179c65be7
📒 Files selected for processing (6)
Sources/Bonsplit/Internal/Views/FullWidthTabHeaderView.swiftSources/Bonsplit/Internal/Views/PaneContainerView.swiftSources/Bonsplit/Internal/Views/TabBarView.swiftSources/Bonsplit/Internal/Views/TabItemView.swiftSources/Bonsplit/Public/BonsplitController.swiftTests/BonsplitTests/BonsplitTests.swift
🚧 Files skipped from review as they are similar to previous changes (5)
- Sources/Bonsplit/Internal/Views/TabItemView.swift
- Sources/Bonsplit/Internal/Views/PaneContainerView.swift
- Sources/Bonsplit/Public/BonsplitController.swift
- Sources/Bonsplit/Internal/Views/TabBarView.swift
- Sources/Bonsplit/Internal/Views/FullWidthTabHeaderView.swift
* Add per-pane Full Width Tab mode (palette + tab context menu) Pane-level analog of pane zoom for single-tab workflows: toggling Full Width Tab makes the bonsplit pane header show a prominent full-width title of the selected tab instead of the tab strip. Exposed via the command palette (palette.toggleFullWidthTab, en+ja) and the tab context menu; no keyboard shortcut in this pass. State lives per pane in bonsplit and persists across restarts via SessionPaneLayoutSnapshot.isFullWidthTabMode (additive optional field, legacy snapshots decode unchanged). Logic lives in Workspace+FullWidthTab.swift with a thin delegate case in Workspace.swift; TabManager.toggleFocusedFullWidthTab mirrors the zoom helper. Pins vendor/bonsplit to the header/API change (manaflow-ai/bonsplit#159). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Bump vendor/bonsplit: full-width header tab drag and drop Picks up manaflow-ai/bonsplit#159 round 2: header drag-out via shared drag-state helper, drop-in append with hover highlight, cancelled-drag monitor leak fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Bump vendor/bonsplit to merged full-width header * Use Swift Testing for full width tab tests * Expose full width tab toggle via tab action * Split full width tab action test fake * Preserve full width tab toggle success semantics * Use normal tab chrome for full-width tabs * Report full width tab toggle failures --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
Adds a per-pane
fullWidthTabMode: when enabled, the pane header renders a prominent full-width title of the selected tab instead of the tab strip. Same height as the tab bar, live title updates, subtle index/count badge when more tabs exist, right-click anywhere on the header opens the selected tab's context menu (the exit path), no new-tab button. AddsTabContextAction.toggleFullWidthTab, per-pane controller API (setFullWidthTabMode/isFullWidthTabMode/toggleFullWidthTabMode/fullWidthTabModePaneIds), en+ja strings, and tests (163 passing).Consumed by the cmux PR that wires this to the command palette and tab context menu (link added there once open).
🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds a per-pane full-width tab mode that replaces the tab strip with a prominent, full-width title header for the selected tab. The header matches tab chrome, supports drag & drop, keeps key tab/pane actions, and toggling from context now selects the target tab first; it’s opt-in per pane.
New Features
toggleFullWidthTabitem (label switches between “Full Width Tab” and “Exit Full Width Tab”); adds a tab switcher menu via the badge and a pane actions menu for configured split buttons (BonsplitConfiguration.splitButtons)..tabTransfer/.fileURLappends at the end with a focus‑aware hover highlight. A sharedSplitViewController+TabDraghelper is used by the header, tab strip, and manual reorder.BonsplitControllerhandlesTabContextAction.toggleFullWidthTab; addsonTabFullWidthToggleRequestfor hosts, plus per‑pane APIs:setFullWidthTabMode(_:inPane:),isFullWidthTabMode(inPane:),toggleFullWidthTabMode(inPane:),fullWidthTabModePaneIds; en/ja strings and tests.Bug Fixes
Written for commit f83584b. Summary will update on new commits.
Summary by CodeRabbit