From 9395b1fda222be9cbd1eab6609c98d924e2b3743 Mon Sep 17 00:00:00 2001 From: Andrew Erickson Date: Sat, 1 May 2021 12:48:55 -0600 Subject: [PATCH] only present a single alert at time --- Xcodes.xcodeproj/project.pbxproj | 4 + Xcodes/Backend/AppState+Install.swift | 3 + Xcodes/Backend/AppState+Update.swift | 1 + Xcodes/Backend/AppState.swift | 8 +- Xcodes/Backend/XcodeCommands.swift | 2 +- Xcodes/Frontend/Common/XcodesAlert.swift | 15 +++ Xcodes/Frontend/MainWindow.swift | 105 ++++++++++-------- .../Frontend/XcodeList/XcodeListViewRow.swift | 2 +- 8 files changed, 92 insertions(+), 48 deletions(-) create mode 100644 Xcodes/Frontend/Common/XcodesAlert.swift diff --git a/Xcodes.xcodeproj/project.pbxproj b/Xcodes.xcodeproj/project.pbxproj index ca0cfce..aa1effa 100644 --- a/Xcodes.xcodeproj/project.pbxproj +++ b/Xcodes.xcodeproj/project.pbxproj @@ -9,6 +9,7 @@ /* Begin PBXBuildFile section */ 536CFDD2263C94DE00026CE0 /* SignedInView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 536CFDD1263C94DE00026CE0 /* SignedInView.swift */; }; 536CFDD4263C9A8000026CE0 /* XcodesSheet.swift in Sources */ = {isa = PBXBuildFile; fileRef = 536CFDD3263C9A8000026CE0 /* XcodesSheet.swift */; }; + 53CBAB2C263DCC9100410495 /* XcodesAlert.swift in Sources */ = {isa = PBXBuildFile; fileRef = 53CBAB2B263DCC9100410495 /* XcodesAlert.swift */; }; 63EAA4EB259944450046AB8F /* ProgressButton.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63EAA4EA259944450046AB8F /* ProgressButton.swift */; }; CA11E7BA2598476C00D2EE1C /* XcodeCommands.swift in Sources */ = {isa = PBXBuildFile; fileRef = CA11E7B92598476C00D2EE1C /* XcodeCommands.swift */; }; CA2518EC25A7FF2B00F08414 /* AppStateUpdateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CA2518EB25A7FF2B00F08414 /* AppStateUpdateTests.swift */; }; @@ -160,6 +161,7 @@ /* Begin PBXFileReference section */ 536CFDD1263C94DE00026CE0 /* SignedInView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SignedInView.swift; sourceTree = ""; }; 536CFDD3263C9A8000026CE0 /* XcodesSheet.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XcodesSheet.swift; sourceTree = ""; }; + 53CBAB2B263DCC9100410495 /* XcodesAlert.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XcodesAlert.swift; sourceTree = ""; }; 63EAA4EA259944450046AB8F /* ProgressButton.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProgressButton.swift; sourceTree = ""; }; CA11E7B92598476C00D2EE1C /* XcodeCommands.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XcodeCommands.swift; sourceTree = ""; }; CA2518EB25A7FF2B00F08414 /* AppStateUpdateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppStateUpdateTests.swift; sourceTree = ""; }; @@ -312,6 +314,7 @@ 63EAA4EA259944450046AB8F /* ProgressButton.swift */, CA452BAF259FD9770072DFA4 /* ProgressIndicator.swift */, 536CFDD3263C9A8000026CE0 /* XcodesSheet.swift */, + 53CBAB2B263DCC9100410495 /* XcodesAlert.swift */, ); path = Common; sourceTree = ""; @@ -783,6 +786,7 @@ CABFA9C52592EEEA00380FEE /* FileManager+.swift in Sources */, CABFA9CD2592EEEA00380FEE /* Foundation.swift in Sources */, CA9FF8872595607900E47BAF /* InstalledXcode.swift in Sources */, + 53CBAB2C263DCC9100410495 /* XcodesAlert.swift in Sources */, CA42DD6E25AEA8B200BC0B0C /* Logger.swift in Sources */, CA61A6E0259835580008926E /* Xcode.swift in Sources */, CAE4247F259A666100B8B246 /* MainWindow.swift in Sources */, diff --git a/Xcodes/Backend/AppState+Install.swift b/Xcodes/Backend/AppState+Install.swift index d3642c1..c3bb450 100644 --- a/Xcodes/Backend/AppState+Install.swift +++ b/Xcodes/Backend/AppState+Install.swift @@ -207,6 +207,7 @@ extension AppState { .handleEvents(receiveCompletion: { [unowned self] completion in if case let .failure(error) = completion { self.error = error + self.presentedAlert = .generic(title: "Unable to install archived Xcode", message: error.legibleLocalizedDescription) } }) .catch { _ in @@ -343,6 +344,7 @@ extension AppState { receiveCompletion: { completion in if case let .failure(error) = completion { self.error = error + self.presentedAlert = .generic(title: "Unable to perform post install steps", message: error.legibleLocalizedDescription) } }, receiveValue: {} @@ -395,6 +397,7 @@ extension AppState { ) } } + self.presentedAlert = .privilegedHelper } return helperInstallConsentSubject diff --git a/Xcodes/Backend/AppState+Update.swift b/Xcodes/Backend/AppState+Update.swift index db8bc0a..9d73e7a 100644 --- a/Xcodes/Backend/AppState+Update.swift +++ b/Xcodes/Backend/AppState+Update.swift @@ -44,6 +44,7 @@ extension AppState { switch completion { case let .failure(error): self.error = error + self.presentedAlert = .generic(title: "Unable to update selected Xcode", message: error.legibleLocalizedDescription) case .finished: Current.defaults.setDate(Current.date(), forKey: "lastUpdated") } diff --git a/Xcodes/Backend/AppState.swift b/Xcodes/Backend/AppState.swift index 6833c2e..d572dd7 100644 --- a/Xcodes/Backend/AppState.swift +++ b/Xcodes/Backend/AppState.swift @@ -42,7 +42,7 @@ class AppState: ObservableObject { @Published var isProcessingAuthRequest = false @Published var secondFactorData: SecondFactorData? @Published var xcodeBeingConfirmedForUninstallation: Xcode? - @Published var xcodeBeingConfirmedForInstallCancellation: Xcode? + @Published var presentedAlert: XcodesAlert? @Published var helperInstallState: HelperInstallState = .notInstalled /// Whether the user is being prepared for the helper installation alert with an explanation. /// This closure will be performed after the user chooses whether or not to proceed. @@ -253,6 +253,7 @@ class AppState: ObservableObject { guard userConsented else { return } self.installHelperIfNecessary(shouldPrepareUserForHelperInstallation: false) } + presentedAlert = .privilegedHelper return } @@ -261,6 +262,7 @@ class AppState: ObservableObject { receiveCompletion: { [unowned self] completion in if case let .failure(error) = completion { self.error = error + self.presentedAlert = .generic(title: "Unable to install helper", message: error.legibleLocalizedDescription) } }, receiveValue: {} @@ -338,6 +340,7 @@ class AppState: ObservableObject { // Prevent setting the app state error if it is an invalid session, we will present the sign in view instead if error as? AuthenticationError != .invalidSession { self.error = error + self.presentedAlert = .generic(title: "Unable to install Xcode", message: error.legibleLocalizedDescription) } if let index = self.allXcodes.firstIndex(where: { $0.id == id }) { self.allXcodes[index].installState = .notInstalled @@ -381,6 +384,7 @@ class AppState: ObservableObject { receiveCompletion: { [unowned self] completion in if case let .failure(error) = completion { self.error = error + self.presentedAlert = .generic(title: "Unable to uninstall Xcode", message: error.legibleLocalizedDescription) } self.uninstallPublisher = nil }, @@ -412,6 +416,7 @@ class AppState: ObservableObject { guard userConsented else { return } self.select(id: id, shouldPrepareUserForHelperInstallation: false) } + presentedAlert = .privilegedHelper return } @@ -431,6 +436,7 @@ class AppState: ObservableObject { receiveCompletion: { [unowned self] completion in if case let .failure(error) = completion { self.error = error + self.presentedAlert = .generic(title: "Unable to select Xcode", message: error.legibleLocalizedDescription) } self.selectPublisher = nil }, diff --git a/Xcodes/Backend/XcodeCommands.swift b/Xcodes/Backend/XcodeCommands.swift index 7f32377..58ee191 100644 --- a/Xcodes/Backend/XcodeCommands.swift +++ b/Xcodes/Backend/XcodeCommands.swift @@ -61,7 +61,7 @@ struct CancelInstallButton: View { private func cancelInstall() { guard let xcode = xcode else { return } - appState.xcodeBeingConfirmedForInstallCancellation = xcode + appState.presentedAlert = .cancelInstall(xcode: xcode) } } diff --git a/Xcodes/Frontend/Common/XcodesAlert.swift b/Xcodes/Frontend/Common/XcodesAlert.swift new file mode 100644 index 0000000..c57d089 --- /dev/null +++ b/Xcodes/Frontend/Common/XcodesAlert.swift @@ -0,0 +1,15 @@ +import Foundation + +enum XcodesAlert: Identifiable { + case cancelInstall(xcode: Xcode) + case privilegedHelper + case generic(title: String, message: String) + + var id: Int { + switch self { + case .cancelInstall: return 1 + case .privilegedHelper: return 2 + case .generic: return 3 + } + } +} diff --git a/Xcodes/Frontend/MainWindow.swift b/Xcodes/Frontend/MainWindow.swift index 954b221..fe87402 100644 --- a/Xcodes/Frontend/MainWindow.swift +++ b/Xcodes/Frontend/MainWindow.swift @@ -48,51 +48,9 @@ struct MainWindow: View { .environmentObject(appState) } } - // This overlay is only here to work around the one-alert-per-view limitation - .overlay( - Color.clear - // This particular alert could be added specifically to InstallationStepView and CancelInstallButton _except_ for when CancelInstallButton is used in the Xcode CommandMenu, so it's here for now. - .alert(item: $appState.xcodeBeingConfirmedForInstallCancellation) { xcode in - Alert(title: Text("Are you sure you want to stop the installation of Xcode \(xcode.description)?"), - message: Text("Any progress will be discarded."), - primaryButton: .destructive(Text("Stop Installation"), action: { self.appState.cancelInstall(id: xcode.id) }), - secondaryButton: .cancel(Text("Cancel"))) - } - ) - // This overlay is only here to work around the one-alert-per-view limitation - .overlay( - Color.clear - .alert(isPresented: $appState.isPreparingUserForActionRequiringHelper.isNotNil) { - Alert( - title: Text("Privileged Helper"), - message: Text("Xcodes uses a separate privileged helper to perform tasks as root. These are things that would require sudo on the command line, including post-install steps and switching Xcode versions with xcode-select.\n\nYou'll be prompted for your macOS account password to install it."), - primaryButton: .default(Text("Install"), action: { - // The isPreparingUserForActionRequiringHelper closure is set to nil by the alert's binding when its dismissed. - // We need to capture it to be invoked after that happens. - let helperAction = appState.isPreparingUserForActionRequiringHelper - DispatchQueue.main.async { - // This really shouldn't be nil, but sometimes this alert is being shown twice and I don't know why. - // There are some DispatchQueue.main.async's scattered around which make this better but in some situations it's still happening. - // When that happens, the second time the user clicks an alert button isPreparingUserForActionRequiringHelper will be nil. - // To at least not crash, we're using ? - helperAction?(true) - } - }), - secondaryButton: .cancel { - // The isPreparingUserForActionRequiringHelper closure is set to nil by the alert's binding when its dismissed. - // We need to capture it to be invoked after that happens. - let helperAction = appState.isPreparingUserForActionRequiringHelper - DispatchQueue.main.async { - // This really shouldn't be nil, but sometimes this alert is being shown twice and I don't know why. - // There are some DispatchQueue.main.async's scattered around which make this better but in some situations it's still happening. - // When that happens, the second time the user clicks an alert button isPreparingUserForActionRequiringHelper will be nil. - // To at least not crash, we're using ? - helperAction?(false) - } - } - ) - } - ) + .alert(item: $appState.presentedAlert, content: { presentedAlert in + alert(for: presentedAlert) + }) // I'm expecting to be able to use this modifier on a List row, but using it at the top level here is the only way that has made XcodeCommands work so far. // FB8954571 focusedValue(_:_:) on List row doesn't propagate value to @FocusedValue .focusedValue(\.selectedXcode, SelectedXcode(appState.allXcodes.first { $0.id == selectedXcodeID })) @@ -136,6 +94,63 @@ struct MainWindow: View { .frame(width: 400) } } + + private func alert(for alertType: XcodesAlert) -> Alert { + switch alertType { + case let .cancelInstall(xcode): + return Alert( + title: Text("Are you sure you want to stop the installation of Xcode \(xcode.description)?"), + message: Text("Any progress will be discarded."), + primaryButton: .destructive( + Text("Stop Installation"), + action: { + self.appState.cancelInstall(id: xcode.id) + } + ), + secondaryButton: .cancel(Text("Cancel")) + ) + case .privilegedHelper: + return Alert( + title: Text("Privileged Helper"), + message: Text("Xcodes uses a separate privileged helper to perform tasks as root. These are things that would require sudo on the command line, including post-install steps and switching Xcode versions with xcode-select.\n\nYou'll be prompted for your macOS account password to install it."), + primaryButton: .default(Text("Install"), action: { + // The isPreparingUserForActionRequiringHelper closure is set to nil by the alert's binding when its dismissed. + // We need to capture it to be invoked after that happens. + let helperAction = appState.isPreparingUserForActionRequiringHelper + DispatchQueue.main.async { + // This really shouldn't be nil, but sometimes this alert is being shown twice and I don't know why. + // There are some DispatchQueue.main.async's scattered around which make this better but in some situations it's still happening. + // When that happens, the second time the user clicks an alert button isPreparingUserForActionRequiringHelper will be nil. + // To at least not crash, we're using ? + helperAction?(true) + appState.presentedAlert = nil + } + }), + secondaryButton: .cancel { + // The isPreparingUserForActionRequiringHelper closure is set to nil by the alert's binding when its dismissed. + // We need to capture it to be invoked after that happens. + let helperAction = appState.isPreparingUserForActionRequiringHelper + DispatchQueue.main.async { + // This really shouldn't be nil, but sometimes this alert is being shown twice and I don't know why. + // There are some DispatchQueue.main.async's scattered around which make this better but in some situations it's still happening. + // When that happens, the second time the user clicks an alert button isPreparingUserForActionRequiringHelper will be nil. + // To at least not crash, we're using ? + helperAction?(false) + appState.presentedAlert = nil + } + } + ) + case let .generic(title, message): + return Alert( + title: Text(title), + message: Text(message), + dismissButton: .default( + Text("Ok"), + action: { appState.presentedAlert = nil } + ) + ) + } + } } struct MainWindow_Previews: PreviewProvider { diff --git a/Xcodes/Frontend/XcodeList/XcodeListViewRow.swift b/Xcodes/Frontend/XcodeList/XcodeListViewRow.swift index 9b0c645..1b7e477 100644 --- a/Xcodes/Frontend/XcodeList/XcodeListViewRow.swift +++ b/Xcodes/Frontend/XcodeList/XcodeListViewRow.swift @@ -112,7 +112,7 @@ struct XcodeListViewRow: View { InstallationStepRowView( installationStep: installationStep, highlighted: selected, - cancel: { appState.xcodeBeingConfirmedForInstallCancellation = xcode } + cancel: { appState.presentedAlert = .cancelInstall(xcode: xcode) } ) } }