Merge pull request #131 from RobotsAndPencils/andrew/duplicateAlerts

only present a single alert at time
This commit is contained in:
Andrew Erickson 2021-05-02 09:27:58 -06:00 committed by GitHub
commit 85340e6189
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 110 additions and 61 deletions

View file

@ -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 */; };
@ -162,6 +163,7 @@
/* Begin PBXFileReference section */
536CFDD1263C94DE00026CE0 /* SignedInView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SignedInView.swift; sourceTree = "<group>"; };
536CFDD3263C9A8000026CE0 /* XcodesSheet.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XcodesSheet.swift; sourceTree = "<group>"; };
53CBAB2B263DCC9100410495 /* XcodesAlert.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XcodesAlert.swift; sourceTree = "<group>"; };
63EAA4EA259944450046AB8F /* ProgressButton.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProgressButton.swift; sourceTree = "<group>"; };
CA11E7B92598476C00D2EE1C /* XcodeCommands.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = XcodeCommands.swift; sourceTree = "<group>"; };
CA2518EB25A7FF2B00F08414 /* AppStateUpdateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppStateUpdateTests.swift; sourceTree = "<group>"; };
@ -316,6 +318,7 @@
63EAA4EA259944450046AB8F /* ProgressButton.swift */,
CA452BAF259FD9770072DFA4 /* ProgressIndicator.swift */,
536CFDD3263C9A8000026CE0 /* XcodesSheet.swift */,
53CBAB2B263DCC9100410495 /* XcodesAlert.swift */,
);
path = Common;
sourceTree = "<group>";
@ -789,6 +792,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 */,

View file

@ -208,6 +208,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
@ -344,6 +345,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: {}
@ -396,6 +398,7 @@ extension AppState {
)
}
}
self.presentedAlert = .privilegedHelper
}
return helperInstallConsentSubject

View file

@ -4,6 +4,7 @@ import Path
import Version
import SwiftSoup
import struct XCModel.Xcode
import AppleAPI
extension AppState {
@ -43,7 +44,11 @@ extension AppState {
receiveCompletion: { [unowned self] completion in
switch completion {
case let .failure(error):
self.error = error
// 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 update selected Xcode", message: error.legibleLocalizedDescription)
}
case .finished:
Current.defaults.setDate(Current.date(), forKey: "lastUpdated")
}

View file

@ -45,7 +45,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.
@ -133,7 +133,7 @@ class AppState: ObservableObject {
authError = nil
signIn(username: username, password: password)
.sink(
receiveCompletion: { _ in },
receiveCompletion: { _ in },
receiveValue: { _ in }
)
.store(in: &cancellables)
@ -209,13 +209,8 @@ class AppState: ObservableObject {
private func handleAuthenticationFlowCompletion(_ completion: Subscribers.Completion<Error>) {
switch completion {
case let .failure(error):
if case .invalidUsernameOrPassword = error as? AuthenticationError,
let username = savedUsername {
// remove any keychain password if we fail to log with an invalid username or password so it doesn't try again.
try? Current.keychain.remove(username)
Current.defaults.removeObject(forKey: "username")
}
// remove saved username and any stored keychain password if authentication fails so it doesn't try again.
clearLoginCredentials()
Logger.appState.error("Authentication error: \(error.legibleDescription)")
self.authError = error
case .finished:
@ -230,10 +225,7 @@ class AppState: ObservableObject {
}
func signOut() {
if let username = savedUsername {
try? Current.keychain.remove(username)
}
Current.defaults.removeObject(forKey: "username")
clearLoginCredentials()
AppleAPI.Current.network.session.configuration.httpCookieStorage?.removeCookies(since: .distantPast)
authenticationState = .unauthenticated
}
@ -256,6 +248,7 @@ class AppState: ObservableObject {
guard userConsented else { return }
self.installHelperIfNecessary(shouldPrepareUserForHelperInstallation: false)
}
presentedAlert = .privilegedHelper
return
}
@ -264,6 +257,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: {}
@ -342,6 +336,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
@ -385,6 +380,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
},
@ -416,6 +412,7 @@ class AppState: ObservableObject {
guard userConsented else { return }
self.select(id: id, shouldPrepareUserForHelperInstallation: false)
}
presentedAlert = .privilegedHelper
return
}
@ -435,6 +432,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
},
@ -565,6 +563,15 @@ class AppState: ObservableObject {
.eraseToAnyPublisher()
}
/// removes saved username and credentials stored in keychain
private func clearLoginCredentials() {
if let username = savedUsername {
try? Current.keychain.remove(username)
}
Current.defaults.removeObject(forKey: "username")
}
// MARK: - Nested Types
struct AlertContent: Identifiable {

View file

@ -61,7 +61,7 @@ struct CancelInstallButton: View {
private func cancelInstall() {
guard let xcode = xcode else { return }
appState.xcodeBeingConfirmedForInstallCancellation = xcode
appState.presentedAlert = .cancelInstall(xcode: xcode)
}
}

View file

@ -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
}
}
}

View file

@ -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 {

View file

@ -112,7 +112,7 @@ struct XcodeListViewRow: View {
InstallationStepRowView(
installationStep: installationStep,
highlighted: selected,
cancel: { appState.xcodeBeingConfirmedForInstallCancellation = xcode }
cancel: { appState.presentedAlert = .cancelInstall(xcode: xcode) }
)
}
}