From 04f72f060109c28a942b00a4bd13f70fe857ec6e Mon Sep 17 00:00:00 2001 From: jahorton Date: Mon, 14 Dec 2020 10:06:49 +0700 Subject: [PATCH] change(ios): incorporating PR feedback - first wave --- .../web/tools/sentry-manager/src/index.ts | 8 +-- .../Classes/Errors/SentryManager.swift | 11 ++-- .../Classes/KeymanWebViewController.swift | 6 +-- .../Settings/SettingsViewController.swift | 50 +++++++++---------- 4 files changed, 38 insertions(+), 37 deletions(-) diff --git a/common/core/web/tools/sentry-manager/src/index.ts b/common/core/web/tools/sentry-manager/src/index.ts index 44e05972e5..fd622b0d67 100644 --- a/common/core/web/tools/sentry-manager/src/index.ts +++ b/common/core/web/tools/sentry-manager/src/index.ts @@ -13,7 +13,7 @@ namespace com.keyman { export class KeymanSentryManager { keymanPlatform: string; - silenced: boolean = false; + _enabled: boolean = true; static STANDARD_ALIASABLE_FILES = { 'keymanweb.js': 'keymanweb.js', @@ -128,7 +128,7 @@ namespace com.keyman { console.log("DEBUG: event object for Sentry") console.log(event); return false; //event - } else if(this.silenced) { + } else if(!this._enabled) { console.error(event); return false; } else { @@ -180,11 +180,11 @@ namespace com.keyman { } get enabled(): boolean { - return !this.silenced; + return this._enabled; } set enabled(value: boolean) { - this.silenced = !value; + this._enabled = value; } } } diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Errors/SentryManager.swift b/ios/engine/KMEI/KeymanEngine/Classes/Errors/SentryManager.swift index 98a7aba216..7894057015 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Errors/SentryManager.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Errors/SentryManager.swift @@ -14,7 +14,7 @@ import Sentry * error reporting. */ public class SentryManager { - private static var silenced: Bool = false + private static var _enabled: Bool = true public static var hasStarted: Bool { return Sentry.Client.shared != nil @@ -59,11 +59,14 @@ public class SentryManager { public static var enabled: Bool { get { - return !SentryManager.silenced + return SentryManager._enabled } set(flag) { - SentryManager.silenced = !flag + SentryManager._enabled = flag + + // Ensure that the embedded KeymanWeb engine's crash-reporting state is also updated. + Manager.shared.inputViewController.refreshCrashReporting() } } @@ -74,7 +77,7 @@ public class SentryManager { // Prevents Sentry from buffering the event. return false #else - return !SentryManager.silenced + return SentryManager.enabled #endif } diff --git a/ios/engine/KMEI/KeymanEngine/Classes/KeymanWebViewController.swift b/ios/engine/KMEI/KeymanEngine/Classes/KeymanWebViewController.swift index 071373dfff..46fe032e68 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/KeymanWebViewController.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/KeymanWebViewController.swift @@ -361,9 +361,7 @@ extension KeymanWebViewController { } func refreshCrashReporting() { - let userDefaults = Storage.active.userDefaults - let reportCrashes = userDefaults.bool(forKey: Key.optShouldReportErrors) - + let reportCrashes = SentryManager.enabled webView?.evaluateJavaScript("sentryManager.enabled = \(reportCrashes ? "true" : "false")") } } @@ -572,7 +570,7 @@ extension KeymanWebViewController: KeymanWebDelegate { isLoading = false log.info("Loaded keyboard.") - self.refreshCrashReporting(); + self.refreshCrashReporting() resizeKeyboard() diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Settings/SettingsViewController.swift b/ios/engine/KMEI/KeymanEngine/Classes/Settings/SettingsViewController.swift index 1280375e11..b8338ceebf 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Settings/SettingsViewController.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Settings/SettingsViewController.swift @@ -147,23 +147,6 @@ open class SettingsViewController: UITableViewController { switch(cellIdentifier) { case "languages": cell.accessoryType = .disclosureIndicator - case "enablecrashreporting": - cell.accessoryType = .none - let enableReportingSwitch = UISwitch() - enableReportingSwitch.translatesAutoresizingMaskIntoConstraints = false - - let switchFrame = frameAtRightOfCell(cell: cell.frame, controlSize: enableReportingSwitch.frame.size) - enableReportingSwitch.frame = switchFrame - - enableReportingSwitch.isOn = reportErrors - enableReportingSwitch.addTarget(self, action: #selector(self.reportingSwitchValueChanged), - for: .valueChanged) - cell.addSubview(enableReportingSwitch) - - if #available(iOSApplicationExtension 9.0, *) { - enableReportingSwitch.rightAnchor.constraint(equalTo: cell.layoutMarginsGuide.rightAnchor).isActive = true - enableReportingSwitch.centerYAnchor.constraint(equalTo: cell.layoutMarginsGuide.centerYAnchor).isActive = true - } case "showbanner": cell.accessoryType = .none let showBannerSwitch = UISwitch() @@ -198,6 +181,25 @@ open class SettingsViewController: UITableViewController { showAgainSwitch.rightAnchor.constraint(equalTo: cell.layoutMarginsGuide.rightAnchor).isActive = true showAgainSwitch.centerYAnchor.constraint(equalTo: cell.layoutMarginsGuide.centerYAnchor).isActive = true } + case "enablecrashreporting": + cell.accessoryType = .none + let enableReportingSwitch = UISwitch() + enableReportingSwitch.translatesAutoresizingMaskIntoConstraints = false + + let switchFrame = frameAtRightOfCell(cell: cell.frame, controlSize: enableReportingSwitch.frame.size) + enableReportingSwitch.frame = switchFrame + + enableReportingSwitch.isOn = reportErrors + enableReportingSwitch.addTarget(self, action: #selector(self.reportingSwitchValueChanged), + for: .valueChanged) + cell.addSubview(enableReportingSwitch) + + if #available(iOSApplicationExtension 9.0, *) { + enableReportingSwitch.rightAnchor.constraint(equalTo: cell.layoutMarginsGuide.rightAnchor).isActive = true + enableReportingSwitch.centerYAnchor.constraint(equalTo: cell.layoutMarginsGuide.centerYAnchor).isActive = true + } + case "systemkeyboardsettings": + cell.accessoryType = .disclosureIndicator case "installfile": cell.accessoryType = .disclosureIndicator case "forcederror": @@ -213,14 +215,13 @@ open class SettingsViewController: UITableViewController { @objc func reportingSwitchValueChanged(_ sender: Any) { let userData = Storage.active.userDefaults if let toggle = sender as? UISwitch { + // Save the preference userData.set(toggle.isOn, forKey: Key.optShouldReportErrors) - SentryManager.enabled = toggle.isOn - userData.synchronize() - } - // Forward the thing to KMW; actually USE that value. - Manager.shared.inputViewController.refreshCrashReporting() + // Propagate the effects + SentryManager.enabled = toggle.isOn + } } @objc func bannerSwitchValueChanged(_ sender: Any) { @@ -290,14 +291,13 @@ open class SettingsViewController: UITableViewController { } private func performAction(for indexPath: IndexPath) { - let cellIdentifier = itemsArray[indexPath.row]["reuseid"] - switch indexPath.section { case 0: + let cellIdentifier = itemsArray[indexPath.row]["reuseid"] switch cellIdentifier { case "languages": showLanguages() - case "systemkeyboardsettings": // TODO: If adding an option to direct-hop to "Full Access". + case "systemkeyboardsettings": guard let appSettings = URL(string: UIApplication.openSettingsURLString) else { log.error("Could not launch keyboard settings menu") return