Repository navigation
feat(tabbar): collapse split buttons into an overflow menu #142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -959,7 +959,7 @@ struct TabBarView: View { | |||||||||||||||||||||
| tabBarHeight: appearance.tabBarHeight, | ||||||||||||||||||||||
| availableWidth: containerWidth, | ||||||||||||||||||||||
| tabContentWidthExcludingSplitButtonLane: tabContentWidthExcludingSplitButtonLane, | ||||||||||||||||||||||
| splitButtonCount: visibleSplitButtons.count, | ||||||||||||||||||||||
| splitButtonCount: effectiveSplitButtonCount, | ||||||||||||||||||||||
| splitButtonLaneVisible: shouldShowSplitButtons, | ||||||||||||||||||||||
| reservesSplitButtonLane: showSplitButtons && !isMinimalMode, | ||||||||||||||||||||||
| measuredSplitButtonLaneWidth: measuredSplitButtonLaneWidth | ||||||||||||||||||||||
|
|
@@ -981,6 +981,15 @@ struct TabBarView: View { | |||||||||||||||||||||
| return appearance.splitButtons | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /// Effective button count for reserving the trailing action lane. When the | ||||||||||||||||||||||
| /// buttons are collapsed into the "⋯" overflow menu, only that single menu | ||||||||||||||||||||||
| /// button occupies the lane, so reserve width for one — not all of them. | ||||||||||||||||||||||
| private var effectiveSplitButtonCount: Int { | ||||||||||||||||||||||
| let count = visibleSplitButtons.count | ||||||||||||||||||||||
| guard count > 0 else { return 0 } | ||||||||||||||||||||||
| return appearance.collapseSplitButtonsIntoMenu ? 1 : count | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| private var shouldRenderSplitButtons: Bool { | ||||||||||||||||||||||
| !visibleSplitButtons.isEmpty | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
@@ -1727,22 +1736,73 @@ struct TabBarView: View { | |||||||||||||||||||||
| private var splitButtonRow: some View { | ||||||||||||||||||||||
| let tooltips = controller.configuration.appearance.splitButtonTooltips | ||||||||||||||||||||||
| let buttons = visibleSplitButtons | ||||||||||||||||||||||
| HStack(spacing: TabBarStyling.splitButtonsSpacing) { | ||||||||||||||||||||||
| Group { | ||||||||||||||||||||||
| if controller.configuration.appearance.collapseSplitButtonsIntoMenu, !buttons.isEmpty { | ||||||||||||||||||||||
| splitButtonOverflowMenu(buttons: buttons, tooltips: tooltips) | ||||||||||||||||||||||
| } else { | ||||||||||||||||||||||
| HStack(spacing: TabBarStyling.splitButtonsSpacing) { | ||||||||||||||||||||||
| ForEach(buttons.indices, id: \.self) { index in | ||||||||||||||||||||||
| let button = buttons[index] | ||||||||||||||||||||||
| Button { | ||||||||||||||||||||||
| performSplitActionButton(button) | ||||||||||||||||||||||
| } label: { | ||||||||||||||||||||||
| splitActionButtonIcon(button.icon) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| .buttonStyle(SplitActionButtonStyle(appearance: appearance, layout: tabBarLayout)) | ||||||||||||||||||||||
| .accessibilityIdentifier(splitActionButtonAccessibilityIdentifier(button)) | ||||||||||||||||||||||
| .safeHelp(splitActionButtonTooltip(button, tooltips: tooltips)) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| .padding(.leading, TabBarStyling.splitButtonsLeadingPadding) | ||||||||||||||||||||||
| .padding(.trailing, TabBarStyling.splitButtonsTrailingPadding) | ||||||||||||||||||||||
| .frame(height: tabBarHeight, alignment: .center) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /// Single "⋯" button that expands the split actions into a menu. Reuses the | ||||||||||||||||||||||
| /// exact same `performSplitActionButton` dispatch as the inline row, so | ||||||||||||||||||||||
| /// every action (including custom ones like New Note) behaves identically. | ||||||||||||||||||||||
| @ViewBuilder | ||||||||||||||||||||||
| private func splitButtonOverflowMenu( | ||||||||||||||||||||||
| buttons: [BonsplitConfiguration.SplitActionButton], | ||||||||||||||||||||||
| tooltips: BonsplitConfiguration.SplitButtonTooltips | ||||||||||||||||||||||
| ) -> some View { | ||||||||||||||||||||||
| Menu { | ||||||||||||||||||||||
| 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)) | ||||||||||||||||||||||
| .safeHelp(splitActionButtonTooltip(button, tooltips: tooltips)) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } label: { | ||||||||||||||||||||||
| Image(systemName: "ellipsis") | ||||||||||||||||||||||
| .font(.system(size: 12)) | ||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: Scale the overflow ellipsis icon with Prompt for AI agents
Suggested change
|
||||||||||||||||||||||
| } | ||||||||||||||||||||||
| .menuStyle(.button) | ||||||||||||||||||||||
|
Comment on lines
+1781
to
+1785
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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! |
||||||||||||||||||||||
| .menuIndicator(.hidden) | ||||||||||||||||||||||
| .buttonStyle(SplitActionButtonStyle(appearance: appearance, layout: tabBarLayout)) | ||||||||||||||||||||||
| .fixedSize() | ||||||||||||||||||||||
| .accessibilityIdentifier("paneTabBarControl.overflowMenu") | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| @ViewBuilder | ||||||||||||||||||||||
| private func splitActionButtonMenuLabel( | ||||||||||||||||||||||
| _ button: BonsplitConfiguration.SplitActionButton, | ||||||||||||||||||||||
| tooltips: BonsplitConfiguration.SplitButtonTooltips | ||||||||||||||||||||||
| ) -> some View { | ||||||||||||||||||||||
| let title = splitActionButtonTooltip(button, tooltips: tooltips) | ||||||||||||||||||||||
| switch button.icon { | ||||||||||||||||||||||
| case .systemImage(let name): | ||||||||||||||||||||||
| Label(title, systemImage: name) | ||||||||||||||||||||||
| case .emoji(let value, _): | ||||||||||||||||||||||
| Text("\(value) \(title)") | ||||||||||||||||||||||
| case .imageData: | ||||||||||||||||||||||
| Text(title) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| .padding(.leading, TabBarStyling.splitButtonsLeadingPadding) | ||||||||||||||||||||||
| .padding(.trailing, TabBarStyling.splitButtonsTrailingPadding) | ||||||||||||||||||||||
| .frame(height: tabBarHeight, alignment: .center) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| private func splitActionButtonAccessibilityIdentifier(_ button: BonsplitConfiguration.SplitActionButton) -> String { | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On macOS,
Buttonviews inside a SwiftUIMenu {}block are bridged toNSMenuItemrather than rendered as standaloneNSViews..accessibilityIdentifier(_:)modifiers applied to thoseButtoninstances typically do not propagate to the resultingNSMenuItem'sAXIdentifierin the accessibility hierarchy — the accessibility tree forNSMenuItems is structured differently from regular view hierarchies. Any existing XCTest UI-automation that queriespaneTabBarControl.newTerminal,paneTabBarControl.splitRight, etc. by identifier will silently fail to find the elements whencollapseSplitButtonsIntoMenuistrue, 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!