popover bug fixes, refactoring

zotero · Aug 8, 2024 · d0b4813 · d0b4813
1 parent 7793223
commit d0b4813
Show file tree

Hide file tree

Showing 7 changed files with 86 additions and 54 deletions.
diff --git a/Zotero/Scenes/Detail/HTML:EPUB/HtmlEpubCoordinator.swift b/Zotero/Scenes/Detail/HTML:EPUB/HtmlEpubCoordinator.swift
@@ -219,7 +219,7 @@ extension HtmlEpubCoordinator: HtmlEpubSidebarCoordinatorDelegate {
         popoverDelegate: UIPopoverPresentationControllerDelegate,
         userInterfaceStyle: UIUserInterfaceStyle
     ) -> PublishSubject<AnnotationPopoverState>? {
-        guard let currentNavigationController = navigationController, let annotation = viewModel.state.selectedAnnotationKey.flatMap({ viewModel.state.annotations[$0] }) else { return nil }
+        guard let currentNavigationController = navigationController, let annotation = viewModel.state.annotationPopoverKey.flatMap({ viewModel.state.annotations[$0] }) else { return nil }
 
         DDLogInfo("HtmlEpubCoordinator: show annotation popover")
 

diff --git a/Zotero/Scenes/Detail/HTML:EPUB/Models/HtmlEpubReaderAction.swift b/Zotero/Scenes/Detail/HTML:EPUB/Models/HtmlEpubReaderAction.swift
@@ -14,6 +14,7 @@ enum HtmlEpubReaderAction {
     case deinitialiseReader
     case deselectAnnotationDuringEditing(String)
     case deselectSelectedAnnotation
+    case hideAnnotationPopover
     case initialiseReader
     case loadDocument
     case parseAndCacheComment(key: String, comment: String)
@@ -22,9 +23,9 @@ enum HtmlEpubReaderAction {
     case saveAnnotations([String: Any])
     case searchAnnotations(String)
     case searchDocument(String)
-    case selectAnnotationDuringEditing(String)
-    case selectAnnotationFromSidebar(String)
-    case selectAnnotationFromDocument(key: String, rect: CGRect)
+    case selectAnnotationDuringEditing(key: String)
+    case selectAnnotationFromSidebar(key: String)
+    case selectAnnotationFromDocument(key: String)
     case setColor(key: String, color: String)
     case setComment(key: String, comment: NSAttributedString)
     case setCommentActive(Bool)
@@ -33,6 +34,7 @@ enum HtmlEpubReaderAction {
     case setTags(key: String, tags: [Tag])
     case setToolOptions(color: String?, size: CGFloat?, tool: AnnotationTool)
     case setViewState([String: Any])
+    case showAnnotationPopover(key: String, rect: CGRect)
     case toggleTool(AnnotationTool)
     case updateAnnotationProperties(key: String, color: String, lineWidth: CGFloat, pageLabel: String, updateSubsequentLabels: Bool, highlightText: String)
 }
diff --git a/Zotero/Scenes/Detail/HTML:EPUB/Models/HtmlEpubReaderState.swift b/Zotero/Scenes/Detail/HTML:EPUB/Models/HtmlEpubReaderState.swift
@@ -26,6 +26,7 @@ struct HtmlEpubReaderState: ViewModelState {
         static let sidebarEditingSelection = Changes(rawValue: 1 << 7)
         static let settings = Changes(rawValue: 1 << 8)
         static let readerInitialised = Changes(rawValue: 1 << 9)
+        static let popover = Changes(rawValue: 1 << 10)
     }
 
     struct DocumentData {
@@ -73,7 +74,11 @@ struct HtmlEpubReaderState: ViewModelState {
     var annotationSearchTerm: String?
     var annotationFilter: AnnotationsFilter?
     var selectedAnnotationKey: String?
-    var selectedAnnotationRect: CGRect?
+    var selectedAnnotationCommentActive: Bool
+    /// Selected annotations when annotations are being edited in sidebar
+    var selectedAnnotationsDuringEditing: Set<String>
+    var annotationPopoverKey: String?
+    var annotationPopoverRect: CGRect?
     var documentSearchTerm: String?
     var comments: [String: NSAttributedString]
     var changes: Changes
@@ -86,12 +91,9 @@ struct HtmlEpubReaderState: ViewModelState {
     var focusSidebarKey: String?
     /// Annotation key to focus in document
     var focusDocumentKey: String?
-    var selectedAnnotationCommentActive: Bool
     var sidebarEditingEnabled: Bool
     var notificationToken: NotificationToken?
     var deletionEnabled: Bool
-    /// Selected annotations when annotations are being edited in sidebar
-    var selectedAnnotationsDuringEditing: Set<String>
 
     init(url: URL, key: String, settings: HtmlEpubSettings, libraryId: LibraryIdentifier, userId: Int, username: String) {
         let originalFile = Files.file(from: url)

diff --git a/Zotero/Scenes/Detail/HTML:EPUB/ViewModels/HtmlEpubReaderActionHandler.swift b/Zotero/Scenes/Detail/HTML:EPUB/ViewModels/HtmlEpubReaderActionHandler.swift
@@ -70,12 +70,26 @@ final class HtmlEpubReaderActionHandler: ViewModelActionHandler, BackgroundDbPro
 
         case .selectAnnotationFromSidebar(let key):
             update(viewModel: viewModel) { state in
-                _select(data: (key, nil), didSelectInDocument: false, state: &state)
+                _select(key: key, didSelectInDocument: false, state: &state)
             }
 
-        case .selectAnnotationFromDocument(let key, let rect):
+        case .selectAnnotationFromDocument(let key):
             update(viewModel: viewModel) { state in
-                _select(data: (key, rect), didSelectInDocument: true, state: &state)
+                _select(key: key, didSelectInDocument: true, state: &state)
+            }
+
+        case .showAnnotationPopover(let key, let rect):
+            update(viewModel: viewModel) { state in
+                state.annotationPopoverKey = key
+                state.annotationPopoverRect = rect
+                state.changes.insert(.popover)
+            }
+
+        case .hideAnnotationPopover:
+            update(viewModel: viewModel) { state in
+                state.annotationPopoverKey = nil
+                state.annotationPopoverRect = nil
+                state.changes.insert(.popover)
             }
 
         case .setComment(let key, let comment):
@@ -91,7 +105,7 @@ final class HtmlEpubReaderActionHandler: ViewModelActionHandler, BackgroundDbPro
 
         case .deselectSelectedAnnotation:
             update(viewModel: viewModel) { state in
-                _select(data: nil, didSelectInDocument: false, state: &state)
+                _select(key: nil, didSelectInDocument: false, state: &state)
             }
 
         case .parseAndCacheComment(key: let key, comment: let comment):
@@ -188,7 +202,7 @@ final class HtmlEpubReaderActionHandler: ViewModelActionHandler, BackgroundDbPro
 
             if enabled {
                 // Deselect selected annotation before editing
-                _select(data: nil, didSelectInDocument: false, state: &state)
+                _select(key: nil, didSelectInDocument: false, state: &state)
             } else {
                 // Deselect selected annotations during editing
                 state.selectedAnnotationsDuringEditing = []
@@ -365,12 +379,11 @@ final class HtmlEpubReaderActionHandler: ViewModelActionHandler, BackgroundDbPro
         }
     }
 
-    private func _select(data: (String, CGRect?)?, didSelectInDocument: Bool, state: inout HtmlEpubReaderState) {
-        guard data?.0 != state.selectedAnnotationKey else { return }
+    private func _select(key: String?, didSelectInDocument: Bool, state: inout HtmlEpubReaderState) {
+        guard key != state.selectedAnnotationKey else { return }
 
         if let existing = state.selectedAnnotationKey {
             add(updatedAnnotationKey: existing, state: &state)
-
             if state.selectedAnnotationCommentActive {
                 state.selectedAnnotationCommentActive = false
                 state.changes.insert(.activeComment)
@@ -379,21 +392,17 @@ final class HtmlEpubReaderActionHandler: ViewModelActionHandler, BackgroundDbPro
 
         state.changes.insert(.selection)
 
-        guard let (key, rect) = data else {
+        guard let key else {
             state.selectedAnnotationKey = nil
-            state.selectedAnnotationRect = nil
             return
         }
 
         state.selectedAnnotationKey = key
-        state.selectedAnnotationRect = rect
-
         if !didSelectInDocument {
             state.focusDocumentKey = key
         } else {
             state.focusSidebarKey = key
         }
-
         add(updatedAnnotationKey: key, state: &state)
 
         func add(updatedAnnotationKey key: String, state: inout HtmlEpubReaderState) {
@@ -792,7 +801,7 @@ final class HtmlEpubReaderActionHandler: ViewModelActionHandler, BackgroundDbPro
 
             // Update selection
             if selectionDeleted {
-                _select(data: nil, didSelectInDocument: true, state: &state)
+                _select(key: nil, didSelectInDocument: true, state: &state)
             }
 
             // Disable sidebar editing if there are no results
@@ -829,11 +838,6 @@ final class HtmlEpubReaderActionHandler: ViewModelActionHandler, BackgroundDbPro
 
 extension RItem {
     fileprivate var htmlEpubAnnotation: (HtmlEpubAnnotation, [String: Any])? {
-        let tags = Array(tags.map({ typedTag in
-            let color: String? = (typedTag.tag?.color ?? "").isEmpty ? nil : typedTag.tag?.color
-            return Tag(name: typedTag.tag?.name ?? "", color: color ?? "")
-        }))
-
         var type: AnnotationType?
         var position: [String: Any] = [:]
         var text: String?
@@ -845,11 +849,8 @@ extension RItem {
 
         for field in fields {
             switch (field.key, field.baseKey) {
-            case (FieldKeys.Item.Annotation.Position.htmlEpubType, FieldKeys.Item.Annotation.position):
-                position[FieldKeys.Item.Annotation.Position.htmlEpubType] = field.value
-
-            case (FieldKeys.Item.Annotation.Position.htmlEpubValue, FieldKeys.Item.Annotation.position):
-                position[FieldKeys.Item.Annotation.Position.htmlEpubValue] = field.value
+            case (_, FieldKeys.Item.Annotation.position):
+                position[field.key] = field.value
 
             case (FieldKeys.Item.Annotation.type, nil):
                 type = AnnotationType(rawValue: field.value)
@@ -882,7 +883,12 @@ extension RItem {
             return nil
         }
 
-        let json: [String: Any] = [
+        let tags = Array(tags.map({ typedTag in
+            let color: String? = (typedTag.tag?.color ?? "").isEmpty ? nil : typedTag.tag?.color
+            return Tag(name: typedTag.tag?.name ?? "", color: color ?? "")
+        }))
+
+        var json: [String: Any] = [
             "id": key,
             "dateCreated": DateFormatter.iso8601WithFractionalSeconds.string(from: dateAdded),
             "dateModified": DateFormatter.iso8601WithFractionalSeconds.string(from: dateModified),
@@ -896,6 +902,10 @@ extension RItem {
             "position": position,
             "tags": tags.map({ ["name": $0.name, "color": $0.color] })
         ]
+        for (key, value) in unknown {
+            json[key] = value
+        }
+
         let annotation = HtmlEpubAnnotation(
             key: key,
             type: type,

diff --git a/Zotero/Scenes/Detail/HTML:EPUB/Views/HtmlEpubDocumentViewController.swift b/Zotero/Scenes/Detail/HTML:EPUB/Views/HtmlEpubDocumentViewController.swift
@@ -208,7 +208,7 @@ class HtmlEpubDocumentViewController: UIViewController {
 
             switch event {
             case "onInitialized":
-                self.viewModel.process(action: .loadDocument)
+                viewModel.process(action: .loadDocument)
 
             case "onSaveAnnotations":
                 guard let params = data["params"] as? [String: Any] else {
@@ -225,7 +225,7 @@ class HtmlEpubDocumentViewController: UIViewController {
                 }
 
                 if params.isEmpty {
-                    viewModel.process(action: .deselectSelectedAnnotation)
+                    viewModel.process(action: .hideAnnotationPopover)
                     return
                 }
 
@@ -236,7 +236,18 @@ class HtmlEpubDocumentViewController: UIViewController {
 
                 let navigationBarInset = (parentDelegate?.statusBarHeight ?? 0) + (parentDelegate?.navigationBarHeight ?? 0)
                 let rect = CGRect(x: rectArray[0], y: rectArray[1] + navigationBarInset, width: rectArray[2] - rectArray[0], height: rectArray[3] - rectArray[1])
-                viewModel.process(action: .selectAnnotationFromDocument(key: key, rect: rect))
+                viewModel.process(action: .showAnnotationPopover(key: key, rect: rect))
+
+            case "onSelectAnnotations":
+                guard let params = data["params"] as? [String: Any], let ids = params["ids"] as? [String] else {
+                    DDLogWarn("HtmlEpubDocumentViewController: event \(event) missing params - \(message)")
+                    return
+                }
+                if let key = ids.first {
+                    viewModel.process(action: .selectAnnotationFromDocument(key: key))
+                } else {
+                    viewModel.process(action: .deselectSelectedAnnotation)
+                }
 
             case "onChangeViewState":
                 guard let params = data["params"] as? [String: Any] else {

diff --git a/Zotero/Scenes/Detail/HTML:EPUB/Views/HtmlEpubReaderViewController.swift b/Zotero/Scenes/Detail/HTML:EPUB/Views/HtmlEpubReaderViewController.swift
@@ -274,7 +274,13 @@ class HtmlEpubReaderViewController: UIViewController, ParentWithSidebarControlle
             navigationController?.overrideUserInterfaceStyle = state.settings.appearance.userInterfaceStyle
         }
 
-        handleAnnotationPopover()
+        if state.changes.contains(.popover) {
+            if let key = state.annotationPopoverKey, let rect = state.annotationPopoverRect {
+                showPopover(forKey: key, rect: rect)
+            } else {
+                hidePopover()
+            }
+        }
 
         func select(activeTool tool: AnnotationTool?) {
             if let tool = activeAnnotationTool {
@@ -290,21 +296,22 @@ class HtmlEpubReaderViewController: UIViewController, ParentWithSidebarControlle
         func show(error: HtmlEpubReaderState.Error) {
         }
 
-        func handleAnnotationPopover() {
-            if let key = state.selectedAnnotationKey {
-                if !isSidebarVisible, let rect = state.selectedAnnotationRect {
-                    let observable = coordinatorDelegate?.showAnnotationPopover(
-                        viewModel: viewModel,
-                        sourceRect: rect,
-                        popoverDelegate: self,
-                        userInterfaceStyle: viewModel.state.settings.appearance.userInterfaceStyle
-                    )
-                    observe(key: key, popoverObservable: observable)
-                }
-            } else if navigationController?.presentedViewController is AnnotationPopover ||
-               (navigationController?.presentedViewController as? UINavigationController)?.topViewController is AnnotationPopover {
-                navigationController?.dismiss(animated: true)
-            }
+        func showPopover(forKey key: String, rect: CGRect) {
+            guard !isSidebarVisible else { return }
+            let observable = coordinatorDelegate?.showAnnotationPopover(
+                viewModel: viewModel,
+                sourceRect: rect,
+                popoverDelegate: self,
+                userInterfaceStyle: viewModel.state.settings.appearance.userInterfaceStyle
+            )
+            observe(key: key, popoverObservable: observable)
+        }
+
+        func hidePopover() {
+            guard navigationController?.presentedViewController is AnnotationPopover ||
+                  (navigationController?.presentedViewController as? UINavigationController)?.topViewController is AnnotationPopover
+            else { return }
+            navigationController?.dismiss(animated: true)
         }
 
         func observe(key: String, popoverObservable observable: PublishSubject<AnnotationPopoverState>?) {

diff --git a/Zotero/Scenes/Detail/HTML:EPUB/Views/HtmlEpubSidebarViewController.swift b/Zotero/Scenes/Detail/HTML:EPUB/Views/HtmlEpubSidebarViewController.swift
@@ -419,9 +419,9 @@ extension HtmlEpubSidebarViewController: UITableViewDelegate {
     func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
         guard let key = dataSource.itemIdentifier(for: indexPath) else { return }
         if viewModel.state.sidebarEditingEnabled {
-            viewModel.process(action: .selectAnnotationDuringEditing(key))
+            viewModel.process(action: .selectAnnotationDuringEditing(key: key))
         } else {
-            viewModel.process(action: .selectAnnotationFromSidebar(key))
+            viewModel.process(action: .selectAnnotationFromSidebar(key: key))
         }
     }