Repository navigation
feat(tabbar): collapse split buttons into an overflow menu - #142
austinywang wants to merge 1 commit into
Conversation
Add Appearance.collapseSplitButtonsIntoMenu; when set, the tab bar's right-side action buttons render as a single ellipsis menu (labels from each button's tooltip) instead of a row, with the action lane sized for one button. Reuses performSplitActionButton dispatch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces a split-button menu collapse feature. The tab bar's right-side action buttons now collapse into a single overflow "⋯" menu when enabled via ChangesSplit-button menu collapse
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Greptile SummaryThis PR adds
Confidence Score: 3/5Safe to merge for the common case, but accessibility identifiers on the individual action buttons break silently when the menu is active, which could blind existing UI automation. The core layout logic (effectiveSplitButtonCount, lane sizing, action dispatch) is correct. Accessibility identifiers placed on Button views inside Menu {} on macOS are bridged to NSMenuItem and typically do not surface in the AX hierarchy under the same identifier path, so existing automation targeting paneTabBarControl.newTerminal etc. will silently miss when the flag is on. The ellipsis trigger is also hardcoded at 12pt rather than scaling with controlIconFontScale, creating a visual mismatch when tabTitleFontSize is customized. TabBarView.swift — specifically the splitButtonOverflowMenu function and the accessibility identifier modifiers applied to menu item Buttons. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[splitButtonRow] --> B{collapseSplitButtonsIntoMenu AND buttons not empty?}
B -- Yes --> C[splitButtonOverflowMenu single menu button]
B -- No --> D[HStack of individual SplitActionButtons]
C --> E[Menu items via ForEach using splitActionButtonMenuLabel]
E --> F[performSplitActionButton dispatch]
D --> F
G[effectiveSplitButtonCount] --> H{collapseSplitButtonsIntoMenu?}
H -- Yes, count gt 0 --> I[returns 1]
H -- No --> J[returns visibleSplitButtons.count]
I --> K[TabBarLayout sizes lane for 1 button]
J --> K
Reviews (1): Last reviewed commit: "feat(tabbar): collapse split buttons int..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ForEach(buttons.indices, id: \.self) { index in | ||
| let button = buttons[index] | ||
| Button { | ||
| performSplitActionButton(button) | ||
| } label: { | ||
| splitActionButtonIcon(button.icon) | ||
| splitActionButtonMenuLabel(button, tooltips: tooltips) | ||
| } | ||
| .buttonStyle(SplitActionButtonStyle(appearance: appearance, layout: tabBarLayout)) | ||
| .accessibilityIdentifier(splitActionButtonAccessibilityIdentifier(button)) |
There was a problem hiding this comment.
On macOS, Button views inside a SwiftUI Menu {} block are bridged to NSMenuItem rather than rendered as standalone NSViews. .accessibilityIdentifier(_:) modifiers applied to those Button instances typically do not propagate to the resulting NSMenuItem's AXIdentifier in the accessibility hierarchy — the accessibility tree for NSMenuItems is structured differently from regular view hierarchies. Any existing XCTest UI-automation that queries paneTabBarControl.newTerminal, paneTabBarControl.splitRight, etc. by identifier will silently fail to find the elements when collapseSplitButtonsIntoMenu is true, because those identifiers will no longer exist in the expected place in the tree.
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!
| } label: { | ||
| Image(systemName: "ellipsis") | ||
| .font(.system(size: 12)) | ||
| } | ||
| .menuStyle(.button) |
There was a problem hiding this comment.
12 * controlIconFontScale so it adapts when the host app sets a custom tabTitleFontSize. With this change, enabling the overflow menu alongside a non-default font size will produce an icon that is visually out of proportion with the rest of the tab bar controls.
| } label: { | |
| Image(systemName: "ellipsis") | |
| .font(.system(size: 12)) | |
| } | |
| .menuStyle(.button) | |
| } label: { | |
| Image(systemName: "ellipsis") | |
| .font(.system(size: 12 * controlIconFontScale)) | |
| } | |
| .menuStyle(.button) |
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!
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/Bonsplit/Internal/Views/TabBarView.swift">
<violation number="1" location="Sources/Bonsplit/Internal/Views/TabBarView.swift:1783">
P3: Scale the overflow ellipsis icon with `controlIconFontScale` so it remains proportionate when the host app uses a non-default tab title font size.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } | ||
| } label: { | ||
| Image(systemName: "ellipsis") | ||
| .font(.system(size: 12)) |
There was a problem hiding this comment.
P3: Scale the overflow ellipsis icon with controlIconFontScale so it remains proportionate when the host app uses a non-default tab title font size.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Bonsplit/Internal/Views/TabBarView.swift, line 1783:
<comment>Scale the overflow ellipsis icon with `controlIconFontScale` so it remains proportionate when the host app uses a non-default tab title font size.</comment>
<file context>
@@ -1727,22 +1736,73 @@ struct TabBarView: View {
}
+ } label: {
+ Image(systemName: "ellipsis")
+ .font(.system(size: 12))
+ }
+ .menuStyle(.button)
</file context>
| .font(.system(size: 12)) | |
| .font(.system(size: 12 * controlIconFontScale)) |
1 participant
Adds
Appearance.collapseSplitButtonsIntoMenu; when set, the tab bar's right-side action buttons render as a single ellipsis (⋯) menu (labels from each button's tooltip) instead of a row, with the action lane sized for one button. ReusesperformSplitActionButtondispatch. Pinned by cmux PR #4332.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds an option to collapse the tab bar’s right-side action buttons into a single "⋯" overflow menu. Reserves space for one button and aligns with the overflow-menu requirement in Linear 4331.
New Features
Appearance.collapseSplitButtonsIntoMenu(default false).Menuwith labels from each button’s tooltip and reusesperformSplitActionButton.paneTabBarControl.overflowMenu.Migration
appearance.collapseSplitButtonsIntoMenu = trueto enable.Written for commit 5be7109. Summary will update on new commits.
Summary by CodeRabbit