Fix sidebar update pill cached popover flow (#2142)

* test: cover cached update pill first-click flow

* fix: use cached sidebar update popover
This commit is contained in:
Austin Wang 2026-03-25 04:21:03 -07:00 committed by GitHub
parent 049d296267
commit 99ca3c9b9a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 286 additions and 92 deletions

View file

@ -1,5 +1,4 @@
import AppKit
import Bonsplit
import Foundation
import SwiftUI
@ -13,12 +12,11 @@ struct UpdatePill: View {
var body: some View {
if model.showsPill {
pillButton
.popover(
isPresented: $showPopover,
attachmentAnchor: .rect(.bounds),
arrowEdge: .top
) {
UpdatePopoverView(model: model)
.background(UpdatePillPopoverAnchor(isPresented: $showPopover, model: model))
.onChange(of: model.showsPill) { _, showsPill in
if !showsPill {
showPopover = false
}
}
.transition(.opacity.combined(with: .scale(scale: 0.95)))
}
@ -26,23 +24,7 @@ struct UpdatePill: View {
@ViewBuilder
private var pillButton: some View {
Button(action: {
if model.showsDetectedBackgroundUpdate {
if showPopover {
showPopover = false
} else {
showPopover = true
AppDelegate.shared?.checkForUpdatesInCustomUI()
}
return
}
if case .notFound(let notFound) = model.state {
model.state = .idle
notFound.acknowledgement()
} else {
showPopover.toggle()
}
}) {
Button(action: handleTap) {
HStack(spacing: 6) {
UpdateBadge(model: model)
.frame(width: 14, height: 14)
@ -68,6 +50,27 @@ struct UpdatePill: View {
.accessibilityIdentifier("UpdatePill")
}
private func handleTap() {
if model.showsDetectedBackgroundUpdate {
if model.hasCachedDetectedUpdateDetails {
showPopover.toggle()
} else if showPopover {
showPopover = false
} else {
showPopover = true
AppDelegate.shared?.checkForUpdatesInCustomUI()
}
return
}
if case .notFound(let notFound) = model.state {
model.state = .idle
notFound.acknowledgement()
} else {
showPopover.toggle()
}
}
private var textWidth: CGFloat? {
let attributes: [NSAttributedString.Key: Any] = [.font: textFont]
let size = (model.maxWidthText as NSString).size(withAttributes: attributes)
@ -75,6 +78,113 @@ struct UpdatePill: View {
}
}
private struct UpdatePillPopoverAnchor: NSViewRepresentable {
@Binding var isPresented: Bool
@ObservedObject var model: UpdateViewModel
func makeNSView(context: Context) -> NSView {
let view = NSView()
context.coordinator.anchorView = view
return view
}
func updateNSView(_ nsView: NSView, context: Context) {
let coordinator = context.coordinator
context.coordinator.anchorView = nsView
context.coordinator.updateRootView(
AnyView(
UpdatePopoverView(model: model) {
[weak coordinator] in
coordinator?.closeFromContent()
}
)
)
if isPresented {
context.coordinator.present()
} else {
context.coordinator.dismiss()
}
}
func makeCoordinator() -> Coordinator {
Coordinator(isPresented: $isPresented)
}
static func dismantleNSView(_ nsView: NSView, coordinator: Coordinator) {
coordinator.dismiss()
}
final class Coordinator: NSObject, NSPopoverDelegate {
@Binding var isPresented: Bool
weak var anchorView: NSView?
private let hostingController = NSHostingController(rootView: AnyView(EmptyView()))
private var popover: NSPopover?
init(isPresented: Binding<Bool>) {
_isPresented = isPresented
}
func updateRootView(_ rootView: AnyView) {
hostingController.rootView = rootView
hostingController.view.invalidateIntrinsicContentSize()
hostingController.view.layoutSubtreeIfNeeded()
updateContentSize()
}
func present() {
guard let anchorView, anchorView.window != nil else {
isPresented = false
dismiss()
return
}
anchorView.superview?.layoutSubtreeIfNeeded()
let popover = popover ?? makePopover()
updateContentSize()
guard !popover.isShown else { return }
popover.show(relativeTo: anchorView.bounds, of: anchorView, preferredEdge: .maxY)
}
func dismiss() {
popover?.performClose(nil)
}
func closeFromContent() {
isPresented = false
dismiss()
}
func popoverDidClose(_ notification: Notification) {
popover = nil
if isPresented {
isPresented = false
}
}
private func makePopover() -> NSPopover {
let popover = NSPopover()
popover.behavior = .semitransient
popover.animates = true
popover.contentViewController = hostingController
popover.delegate = self
self.popover = popover
return popover
}
private func updateContentSize() {
let fittingSize = hostingController.view.fittingSize
guard fittingSize.width > 0, fittingSize.height > 0 else { return }
popover?.contentSize = NSSize(
width: ceil(fittingSize.width),
height: ceil(fittingSize.height)
)
}
}
}
/// Menu item that shows "Install Update and Relaunch" when an update is ready.
struct InstallUpdateMenuItem: View {
@ObservedObject var model: UpdateViewModel

View file

@ -1,19 +1,27 @@
import AppKit
import SwiftUI
import Sparkle
import SwiftUI
/// Popover view that displays detailed update information and actions.
struct UpdatePopoverView: View {
@ObservedObject var model: UpdateViewModel
@Environment(\.dismiss) private var dismiss
let dismiss: () -> Void
init(model: UpdateViewModel, dismiss: @escaping () -> Void = {}) {
self.model = model
self.dismiss = dismiss
}
var body: some View {
VStack(alignment: .leading, spacing: 0) {
switch model.effectiveState {
case .idle:
if let detectedVersion = model.detectedUpdateVersion,
if model.showsDetectedBackgroundUpdate,
let detectedItem = model.detectedUpdateItem {
DetectedBackgroundUpdateView(item: detectedItem, dismiss: dismiss)
} else if let detectedVersion = model.detectedUpdateVersion,
model.showsDetectedBackgroundUpdate {
DetectedBackgroundUpdateView(version: detectedVersion)
DetectedBackgroundUpdatePendingView(version: detectedVersion)
} else {
EmptyView()
}
@ -47,7 +55,113 @@ struct UpdatePopoverView: View {
}
}
fileprivate struct UpdateMetadataView: View {
let item: SUAppcastItem
let labelWidth: CGFloat
var body: some View {
VStack(alignment: .leading, spacing: 4) {
HStack(spacing: 6) {
Text(String(localized: "update.popover.version", defaultValue: "Version:"))
.foregroundColor(.secondary)
.frame(width: labelWidth, alignment: .trailing)
Text(item.displayVersionString)
}
.font(.system(size: 11))
if item.contentLength > 0 {
HStack(spacing: 6) {
Text(String(localized: "update.popover.size", defaultValue: "Size:"))
.foregroundColor(.secondary)
.frame(width: labelWidth, alignment: .trailing)
Text(ByteCountFormatter.string(fromByteCount: Int64(item.contentLength), countStyle: .file))
}
.font(.system(size: 11))
}
if let date = item.date {
HStack(spacing: 6) {
Text(String(localized: "update.popover.released", defaultValue: "Released:"))
.foregroundColor(.secondary)
.frame(width: labelWidth, alignment: .trailing)
Text(date.formatted(date: .abbreviated, time: .omitted))
}
.font(.system(size: 11))
}
}
.textSelection(.enabled)
}
}
fileprivate struct UpdateReleaseNotesLink: View {
let notes: UpdateState.ReleaseNotes
var body: some View {
Link(destination: notes.url) {
HStack {
Image(systemName: "doc.text")
.font(.system(size: 11))
Text(notes.label)
.font(.system(size: 11, weight: .medium))
Spacer()
Image(systemName: "arrow.up.right")
.font(.system(size: 10))
}
.foregroundColor(.primary)
.padding(12)
.frame(maxWidth: .infinity)
.background(Color(nsColor: .controlBackgroundColor))
.contentShape(Rectangle())
}
.buttonStyle(.plain)
}
}
fileprivate struct DetectedBackgroundUpdateView: View {
let item: SUAppcastItem
let dismiss: () -> Void
private let labelWidth: CGFloat = 60
var body: some View {
VStack(alignment: .leading, spacing: 0) {
VStack(alignment: .leading, spacing: 12) {
VStack(alignment: .leading, spacing: 8) {
Text(String(localized: "update.popover.updateAvailable", defaultValue: "Update Available"))
.font(.system(size: 13, weight: .semibold))
UpdateMetadataView(item: item, labelWidth: labelWidth)
}
HStack(spacing: 8) {
Button(String(localized: "common.later", defaultValue: "Later")) {
dismiss()
}
.controlSize(.small)
.keyboardShortcut(.cancelAction)
Spacer()
Button(String(localized: "common.installAndRelaunch", defaultValue: "Install and Relaunch")) {
AppDelegate.shared?.attemptUpdate(nil)
dismiss()
}
.keyboardShortcut(.defaultAction)
.buttonStyle(.borderedProminent)
.controlSize(.small)
}
}
.padding(16)
if let notes = UpdateState.ReleaseNotes(displayVersionString: item.displayVersionString) {
Divider()
UpdateReleaseNotesLink(notes: notes)
}
}
}
}
fileprivate struct DetectedBackgroundUpdatePendingView: View {
let version: String
var body: some View {
@ -78,7 +192,7 @@ fileprivate struct DetectedBackgroundUpdateView: View {
fileprivate struct PermissionRequestView: View {
let request: UpdateState.PermissionRequest
let dismiss: DismissAction
let dismiss: () -> Void
var body: some View {
VStack(alignment: .leading, spacing: 16) {
@ -96,7 +210,8 @@ fileprivate struct PermissionRequestView: View {
Button(String(localized: "common.notNow", defaultValue: "Not Now")) {
request.reply(SUUpdatePermissionResponse(
automaticUpdateChecks: false,
sendSystemProfile: false))
sendSystemProfile: false
))
dismiss()
}
.keyboardShortcut(.cancelAction)
@ -106,7 +221,8 @@ fileprivate struct PermissionRequestView: View {
Button(String(localized: "common.allow", defaultValue: "Allow")) {
request.reply(SUUpdatePermissionResponse(
automaticUpdateChecks: true,
sendSystemProfile: false))
sendSystemProfile: false
))
dismiss()
}
.keyboardShortcut(.defaultAction)
@ -119,7 +235,7 @@ fileprivate struct PermissionRequestView: View {
fileprivate struct CheckingView: View {
let checking: UpdateState.Checking
let dismiss: DismissAction
let dismiss: () -> Void
var body: some View {
VStack(alignment: .leading, spacing: 16) {
@ -146,7 +262,7 @@ fileprivate struct CheckingView: View {
fileprivate struct UpdateAvailableView: View {
let update: UpdateState.UpdateAvailable
let dismiss: DismissAction
let dismiss: () -> Void
private let labelWidth: CGFloat = 60
@ -157,36 +273,7 @@ fileprivate struct UpdateAvailableView: View {
Text(String(localized: "update.popover.updateAvailable", defaultValue: "Update Available"))
.font(.system(size: 13, weight: .semibold))
VStack(alignment: .leading, spacing: 4) {
HStack(spacing: 6) {
Text(String(localized: "update.popover.version", defaultValue: "Version:"))
.foregroundColor(.secondary)
.frame(width: labelWidth, alignment: .trailing)
Text(update.appcastItem.displayVersionString)
}
.font(.system(size: 11))
if update.appcastItem.contentLength > 0 {
HStack(spacing: 6) {
Text(String(localized: "update.popover.size", defaultValue: "Size:"))
.foregroundColor(.secondary)
.frame(width: labelWidth, alignment: .trailing)
Text(ByteCountFormatter.string(fromByteCount: Int64(update.appcastItem.contentLength), countStyle: .file))
}
.font(.system(size: 11))
}
if let date = update.appcastItem.date {
HStack(spacing: 6) {
Text(String(localized: "update.popover.released", defaultValue: "Released:"))
.foregroundColor(.secondary)
.frame(width: labelWidth, alignment: .trailing)
Text(date.formatted(date: .abbreviated, time: .omitted))
}
.font(.system(size: 11))
}
}
.textSelection(.enabled)
UpdateMetadataView(item: update.appcastItem, labelWidth: labelWidth)
}
HStack(spacing: 8) {
@ -218,24 +305,7 @@ fileprivate struct UpdateAvailableView: View {
if let notes = update.releaseNotes {
Divider()
Link(destination: notes.url) {
HStack {
Image(systemName: "doc.text")
.font(.system(size: 11))
Text(notes.label)
.font(.system(size: 11, weight: .medium))
Spacer()
Image(systemName: "arrow.up.right")
.font(.system(size: 10))
}
.foregroundColor(.primary)
.padding(12)
.frame(maxWidth: .infinity)
.background(Color(nsColor: .controlBackgroundColor))
.contentShape(Rectangle())
}
.buttonStyle(.plain)
UpdateReleaseNotesLink(notes: notes)
}
}
}
@ -243,7 +313,7 @@ fileprivate struct UpdateAvailableView: View {
fileprivate struct DownloadingView: View {
let download: UpdateState.Downloading
let dismiss: DismissAction
let dismiss: () -> Void
var body: some View {
VStack(alignment: .leading, spacing: 16) {
@ -300,7 +370,7 @@ fileprivate struct ExtractingView: View {
fileprivate struct InstallingView: View {
let installing: UpdateState.Installing
let dismiss: DismissAction
let dismiss: () -> Void
var body: some View {
VStack(alignment: .leading, spacing: 16) {
@ -339,7 +409,7 @@ fileprivate struct InstallingView: View {
fileprivate struct NotFoundView: View {
let notFound: UpdateState.NotFound
let dismiss: DismissAction
let dismiss: () -> Void
var body: some View {
VStack(alignment: .leading, spacing: 16) {
@ -369,7 +439,7 @@ fileprivate struct NotFoundView: View {
fileprivate struct UpdateErrorView: View {
let error: UpdateState.Error
let dismiss: DismissAction
let dismiss: () -> Void
var body: some View {
let title = UpdateViewModel.userFacingErrorTitle(for: error.error)

View file

@ -10,9 +10,13 @@ enum UpdateTestSupport {
if let detectedVersion = env["CMUX_UI_TEST_DETECTED_UPDATE_VERSION"],
!detectedVersion.isEmpty {
DispatchQueue.main.async {
if let item = makeAppcastItem(displayVersion: detectedVersion) {
viewModel.recordDetectedUpdate(item)
} else {
viewModel.detectedUpdateVersion = UpdateViewModel.normalizedDetectedUpdateVersion(from: detectedVersion)
}
}
}
guard let state = env["CMUX_UI_TEST_UPDATE_STATE"] else { return }
@ -87,6 +91,7 @@ enum UpdateTestSupport {
]
let dict: [String: Any] = [
"title": "cmux \(displayVersion)",
"pubDate": "Wed, 25 Mar 2026 12:00:00 +0000",
"enclosure": enclosure,
]
return SUAppcastItem(dictionary: dict)

View file

@ -7,6 +7,7 @@ class UpdateViewModel: ObservableObject {
@Published var state: UpdateState = .idle
@Published var overrideState: UpdateState?
@Published var detectedUpdateVersion: String?
@Published private(set) var detectedUpdateItem: SUAppcastItem?
#if DEBUG
@Published var debugOverrideText: String?
#endif
@ -19,15 +20,22 @@ class UpdateViewModel: ObservableObject {
effectiveState.isIdle && detectedUpdateVersion != nil
}
var hasCachedDetectedUpdateDetails: Bool {
detectedUpdateItem != nil
}
var showsPill: Bool {
!effectiveState.isIdle || showsDetectedBackgroundUpdate
}
func recordDetectedUpdate(_ item: SUAppcastItem) {
detectedUpdateVersion = Self.normalizedDetectedUpdateVersion(from: item.displayVersionString)
let version = Self.normalizedDetectedUpdateVersion(from: item.displayVersionString)
detectedUpdateItem = version == nil ? nil : item
detectedUpdateVersion = version
}
func clearDetectedUpdate() {
detectedUpdateItem = nil
detectedUpdateVersion = nil
}

View file

@ -67,9 +67,7 @@ final class UpdatePillUITests: XCTestCase {
app.launchEnvironment["CMUX_UI_TEST_MODE"] = "1"
app.launchEnvironment["CMUX_UI_TEST_DETECTED_UPDATE_VERSION"] = "9.9.9"
app.launchEnvironment["CMUX_UI_TEST_FEED_URL"] = "https://cmux.test/appcast.xml"
app.launchEnvironment["CMUX_UI_TEST_FEED_MODE"] = "available"
app.launchEnvironment["CMUX_UI_TEST_UPDATE_VERSION"] = "9.9.9"
app.launchEnvironment["CMUX_UI_TEST_AUTO_ALLOW_PERMISSION"] = "1"
app.launchEnvironment["CMUX_UI_TEST_FEED_MODE"] = "none"
launchAndActivate(app)
let pill = pillButton(app: app, expectedLabel: "Update Available: 9.9.9")
@ -82,7 +80,10 @@ final class UpdatePillUITests: XCTestCase {
app.staticTexts["Update Available"].waitForExistence(timeout: 8.0),
"Expected the first click on a background-detected update pill to open the popover"
)
XCTAssertTrue(app.buttons["Install and Relaunch"].waitForExistence(timeout: 2.0))
XCTAssertTrue(
app.buttons["Install and Relaunch"].waitForExistence(timeout: 2.0),
"Expected cached update info to show the install action without running a new update check"
)
}
func testUpdatePillShowsForNoUpdateThenDismisses() {