From 8f1a014afd9c6724b767b2fbf65d27d26b3c01e5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 12 Oct 2025 15:20:26 -0700 Subject: [PATCH] macos: clean up the "installing" update state (#9170) This includes multiple changes to clean up the "installing" state: - Ghostty will not confirm quit, since the user has already confirmed they want to restart to install the update. - If termination fails for any reason, the popover has a button to retry restarting. - The copy and badge symbol have been updated to better match the reality of the "installing" state. CleanShot 2025-10-12 at 15 04 08@2x AI written: https://ampcode.com/threads/T-623d1030-419f-413f-a285-e79c86a4246b fully understood. --- macos/Sources/App/macOS/AppDelegate.swift | 6 ++++ .../Sources/Features/Update/UpdateBadge.swift | 2 +- .../Features/Update/UpdateController.swift | 5 +++ .../Features/Update/UpdateDriver.swift | 2 +- .../Features/Update/UpdatePopoverView.swift | 33 +++++++++++++------ .../Features/Update/UpdateSimulator.swift | 18 ++++++++-- .../Features/Update/UpdateViewModel.swift | 10 ++++-- macos/Tests/Update/UpdateStateTests.swift | 4 +-- macos/Tests/Update/UpdateViewModelTests.swift | 4 +-- 9 files changed, 62 insertions(+), 22 deletions(-) diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index 3319189b9..9a6eab47b 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -322,6 +322,12 @@ class AppDelegate: NSObject, func applicationShouldTerminate(_ sender: NSApplication) -> NSApplication.TerminateReply { let windows = NSApplication.shared.windows if (windows.isEmpty) { return .terminateNow } + + // If we've already accepted to install an update, then we don't need to + // confirm quit. The user is already expecting the update to happen. + if updateController.isInstalling { + return .terminateNow + } // This probably isn't fully safe. The isEmpty check above is aspirational, it doesn't // quite work with SwiftUI because windows are retained on close. So instead we check diff --git a/macos/Sources/Features/Update/UpdateBadge.swift b/macos/Sources/Features/Update/UpdateBadge.swift index a4a95f411..054fdf971 100644 --- a/macos/Sources/Features/Update/UpdateBadge.swift +++ b/macos/Sources/Features/Update/UpdateBadge.swift @@ -32,7 +32,7 @@ struct UpdateBadge: View { case .extracting(let extracting): ProgressRingView(progress: min(1, max(0, extracting.progress))) - case .checking, .installing: + case .checking: if let iconName = model.iconName { Image(systemName: iconName) .rotationEffect(.degrees(rotationAngle)) diff --git a/macos/Sources/Features/Update/UpdateController.swift b/macos/Sources/Features/Update/UpdateController.swift index aa875567c..2dfb0a420 100644 --- a/macos/Sources/Features/Update/UpdateController.swift +++ b/macos/Sources/Features/Update/UpdateController.swift @@ -17,6 +17,11 @@ class UpdateController { userDriver.viewModel } + /// True if we're installing an update. + var isInstalling: Bool { + installCancellable != nil + } + /// Initialize a new update controller. init() { let hostBundle = Bundle.main diff --git a/macos/Sources/Features/Update/UpdateDriver.swift b/macos/Sources/Features/Update/UpdateDriver.swift index ed58f1663..4bddda809 100644 --- a/macos/Sources/Features/Update/UpdateDriver.swift +++ b/macos/Sources/Features/Update/UpdateDriver.swift @@ -172,7 +172,7 @@ class UpdateDriver: NSObject, SPUUserDriver { } func showInstallingUpdate(withApplicationTerminated applicationTerminated: Bool, retryTerminatingApplication: @escaping () -> Void) { - viewModel.state = .installing + viewModel.state = .installing(.init(retryTerminatingApplication: retryTerminatingApplication)) if !hasUnobtrusiveTarget { standard.showInstallingUpdate(withApplicationTerminated: applicationTerminated, retryTerminatingApplication: retryTerminatingApplication) diff --git a/macos/Sources/Features/Update/UpdatePopoverView.swift b/macos/Sources/Features/Update/UpdatePopoverView.swift index 236649c21..770b9aedd 100644 --- a/macos/Sources/Features/Update/UpdatePopoverView.swift +++ b/macos/Sources/Features/Update/UpdatePopoverView.swift @@ -38,8 +38,8 @@ struct UpdatePopoverView: View { case .readyToInstall(let ready): ReadyToInstallView(ready: ready, dismiss: dismiss) - case .installing: - InstallingView() + case .installing(let installing): + InstallingView(installing: installing, dismiss: dismiss) case .notFound(let notFound): NotFoundView(notFound: notFound, dismiss: dismiss) @@ -313,18 +313,31 @@ fileprivate struct ReadyToInstallView: View { } fileprivate struct InstallingView: View { + let installing: UpdateState.Installing + let dismiss: DismissAction + var body: some View { - VStack(alignment: .leading, spacing: 8) { - HStack(spacing: 10) { - ProgressView() - .controlSize(.small) - Text("Installing…") + VStack(alignment: .leading, spacing: 16) { + VStack(alignment: .leading, spacing: 8) { + Text("Restart Required") .font(.system(size: 13, weight: .semibold)) + + Text("The update is ready. Please restart the application to complete the installation.") + .font(.system(size: 11)) + .foregroundColor(.secondary) + .fixedSize(horizontal: false, vertical: true) } - Text("The application will relaunch shortly.") - .font(.system(size: 11)) - .foregroundColor(.secondary) + HStack { + Spacer() + Button("Restart Now") { + installing.retryTerminatingApplication() + dismiss() + } + .keyboardShortcut(.defaultAction) + .buttonStyle(.borderedProminent) + .controlSize(.small) + } } .padding(16) } diff --git a/macos/Sources/Features/Update/UpdateSimulator.swift b/macos/Sources/Features/Update/UpdateSimulator.swift index f40bbee1b..c855282c0 100644 --- a/macos/Sources/Features/Update/UpdateSimulator.swift +++ b/macos/Sources/Features/Update/UpdateSimulator.swift @@ -28,6 +28,9 @@ enum UpdateSimulator { /// User cancels while checking: checking (1s) → cancels → idle case cancelDuringChecking + /// Shows the installing state with restart button: installing (stays until dismissed) + case installing + func simulate(with viewModel: UpdateViewModel) { switch self { case .happyPath: @@ -44,6 +47,8 @@ enum UpdateSimulator { simulateCancelDuringDownload(viewModel) case .cancelDuringChecking: simulateCancelDuringChecking(viewModel) + case .installing: + simulateInstalling(viewModel) } } @@ -260,10 +265,10 @@ enum UpdateSimulator { viewModel.state = .readyToInstall(.init( reply: { choice in if choice == .install { - viewModel.state = .installing - DispatchQueue.main.asyncAfter(deadline: .now() + 2.0) { + viewModel.state = .installing(.init(retryTerminatingApplication: { + print("Restart button clicked in simulator - resetting to idle") viewModel.state = .idle - } + })) } else { viewModel.state = .idle } @@ -274,4 +279,11 @@ enum UpdateSimulator { } } } + + private func simulateInstalling(_ viewModel: UpdateViewModel) { + viewModel.state = .installing(.init(retryTerminatingApplication: { + print("Restart button clicked in simulator - resetting to idle") + viewModel.state = .idle + })) + } } diff --git a/macos/Sources/Features/Update/UpdateViewModel.swift b/macos/Sources/Features/Update/UpdateViewModel.swift index ccb03e731..f0b39ed2d 100644 --- a/macos/Sources/Features/Update/UpdateViewModel.swift +++ b/macos/Sources/Features/Update/UpdateViewModel.swift @@ -33,7 +33,7 @@ class UpdateViewModel: ObservableObject { case .readyToInstall: return "Install Update" case .installing: - return "Installing…" + return "Restart to Complete Update" case .notFound: return "No Updates Available" case .error(let err): @@ -72,7 +72,7 @@ class UpdateViewModel: ObservableObject { case .readyToInstall: return "checkmark.circle.fill" case .installing: - return "gear" + return "power.circle" case .notFound: return "info.circle" case .error: @@ -192,7 +192,7 @@ enum UpdateState: Equatable { case downloading(Downloading) case extracting(Extracting) case readyToInstall(ReadyToInstall) - case installing + case installing(Installing) var isIdle: Bool { if case .idle = self { return true } @@ -382,4 +382,8 @@ enum UpdateState: Equatable { struct ReadyToInstall { let reply: @Sendable (SPUUserUpdateChoice) -> Void } + + struct Installing { + let retryTerminatingApplication: () -> Void + } } diff --git a/macos/Tests/Update/UpdateStateTests.swift b/macos/Tests/Update/UpdateStateTests.swift index 01819bb25..269cd3153 100644 --- a/macos/Tests/Update/UpdateStateTests.swift +++ b/macos/Tests/Update/UpdateStateTests.swift @@ -25,8 +25,8 @@ struct UpdateStateTests { } @Test func testInstallingEquality() { - let state1: UpdateState = .installing - let state2: UpdateState = .installing + let state1: UpdateState = .installing(.init(retryTerminatingApplication: {})) + let state2: UpdateState = .installing(.init(retryTerminatingApplication: {})) #expect(state1 == state2) } diff --git a/macos/Tests/Update/UpdateViewModelTests.swift b/macos/Tests/Update/UpdateViewModelTests.swift index d3b2e060b..56748ff83 100644 --- a/macos/Tests/Update/UpdateViewModelTests.swift +++ b/macos/Tests/Update/UpdateViewModelTests.swift @@ -58,8 +58,8 @@ struct UpdateViewModelTests { @Test func testInstallingText() { let viewModel = UpdateViewModel() - viewModel.state = .installing - #expect(viewModel.text == "Installing…") + viewModel.state = .installing(.init(retryTerminatingApplication: {})) + #expect(viewModel.text == "Restart to Complete Update") } @Test func testNotFoundText() {