Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,10 @@ public struct ArrowlessPopoverAnchor<PopoverContent: View>: NSViewRepresentable
of: anchorView,
preferredEdge: preferredEdge
)
if popover.isShown {
// AppKit can report `isShown == false` while an opening animation is
// still in flight. Register immediately so animated roots participate
// in grouped dismissal just like immediately shown popovers.
if groupMemberID == nil {
groupMemberID = group?.register(popover: popover, anchor: anchorView)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ public final class CmuxPopoverGroup {
containsPointer: ((Int?, CGPoint) -> Bool)? = nil,
close: @escaping () -> Void
) {
if parent == nil, !members.isEmpty {
dismissAll()
}
members.append(Member(
id: id,
parent: parent,
Expand Down
12 changes: 6 additions & 6 deletions Sources/ContentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15425,7 +15425,7 @@ struct SidebarFooterButtons: View {
private var workspacePresentationMode = WorkspacePresentationModeSettings.defaultMode.rawValue
/// Owns the discovery popover so it persists after ⌘ is released.
@State private var isShortcutPopoverPresented = false

@State private var footerPopoverGroup = CmuxPopoverGroup()
private var presentationMode: WorkspacePresentationModeSettings.Mode {
WorkspacePresentationModeSettings.mode(for: workspacePresentationMode)
}
Expand All @@ -15439,13 +15439,13 @@ struct SidebarFooterButtons: View {
if shows(.account) || shows(.mobileConnect) || shows(.help) {
HStack(spacing: 0) {
if shows(.account), CmuxFeatureFlags.shared.isSidebarAccountButtonEnabled {
SidebarAccountMenuButton()
SidebarAccountMenuButton(popoverGroup: footerPopoverGroup)
}
if shows(.mobileConnect), CmuxFeatureFlags.shared.isMobileConnectButtonEnabled {
SidebarMobileConnectButton()
}
if shows(.help) {
SidebarHelpMenuButton(onSendFeedback: onSendFeedback)
SidebarHelpMenuButton(onSendFeedback: onSendFeedback, popoverGroup: footerPopoverGroup)
}
}
}
Expand Down Expand Up @@ -15517,7 +15517,7 @@ private struct SidebarHelpMenuButton: View {
@State private var keyboardShortcutSettingsObserver = KeyboardShortcutSettingsObserver.shared

let onSendFeedback: () -> Void

let popoverGroup: CmuxPopoverGroup
@State private var isPopoverPresented = false

private var iconSize: CGFloat {
Expand Down Expand Up @@ -15549,7 +15549,7 @@ private struct SidebarHelpMenuButton: View {

var body: some View {
Button {
isPopoverPresented.toggle()
let shouldPresent = !isPopoverPresented; popoverGroup.dismissAll(); isPopoverPresented = shouldPresent
} label: {
SidebarFooterHelpIcon(pointSize: iconSize, weight: iconWeight)
.frame(width: buttonSize, height: buttonSize, alignment: .center)
Expand All @@ -15559,7 +15559,7 @@ private struct SidebarHelpMenuButton: View {
.background(ArrowlessPopoverAnchor(
isPresented: $isPopoverPresented,
preferredEdge: .maxY,
detachedGap: 4
detachedGap: 4, presentationAnimation: .enabled, group: popoverGroup
) {
helpPopover
})
Expand Down
6 changes: 3 additions & 3 deletions Sources/SidebarAccountTeamPopover.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ struct SidebarAccountMenuButton: View {
private let buttonSize = SidebarFooterButtonMetrics.buttonSize
@State private var isPopoverPresented = false
@State private var isShowingTeamPicker = false
@State private var popoverGroup = CmuxPopoverGroup()
let popoverGroup: CmuxPopoverGroup
#if DEBUG
@AppStorage(SidebarFooterProfileIconDebugSettings.sizeKey)
private var debugIconSize = SidebarFooterProfileIconDebugSettings.defaultSize
Expand Down Expand Up @@ -71,7 +71,7 @@ struct SidebarAccountMenuButton: View {
)
Button {
if isSignedIn {
isPopoverPresented.toggle()
let shouldPresent = !isPopoverPresented; popoverGroup.dismissAll(); isPopoverPresented = shouldPresent
} else {
_ = AppDelegate.shared?.performAccountSignInWorkspaceAction(
tabManager: tabManager,
Expand Down Expand Up @@ -111,7 +111,7 @@ struct SidebarAccountMenuButton: View {
.task {
for await _ in NotificationCenter.default.notifications(named: .cmuxTeamPickerShortcutRequested) {
guard !Task.isCancelled else { return }
isPopoverPresented = true
popoverGroup.dismissAll(); isPopoverPresented = true
}
}
.onChange(of: isPopoverPresented) { _, presented in
Expand Down
113 changes: 113 additions & 0 deletions cmuxTests/CmuxPopoverGroupTests.swift
Original file line number Diff line number Diff line change
@@ -1,10 +1,49 @@
import AppKit
import SwiftUI
import Testing
@testable import CmuxAppKitSupportUI

@MainActor
@Suite
struct CmuxPopoverGroupTests {
@Test func animatedRootRegistersBeforeItsOpeningTransitionFinishes() {
let group = CmuxPopoverGroup()
let window = NSWindow(
contentRect: CGRect(x: 0, y: 0, width: 360, height: 240),
styleMask: [.borderless],
backing: .buffered,
defer: false
)
let accountAnchor = NSView(frame: CGRect(x: 24, y: 160, width: 40, height: 24))
let helpAnchor = NSView(frame: CGRect(x: 84, y: 160, width: 40, height: 24))
window.contentView?.addSubview(accountAnchor)
window.contentView?.addSubview(helpAnchor)
window.orderFrontRegardless()
defer { window.close() }

var accountPresented = true
let accountCoordinator = ArrowlessPopoverAnchor<EmptyView>.Coordinator(
isPresented: Binding(get: { accountPresented }, set: { accountPresented = $0 }),
presentationAnimation: .automatic,
group: group
)
accountCoordinator.anchorView = accountAnchor
accountCoordinator.updateRootView(AnyView(EmptyView()))
accountCoordinator.present(preferredEdge: .maxY, detachedGap: 4)

let helpCoordinator = ArrowlessPopoverAnchor<EmptyView>.Coordinator(
isPresented: .constant(true),
presentationAnimation: .enabled,
group: group
)
helpCoordinator.anchorView = helpAnchor
helpCoordinator.updateRootView(AnyView(EmptyView()))
helpCoordinator.present(preferredEdge: .maxY, detachedGap: 4)

#expect(!accountPresented)
group.dismissAll()
}

@Test func groupedPickerCanOptIntoNativeOpeningAnimation() {
#expect(
CmuxPopoverPresentationAnimation.enabled.animates(
Expand Down Expand Up @@ -175,4 +214,78 @@ struct CmuxPopoverGroupTests {
group.handleClick(windowNumber: nil, point: .zero)
#expect(closed == [first, second])
}

@Test func openingHelpClosesTheTeamPickerBeforeTheAccountMenu() {
let group = CmuxPopoverGroup()
let account = UUID()
let picker = UUID()
let help = UUID()
var closed: [UUID] = []
group.register(id: account, parent: nil, contains: { _, _ in true }, close: {
closed.append(account)
group.unregister(account)
})
group.register(id: picker, parent: account, contains: { _, _ in true }, close: {
closed.append(picker)
group.unregister(picker)
})
#expect(closed.isEmpty)

group.register(id: help, parent: nil, contains: { _, point in point.x < 220 }, close: {
closed.append(help)
group.unregister(help)
})
#expect(closed == [picker, account])

group.handleClick(windowNumber: nil, point: CGPoint(x: 40, y: 80))
#expect(closed == [picker, account])
group.handleClick(windowNumber: nil, point: CGPoint(x: 600, y: 80))
#expect(closed == [picker, account, help])
}

@Test(arguments: [false, true])
func switchingBetweenFooterMenusLeavesOnlyTheLatestRoot(startWithAccount: Bool) {
let group = CmuxPopoverGroup()
let names = startWithAccount ? ["account", "help", "account"] : ["help", "account", "help"]
var closed: [String] = []
var previousID: UUID?

for (index, name) in names.enumerated() {
let id = UUID()
group.register(id: id, parent: nil, contains: { _, _ in true }, close: {
closed.append(name)
group.unregister(id)
})
// Cleanup from the old root cannot retire the newly registered menu.
if let previousID { group.unregister(previousID) }
#expect(closed == Array(names.prefix(index)))
previousID = id
}

group.dismissAll()
#expect(closed == names)
group.dismissAll()
#expect(closed == names)
}

@Test func replacingAFooterRootDoesNotDismissAnotherWindowsGroup() {
let firstWindow = CmuxPopoverGroup()
let secondWindow = CmuxPopoverGroup()
var firstClosed = 0
var secondClosed = 0
firstWindow.register(id: UUID(), parent: nil, contains: { _, _ in true }, close: {
firstClosed += 1
})
secondWindow.register(id: UUID(), parent: nil, contains: { _, _ in true }, close: {
secondClosed += 1
})

firstWindow.register(id: UUID(), parent: nil, contains: { _, _ in true }, close: {})
#expect(firstClosed == 1)
#expect(secondClosed == 0)
firstWindow.dismissAll()
#expect(secondClosed == 0)
secondWindow.dismissAll()
#expect(secondClosed == 1)
}
}
Loading