From b8981121987616b05cb4b0b165f09899ab8e1a10 Mon Sep 17 00:00:00 2001 From: Anh Do <18567+quanganhdo@users.noreply.github.com> Date: Mon, 7 Oct 2024 10:15:10 -0400 Subject: [PATCH] Design improvements to automatic update flow (#3344) Task/Issue URL: https://app.asana.com/0/1201037661562251/1208183525704010/f Tech Design URL: CC: Figma: https://www.figma.com/design/vDBpSFDzW0eN7nwpS95gju/macOS-update-process?node-id=2359-52971&t=KnvIdm2f7PRjNwfK-4 **Description**: **Steps to test this PR**: 1. Change the appcast URL to an invalid URL to test the error state 2. Play with different update options to see if update works **Definition of Done**: * [ ] Does this PR satisfy our [Definition of Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)? --- ###### Internal references: [Pull Request Review Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f) [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) [Pull Request Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f) --- DuckDuckGo/Common/Localizables/UserText.swift | 6 +- DuckDuckGo/Localizable.xcstrings | 78 ++++++++++++------- .../Preferences/Model/AboutPreferences.swift | 2 + .../View/PreferencesAboutView.swift | 38 ++++----- DuckDuckGo/Updates/UpdateController.swift | 8 +- DuckDuckGo/Updates/UpdateUserDriver.swift | 18 ++++- 6 files changed, 96 insertions(+), 54 deletions(-) diff --git a/DuckDuckGo/Common/Localizables/UserText.swift b/DuckDuckGo/Common/Localizables/UserText.swift index dc19db6bba..03707dda3a 100644 --- a/DuckDuckGo/Common/Localizables/UserText.swift +++ b/DuckDuckGo/Common/Localizables/UserText.swift @@ -1216,10 +1216,14 @@ struct UserText { static let automaticUpdates = NSLocalizedString("settings.automatic.updates", value: "Automatically install updates (recommended)", comment: "Title of the checkbox item to set up automatic updates of the browser") static let manualUpdates = NSLocalizedString("settings.manual.updates", value: "Check for updates but let you choose to install them", comment: "Title of the checkbox item to set up manual updates of the browser") static let checkingForUpdate = NSLocalizedString("settings.checking.for.update", value: "Checking for update", comment: "Label informing users the app is currently checking for new update") + static let downloadingUpdate = NSLocalizedString("settings.downloading.update", value: "Downloading update %@", comment: "Label informing users the app is currently downloading the update. This will contain a percentage") + static let preparingUpdate = NSLocalizedString("settings.preparing.update", value: "Preparing update", comment: "Label informing users the app is preparing to update.") + static let updateFailed = NSLocalizedString("settings.update.failed", value: "Update failed", comment: "Label informing users the app is unable to update.") static let upToDate = NSLocalizedString("settings.up.to.date", value: "DuckDuckGo is up to date", comment: "Label informing users the app is currently up to date and no update is required.") static let newerVersionAvailable = NSLocalizedString("settings.newer.version.available", value: "Newer version available", comment: "Label informing users the newer version of the app is available to install.") static let lastChecked = NSLocalizedString("settings.last.checked", value: "Last checked", comment: "Label informing users what is the last time the app checked for the update.") - static let runUpdate = NSLocalizedString("settings.restart.to.update", value: "Update DuckDuckGo", comment: "Button label trigering restart and update of the application.") + static let runUpdate = NSLocalizedString("settings.restart.to.update", value: "Update DuckDuckGo", comment: "Button label triggering restart and update of the application.") + static let retryUpdate = NSLocalizedString("settings.retry.update", value: "Retry Update", comment: "Button label triggering a retry of the update.") static let browserUpdatedNotification = NSLocalizedString("notification.browser.updated", value: "Browser Updated", comment: "Notification informing user the app has been updated") static let browserDowngradedNotification = NSLocalizedString("notification.browser.downgraded", value: "Browser Downgraded", comment: "Notification informing user the app has been downgraded") static let criticalUpdateNotification = NSLocalizedString("notification.critical.update", value: "Critical update required. Restart to update.", comment: "Notification informing user a critical update is required.") diff --git a/DuckDuckGo/Localizable.xcstrings b/DuckDuckGo/Localizable.xcstrings index 55ac53bbbf..f9fc0a1f3c 100644 --- a/DuckDuckGo/Localizable.xcstrings +++ b/DuckDuckGo/Localizable.xcstrings @@ -52,31 +52,6 @@ } } } - }, - " — Downloading %@ / %@" : { - "localizations" : { - "en" : { - "stringUnit" : { - "state" : "new", - "value" : " — Downloading %1$@ / %2$@" - } - } - } - }, - " — Downloading update" : { - - }, - " — Extracting update" : { - - }, - " — Extracting update (%@%%)" : { - - }, - " — Installing" : { - - }, - " — Ready to install" : { - }, " %@" : { "localizations" : { @@ -12620,9 +12595,6 @@ } } } - }, - "Checking for update" : { - }, "clear" : { "comment" : "Clear button", @@ -55268,6 +55240,18 @@ } } }, + "settings.downloading.update" : { + "comment" : "Label informing users the app is currently downloading the update. This will contain a percentage", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Downloading update %@" + } + } + } + }, "settings.last.checked" : { "comment" : "Label informing users what is the last time the app checked for the update.", "extractionState" : "extracted_with_value", @@ -55448,8 +55432,20 @@ } } }, + "settings.preparing.update" : { + "comment" : "Label informing users the app is preparing to update.", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Preparing update" + } + } + } + }, "settings.restart.to.update" : { - "comment" : "Button label trigering restart and update of the application.", + "comment" : "Button label triggering restart and update of the application.", "extractionState" : "extracted_with_value", "localizations" : { "de" : { @@ -55508,6 +55504,18 @@ } } }, + "settings.retry.update" : { + "comment" : "Button label triggering a retry of the update.", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Retry Update" + } + } + } + }, "settings.up.to.date" : { "comment" : "Label informing users the app is currently up to date and no update is required.", "extractionState" : "extracted_with_value", @@ -55568,6 +55576,18 @@ } } }, + "settings.update.failed" : { + "comment" : "Label informing users the app is unable to update.", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Update failed" + } + } + } + }, "share.menu.item" : { "comment" : "Menu item title", "extractionState" : "extracted_with_value", diff --git a/DuckDuckGo/Preferences/Model/AboutPreferences.swift b/DuckDuckGo/Preferences/Model/AboutPreferences.swift index 1ec96bc6d0..dafbb1e361 100644 --- a/DuckDuckGo/Preferences/Model/AboutPreferences.swift +++ b/DuckDuckGo/Preferences/Model/AboutPreferences.swift @@ -33,6 +33,8 @@ final class AboutPreferences: ObservableObject, PreferencesTabOpening { init(from update: Update?, progress: UpdateCycleProgress) { if let update, !update.isInstalled { self = .updateCycle(progress) + } else if progress.isFailed { + self = .updateCycle(progress) } else { self = .upToDate } diff --git a/DuckDuckGo/Preferences/View/PreferencesAboutView.swift b/DuckDuckGo/Preferences/View/PreferencesAboutView.swift index fc30e700a0..ee753f679a 100644 --- a/DuckDuckGo/Preferences/View/PreferencesAboutView.swift +++ b/DuckDuckGo/Preferences/View/PreferencesAboutView.swift @@ -175,10 +175,10 @@ extension Preferences { } #if SPARKLE - private var formatter: ByteCountFormatter { - let formatter = ByteCountFormatter() - formatter.allowsNonnumericFormatting = false - formatter.allowedUnits = [.useKB, .useMB, .useGB] + private var formatter: NumberFormatter { + let formatter = NumberFormatter() + formatter.numberStyle = .percent + formatter.maximumFractionDigits = 0 return formatter } @@ -188,17 +188,14 @@ extension Preferences { case .updateCycleDidStart: Text(" — " + UserText.checkingForUpdate) case .downloadDidStart: - Text(" — Downloading update") - case .downloading(let bytesDownloaded, let bytesToDownload): - Text(" — Downloading \(formatter.string(fromByteCount: Int64(bytesDownloaded))) / \(formatter.string(fromByteCount: Int64(bytesToDownload)))") - case .extractionDidStart: - Text(" — Extracting update") - case .extracting(let percentage): - Text(" — Extracting update (\(String(format: "%.1f", percentage * 100))%)") - case .readyToInstallAndRelaunch: - Text(" — Ready to install") - case .installationDidStart, .installing: - Text(" — Installing") + Text(" — " + String(format: UserText.downloadingUpdate, "")) + case .downloading(let percentage): + Text(" — " + String(format: UserText.downloadingUpdate, + formatter.string(from: NSNumber(value: percentage)) ?? "")) + case .extractionDidStart, .extracting, .readyToInstallAndRelaunch, .installationDidStart, .installing: + Text(" — " + UserText.preparingUpdate) + case .updaterError: + Text(" — " + UserText.updateFailed) case .updateCycleNotStarted, .updateCycleDone: EmptyView() } @@ -210,8 +207,8 @@ extension Preferences { case .upToDate: Image(systemName: "checkmark.circle.fill") .foregroundColor(.green) - case .updateCycle: - if hasPendingUpdate { + case .updateCycle(let progress): + if hasPendingUpdate || progress.isFailed { Image(systemName: "exclamationmark.circle.fill") .foregroundColor(.red) } else { @@ -249,12 +246,17 @@ extension Preferences { model.checkForUpdate() } .buttonStyle(UpdateButtonStyle(enabled: true)) - case .updateCycle: + case .updateCycle(let progress): if hasPendingUpdate { Button(UserText.runUpdate) { model.runUpdate() } .buttonStyle(UpdateButtonStyle(enabled: true)) + } else if progress.isFailed { + Button(UserText.retryUpdate) { + model.checkForUpdate() + } + .buttonStyle(UpdateButtonStyle(enabled: true)) } else { Button(UserText.checkForUpdate) { model.checkForUpdate() diff --git a/DuckDuckGo/Updates/UpdateController.swift b/DuckDuckGo/Updates/UpdateController.swift index 4efa2f5fa6..69bf87dc92 100644 --- a/DuckDuckGo/Updates/UpdateController.swift +++ b/DuckDuckGo/Updates/UpdateController.swift @@ -259,8 +259,12 @@ extension UpdateController: SPUUpdaterDelegate { } func updater(_ updater: SPUUpdater, didFinishUpdateCycleFor updateCheck: SPUUpdateCheck, error: (any Error)?) { - Logger.updates.debug("Updater did finish update cycle") - updateProgress = .updateCycleDone + if error == nil { + Logger.updates.debug("Updater did finish update cycle") + updateProgress = .updateCycleDone + } else { + Logger.updates.debug("Updater did finish update cycle with error") + } } } diff --git a/DuckDuckGo/Updates/UpdateUserDriver.swift b/DuckDuckGo/Updates/UpdateUserDriver.swift index 7d43108b4c..a6c9ae034c 100644 --- a/DuckDuckGo/Updates/UpdateUserDriver.swift +++ b/DuckDuckGo/Updates/UpdateUserDriver.swift @@ -30,12 +30,13 @@ enum UpdateCycleProgress { case updateCycleDidStart case updateCycleDone case downloadDidStart - case downloading(UInt64, UInt64) + case downloading(Double) case extractionDidStart case extracting(Double) case readyToInstallAndRelaunch case installationDidStart case installing + case updaterError(Error) static var `default` = UpdateCycleProgress.updateCycleNotStarted @@ -48,7 +49,14 @@ enum UpdateCycleProgress { var isIdle: Bool { switch self { - case .updateCycleDone, .updateCycleNotStarted: return true + case .updateCycleDone, .updateCycleNotStarted, .updaterError: return true + default: return false + } + } + + var isFailed: Bool { + switch self { + case .updaterError: return true default: return false } } @@ -128,7 +136,8 @@ final class UpdateUserDriver: NSObject, SPUUserDriver { } func showUpdaterError(_ error: any Error, acknowledgement: @escaping () -> Void) { - // no-op + updateProgress = .updaterError(error) + acknowledgement() } func showDownloadInitiated(cancellation: @escaping () -> Void) { @@ -145,7 +154,7 @@ final class UpdateUserDriver: NSObject, SPUUserDriver { if bytesDownloaded > bytesToDownload { bytesToDownload = bytesDownloaded } - updateProgress = .downloading(bytesDownloaded, bytesToDownload) + updateProgress = .downloading(Double(bytesDownloaded) / Double(bytesToDownload)) } func showDownloadDidStartExtractingUpdate() { @@ -182,6 +191,7 @@ final class UpdateUserDriver: NSObject, SPUUserDriver { } func dismissUpdateInstallation() { + guard !updateProgress.isFailed else { return } updateProgress = .updateCycleDone } }