From 2c07dbf8ff5ba3a537a06613462f243ee04a6d6f Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Tue, 1 Nov 2022 17:39:45 +0800 Subject: [PATCH] Better handling of errors when dispatching annotation events to JS --- .../App/Views/LinkItemDetailView.swift | 4 +- .../App/Views/WebReader/WebReader.swift | 11 +- .../Views/WebReader/WebReaderContainer.swift | 4 +- .../Sources/Utils/ShowInSnackbar.swift | 18 ++ .../Views/Article/OmnivoreWebView.swift | 164 +++++++++++++----- .../Views/FontSizeAdjustmentPopoverView.swift | 4 +- .../Sources/Views/Web/BasicWebAppView.swift | 3 +- 7 files changed, 152 insertions(+), 56 deletions(-) create mode 100644 apple/OmnivoreKit/Sources/Utils/ShowInSnackbar.swift diff --git a/apple/OmnivoreKit/Sources/App/Views/LinkItemDetailView.swift b/apple/OmnivoreKit/Sources/App/Views/LinkItemDetailView.swift index 04f6a5e5c..a55bbd7d6 100644 --- a/apple/OmnivoreKit/Sources/App/Views/LinkItemDetailView.swift +++ b/apple/OmnivoreKit/Sources/App/Views/LinkItemDetailView.swift @@ -25,12 +25,12 @@ import Views func handleArchiveAction(dataService: DataService) { guard let objectID = item?.objectID ?? pdfItem?.objectID else { return } dataService.archiveLink(objectID: objectID, archived: !isItemArchived) - Snackbar.show(message: !isItemArchived ? "Link archived" : "Link moved to Inbox") + showInSnackbar(!isItemArchived ? "Link archived" : "Link moved to Inbox") } func handleDeleteAction(dataService: DataService) { guard let objectID = item?.objectID ?? pdfItem?.objectID else { return } - Snackbar.show(message: "Link removed") + showInSnackbar("Link removed") dataService.removeLink(objectID: objectID) } diff --git a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReader.swift b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReader.swift index 708b03e37..634398536 100644 --- a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReader.swift +++ b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReader.swift @@ -17,6 +17,7 @@ struct WebReader: PlatformViewRepresentable { @Binding var shareActionID: UUID? @Binding var annotation: String @Binding var showBottomBar: Bool + @Binding var showHighlightAnnotationModal: Bool func makeCoordinator() -> WebReaderCoordinator { WebReaderCoordinator() @@ -84,7 +85,15 @@ struct WebReader: PlatformViewRepresentable { private func updatePlatformView(_ webView: WKWebView, context: Context) { if annotationSaveTransactionID != context.coordinator.lastSavedAnnotationID { context.coordinator.lastSavedAnnotationID = annotationSaveTransactionID - (webView as? OmnivoreWebView)?.dispatchEvent(.saveAnnotation(annotation: annotation)) + do { + try (webView as? OmnivoreWebView)?.dispatchEvent(.saveAnnotation(annotation: annotation)) + DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(100)) { + showHighlightAnnotationModal = false + } + } catch { + showInSnackbar("Error saving note.") + showHighlightAnnotationModal = true + } } if readerSettingsChangedTransactionID != context.coordinator.previousReaderSettingsChangedUUID { diff --git a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift index 4ff5f2f62..a2fdc890d 100644 --- a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift +++ b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContainer.swift @@ -300,7 +300,8 @@ struct WebReaderContainerView: View { showNavBarActionID: $showNavBarActionID, shareActionID: $shareActionID, annotation: $annotation, - showBottomBar: $showBottomBar + showBottomBar: $showBottomBar, + showHighlightAnnotationModal: $showHighlightAnnotationModal ) .background(Theme.fromName(themeName: ThemeManager.currentThemeName)?.bgColor ?? .clear) .onTapGesture { @@ -319,7 +320,6 @@ struct WebReaderContainerView: View { annotation: $annotation, onSave: { annotationSaveTransactionID = UUID() - showHighlightAnnotationModal = false }, onCancel: { showHighlightAnnotationModal = false diff --git a/apple/OmnivoreKit/Sources/Utils/ShowInSnackbar.swift b/apple/OmnivoreKit/Sources/Utils/ShowInSnackbar.swift new file mode 100644 index 000000000..9fd39779a --- /dev/null +++ b/apple/OmnivoreKit/Sources/Utils/ShowInSnackbar.swift @@ -0,0 +1,18 @@ +// +// ShowInSnackbar.swift +// +// +// Created by Jackson Harper on 11/1/22. +// + +import Foundation + +public func showInSnackbar(_ message: String) { + let nname = Notification.Name("OperationSuccess") + NotificationCenter.default.post(name: nname, object: nil, userInfo: ["message": message]) +} + +public func showErrorInSnackbar(_ message: String) { + let nname = Notification.Name("OperationFailure") + NotificationCenter.default.post(name: nname, object: nil, userInfo: ["message": message]) +} diff --git a/apple/OmnivoreKit/Sources/Views/Article/OmnivoreWebView.swift b/apple/OmnivoreKit/Sources/Views/Article/OmnivoreWebView.swift index 2b97441d4..e886f6814 100644 --- a/apple/OmnivoreKit/Sources/Views/Article/OmnivoreWebView.swift +++ b/apple/OmnivoreKit/Sources/Views/Article/OmnivoreWebView.swift @@ -1,6 +1,7 @@ import Utils import WebKit + /// Describes actions that can be sent from the WebView back to native views. /// The names on the javascript side must match for an action to be handled. public enum WebViewAction: String, CaseIterable { @@ -31,12 +32,6 @@ public final class OmnivoreWebView: WKWebView { self.isFindInteractionEnabled = true } #endif - - NotificationCenter.default.addObserver(forName: NSNotification.Name("SpeakingReaderItem"), object: nil, queue: OperationQueue.main, using: { notification in - if let pageID = notification.userInfo?["pageID"] as? String, let anchorIdx = notification.userInfo?["anchorIdx"] as? String { - self.dispatchEvent(.speakingSection(anchorIdx: anchorIdx)) - } - }) } @available(*, unavailable) @@ -45,20 +40,32 @@ public final class OmnivoreWebView: WKWebView { } public func updateTheme() { - if let themeName = UserDefaults.standard.value(forKey: UserDefaultKey.themeName.rawValue) as? String { - dispatchEvent(.updateTheme(themeName: "Gray" /* themeName */ )) + do { + if let themeName = UserDefaults.standard.value(forKey: UserDefaultKey.themeName.rawValue) as? String { + try dispatchEvent(.updateTheme(themeName: "Gray" /* themeName */ )) + } + } catch { + showErrorInSnackbar("Error updating theme") } } public func updateFontFamily() { - if let fontFamily = UserDefaults.standard.value(forKey: UserDefaultKey.preferredWebFont.rawValue) as? String { - dispatchEvent(.updateFontFamily(family: fontFamily)) + do { + if let fontFamily = UserDefaults.standard.value(forKey: UserDefaultKey.preferredWebFont.rawValue) as? String { + try dispatchEvent(.updateFontFamily(family: fontFamily)) + } + } catch { + showErrorInSnackbar("Error updating font") } } public func updateFontSize() { - if let fontSize = UserDefaults.standard.value(forKey: UserDefaultKey.preferredWebFontSize.rawValue) as? Int { - dispatchEvent(.updateFontSize(size: fontSize)) + do { + if let fontSize = UserDefaults.standard.value(forKey: UserDefaultKey.preferredWebFontSize.rawValue) as? Int { + try dispatchEvent(.updateFontSize(size: fontSize)) + } + } catch { + showErrorInSnackbar("Error updating font") } } @@ -66,13 +73,21 @@ public final class OmnivoreWebView: WKWebView { if let maxWidthPercentage = UserDefaults.standard.value( forKey: UserDefaultKey.preferredWebMaxWidthPercentage.rawValue ) as? Int { - dispatchEvent(.updateMaxWidthPercentage(maxWidthPercentage: maxWidthPercentage)) + do { + try dispatchEvent(.updateMaxWidthPercentage(maxWidthPercentage: maxWidthPercentage)) + } catch { + showErrorInSnackbar("Error updating max width") + } } } public func updateLineHeight() { if let height = UserDefaults.standard.value(forKey: UserDefaultKey.preferredWebLineSpacing.rawValue) as? Int { - dispatchEvent(.updateLineHeight(height: height)) + do { + try dispatchEvent(.updateLineHeight(height: height)) + } catch { + showErrorInSnackbar("Error updating line height") + } } } @@ -82,17 +97,35 @@ public final class OmnivoreWebView: WKWebView { ) as? Bool if let isHighContrast = isHighContrast { - dispatchEvent(.handleFontContrastChange(isHighContrast: isHighContrast)) + do { + try dispatchEvent(.handleFontContrastChange(isHighContrast: isHighContrast)) + } catch { + showErrorInSnackbar("Error updating text contrast") + } } } public func shareOriginalItem() { - dispatchEvent(.share) + do { + try dispatchEvent(.share) + } catch { + showErrorInSnackbar("Error updating line height") + } } - public func dispatchEvent(_ event: WebViewDispatchEvent) { - evaluateJavaScript(event.script) { _, err in - if let err = err { print("evaluateJavaScript error", err) } + public func dispatchEvent(_ event: WebViewDispatchEvent) throws { + let script = try event.script + var errResult: Error? + + evaluateJavaScript(script) { _, err in + if let err = err { + print("evaluateJavaScript error", err) + errResult = err + } + } + + if let errResult = errResult { + throw errResult } } @@ -100,7 +133,11 @@ public final class OmnivoreWebView: WKWebView { override public func traitCollectionDidChange(_ previousTraitCollection: UITraitCollection?) { super.traitCollectionDidChange(previousTraitCollection) guard previousTraitCollection?.userInterfaceStyle != traitCollection.userInterfaceStyle else { return } - dispatchEvent(.updateColorMode(isDark: traitCollection.userInterfaceStyle == .dark)) + do { + try dispatchEvent(.updateColorMode(isDark: traitCollection.userInterfaceStyle == .dark)) + } catch { + showErrorInSnackbar("Error updating theme") + } } #elseif os(macOS) @@ -210,28 +247,48 @@ public final class OmnivoreWebView: WKWebView { } @objc private func annotateSelection() { - dispatchEvent(.annotate) + do { + try dispatchEvent(.annotate) + } catch { + showErrorInSnackbar("Error creating highlight") + } hideMenu() } @objc private func highlightSelection() { - dispatchEvent(.highlight) + do { + try dispatchEvent(.highlight) + } catch { + showErrorInSnackbar("Error creating highlight") + } hideMenu() } @objc private func shareSelection() { - dispatchEvent(.share) + do { + try dispatchEvent(.share) + } catch { + showErrorInSnackbar("Error sharing highlight") + } hideMenu() } @objc private func removeSelection() { - dispatchEvent(.remove) + do { + try dispatchEvent(.remove) + } catch { + showErrorInSnackbar("Error deleting highlight") + } hideMenu() } @objc override public func copy(_ sender: Any?) { super.copy(sender) - dispatchEvent(.copyHighlight) + do { + try dispatchEvent(.copyHighlight) + } catch { + showErrorInSnackbar("Error copying highlight") + } hideMenu() } @@ -265,7 +322,7 @@ public final class OmnivoreWebView: WKWebView { private func hideMenuAndDismissHighlight() { hideMenu() - dispatchEvent(.dismissHighlight) + try? dispatchEvent(.dismissHighlight) } private func showHighlightMenu(_ rect: CGRect) { @@ -310,7 +367,10 @@ public enum WebViewDispatchEvent { case speakingSection(anchorIdx: String) var script: String { - "var event = new Event('\(eventName)');\(scriptPropertyLine)document.dispatchEvent(event);" + get throws { + let propertyLine = try scriptPropertyLine + return "var event = new Event('\(eventName)');\(propertyLine)document.dispatchEvent(event);" + } } private var eventName: String { @@ -349,27 +409,35 @@ public enum WebViewDispatchEvent { } private var scriptPropertyLine: String { - switch self { - case let .handleFontContrastChange(isHighContrast: isHighContrast): - return "event.fontContrast = '\(isHighContrast ? "high" : "normal")';" - case let .updateLineHeight(height: height): - return "event.lineHeight = '\(height)';" - case let .updateMaxWidthPercentage(maxWidthPercentage: maxWidthPercentage): - return "event.maxWidthPercentage = '\(maxWidthPercentage)';" - case let .updateTheme(themeName: themeName): - return "event.themeName = '\(themeName)';" - case let .updateFontSize(size: size): - return "event.fontSize = '\(size)';" - case let .updateColorMode(isDark: isDark): - return "event.isDark = '\(isDark)';" - case let .updateFontFamily(family: family): - return "event.fontFamily = '\(family)';" - case let .saveAnnotation(annotation: annotation): - return "event.annotation = '\(annotation)';" - case let .speakingSection(anchorIdx: anchorIdx): - return "event.anchorIdx = '\(anchorIdx)';" - case .annotate, .highlight, .share, .remove, .copyHighlight, .dismissHighlight: - return "" + get throws { + switch self { + case let .handleFontContrastChange(isHighContrast: isHighContrast): + return "event.fontContrast = '\(isHighContrast ? "high" : "normal")';" + case let .updateLineHeight(height: height): + return "event.lineHeight = '\(height)';" + case let .updateMaxWidthPercentage(maxWidthPercentage: maxWidthPercentage): + return "event.maxWidthPercentage = '\(maxWidthPercentage)';" + case let .updateTheme(themeName: themeName): + return "event.themeName = '\(themeName)';" + case let .updateFontSize(size: size): + return "event.fontSize = '\(size)';" + case let .updateColorMode(isDark: isDark): + return "event.isDark = '\(isDark)';" + case let .updateFontFamily(family: family): + return "event.fontFamily = '\(family)';" + case let .saveAnnotation(annotation: annotation): + let encoder = JSONEncoder() + if let encoded = try? encoder.encode(annotation) { + let str = String(decoding: encoded, as: UTF8.self) + return "event.annotation = '\(str)';" + } else { + throw BasicError.message(messageText: "Unable to serialize highlight note.") + } + case let .speakingSection(anchorIdx: anchorIdx): + return "event.anchorIdx = '\(anchorIdx)';" + case .annotate, .highlight, .share, .remove, .copyHighlight, .dismissHighlight: + return "" + } } } } diff --git a/apple/OmnivoreKit/Sources/Views/FontSizeAdjustmentPopoverView.swift b/apple/OmnivoreKit/Sources/Views/FontSizeAdjustmentPopoverView.swift index 9d9d12f93..564e60611 100644 --- a/apple/OmnivoreKit/Sources/Views/FontSizeAdjustmentPopoverView.swift +++ b/apple/OmnivoreKit/Sources/Views/FontSizeAdjustmentPopoverView.swift @@ -103,8 +103,8 @@ public enum WebFont: String, CaseIterable { public var body: some View { NavigationView { VStack(alignment: .center) { - themePicker - .padding(.bottom, 16) +// themePicker +// .padding(.bottom, 16) LabelledStepper( labelText: "Font Size", diff --git a/apple/OmnivoreKit/Sources/Views/Web/BasicWebAppView.swift b/apple/OmnivoreKit/Sources/Views/Web/BasicWebAppView.swift index 2618c0570..76e491f80 100644 --- a/apple/OmnivoreKit/Sources/Views/Web/BasicWebAppView.swift +++ b/apple/OmnivoreKit/Sources/Views/Web/BasicWebAppView.swift @@ -19,7 +19,8 @@ import WebKit webView.isOpaque = false webView.backgroundColor = UIColor.clear if let url = request.url { - let themeID = Color.isDarkMode ? "Gray" /* "Sepia" */ : "Charcoal" + // let themeID = Color.isDarkMode ? "Gray" /* "Sepia" */ : "Charcoal" + let themeID = Color.isDarkMode ? "Gray" : "LightGray" webView.injectCookie(cookieString: "theme=\(themeID); Max-Age=31536000;", url: url) } return webView