Note editor adjustments, removed callback from note editor

Zotero/Scenes/AppCoordinator.swift

@@ -913,9 +913,7 @@ extension AppCoordinator: OpenItemsPresenter {
 
                     case .note(let library, let key, let text, let tags, let parentTitleData, let title):
                         let kind: NoteEditorKind = library.metadataEditable ? .edit(key: key) : .readOnly(key: key)
-                        // TODO: Check if a callback is required
-                        return coordinator.createNoteController(library: library, kind: kind, text: text, tags: tags, parentTitleData: parentTitleData, title: title) { _, _ in
-                        }
+                        return coordinator.createNoteController(library: library, kind: kind, text: text, tags: tags, parentTitleData: parentTitleData, title: title)
                     }
                 },
                 by: mainController,

Zotero/Scenes/Detail/DetailCoordinator.swift

@@ -68,7 +68,7 @@ protocol DetailItemDetailCoordinatorDelegate: AnyObject {
 }
 
 protocol DetailNoteEditorCoordinatorDelegate: AnyObject {
-    func showNote(library: Library, kind: NoteEditorKind, text: String, tags: [Tag], parentTitleData: NoteEditorState.TitleData?, title: String?, saveCallback: @escaping NoteEditorSaveCallback)
+    func showNote(library: Library, kind: NoteEditorKind, text: String, tags: [Tag], parentTitleData: NoteEditorState.TitleData?, title: String?, saveCallback: ((Note) -> Void)?)
 }
 
 protocol ItemsTagFilterDelegate: AnyObject {
@@ -400,9 +400,7 @@ extension DetailCoordinator: DetailItemsCoordinatorDelegate {
 
         controller.addAction(UIAlertAction(title: L10n.Items.newNote, style: .default, handler: { [weak self, weak viewModel] _ in
             guard let self, let viewModel else { return }
-            showNote(library: viewModel.state.library, kind: .standaloneCreation(collection: viewModel.state.collection)) { [weak viewModel] _, result in
-                viewModel?.process(action: .processNoteSaveResult(result))
-            }
+            showNote(library: viewModel.state.library, kind: .standaloneCreation(collection: viewModel.state.collection), saveCallback: nil)
         }))
 
         if viewModel.state.library.metadataAndFilesEditable {
@@ -481,9 +479,19 @@ extension DetailCoordinator: DetailItemsCoordinatorDelegate {
         text: String,
         tags: [Tag],
         parentTitleData: NoteEditorState.TitleData?,
-        title: String?,
-        saveCallback: @escaping NoteEditorSaveCallback
+        title: String?
     ) -> NavigationViewController {
+        return createNoteController(library: library, kind: kind, text: text, tags: tags, parentTitleData: parentTitleData, title: title).0
+    }
+
+    private func createNoteController(
+        library: Library,
+        kind: NoteEditorKind,
+        text: String,
+        tags: [Tag],
+        parentTitleData: NoteEditorState.TitleData?,
+        title: String?
+    ) -> (NavigationViewController, ViewModel<NoteEditorActionHandler>) {
         let navigationController = NavigationViewController()
         navigationController.modalPresentationStyle = .fullScreen
         navigationController.isModalInPresentation = true
@@ -495,15 +503,14 @@ extension DetailCoordinator: DetailItemsCoordinatorDelegate {
             tags: tags,
             parentTitleData: parentTitleData,
             title: title,
-            saveCallback: saveCallback,
             navigationController: navigationController,
             controllers: controllers
         )
         coordinator.parentCoordinator = self
         childCoordinators.append(coordinator)
         coordinator.start(animated: false)
 
-        return navigationController
+        return (navigationController, coordinator.viewModel!)
     }
 
     func showItemDetail(for type: ItemDetailState.DetailType, libraryId: LibraryIdentifier, scrolledToKey childKey: String?, animated: Bool) {
@@ -964,7 +971,7 @@ extension DetailCoordinator: DetailNoteEditorCoordinatorDelegate {
         tags: [Tag] = [],
         parentTitleData: NoteEditorState.TitleData? = nil,
         title: String? = nil,
-        saveCallback: @escaping NoteEditorSaveCallback = { _, _  in }
+        saveCallback: ((Note) -> Void)?
     ) {
         guard let navigationController else { return }
         switch kind {
@@ -974,8 +981,18 @@ extension DetailCoordinator: DetailNoteEditorCoordinatorDelegate {
         case .edit(let key), .readOnly(let key):
             DDLogInfo("DetailCoordinator: show note \(key)")
         }
-        let controller = createNoteController(library: library, kind: kind, text: text, tags: tags, parentTitleData: parentTitleData, title: title, saveCallback: saveCallback)
+        let (controller, viewModel) = createNoteController(library: library, kind: kind, text: text, tags: tags, parentTitleData: parentTitleData, title: title)
         navigationController.present(controller, animated: true)
+
+        if let saveCallback {
+            viewModel.stateObservable
+                .observe(on: MainScheduler.instance)
+                .subscribe(onNext: { [weak self] state in
+                    guard let self, state.changes.contains(.saved), case .edit(let key) = state.kind else { return }
+                    saveCallback(Note(key: key, text: state.text, tags: state.tags))
+                })
+                .disposed(by: disposeBag)
+        }
     }
 }
 

Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailAction.swift

@@ -26,7 +26,7 @@ enum ItemDetailAction {
     case openAttachment(String)
     case attachmentOpened(String)
     case reloadData
-    case processNoteSaveResult(key: String?, result: NoteEditorSaveResult)
+    case processNoteSaveResult(note: Note)
     case setFieldValue(id: String, value: String)
     case setTags([Tag])
     case setTitle(String)

Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift

@@ -94,8 +94,8 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
         case .moveCreators(let diff):
             self.moveCreators(diff: diff, in: viewModel)
 
-        case .processNoteSaveResult(let key, let result):
-            self.processNoteSaveResult(key: key, result: result, in: viewModel)
+        case .processNoteSaveResult(let note):
+            processNoteSaveResult(note: note, in: viewModel)
 
         case .setTags(let tags):
             self.set(tags: tags, in: viewModel)
@@ -350,44 +350,14 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
 
     // MARK: - Notes
 
-    private func processNoteSaveResult(key: String?, result: NoteEditorSaveResult, in viewModel: ViewModel<ItemDetailActionHandler>) {
-        var oldIndex: Int?
-        var oldNote: Note?
-        if let key {
-            oldIndex = viewModel.state.notes.firstIndex(where: { $0.key == key })
-            if let oldIndex {
-                oldNote = viewModel.state.notes[oldIndex]
-            }
-        }
-        switch result {
-        case .success((let note, _)):
-            update(viewModel: viewModel) { state in
-                if let oldIndex {
-                    state.notes[oldIndex] = note
-                } else {
-                    state.notes.append(note)
-                }
-
-                state.reload = .section(.notes)
-            }
-
-        case .failure(let error):
-            if let key {
-                DDLogError("ItemDetailActionHandler: Can't update item note \(key) - \(error)")
+    private func processNoteSaveResult(note: Note, in viewModel: ViewModel<ItemDetailActionHandler>) {
+        update(viewModel: viewModel) { state in
+            if let index = state.notes.firstIndex(where: { $0.key == note.key }) {
+                state.notes[index] = note
             } else {
-                DDLogError("ItemDetailActionHandler: Can't create item note - \(error)")
-            }
-            update(viewModel: viewModel) { state in
-                state.reload = .section(.notes)
-                state.error = .cantSaveNote
-
-                guard let oldIndex else { return }
-                if let oldNote {
-                    state.notes[oldIndex] = oldNote
-                } else {
-                    state.notes.remove(at: oldIndex)
-                }
+                state.notes.append(note)
             }
+            state.reload = .section(.notes)
         }
     }
 

Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailViewController.swift

@@ -137,8 +137,8 @@ final class ItemDetailViewController: UIViewController {
                 tags = note.tags
                 title = note.title
             }
-            coordinatorDelegate?.showNote(library: library, kind: kind, text: text, tags: tags, parentTitleData: parentTitleData, title: title) { [weak self] key, result in
-                self?.viewModel.process(action: .processNoteSaveResult(key: key, result: result))
+            coordinatorDelegate?.showNote(library: library, kind: kind, text: text, tags: tags, parentTitleData: parentTitleData, title: title) { [weak self] note in
+                self?.viewModel.process(action: .processNoteSaveResult(note: note))
             }
 
         case .openTagPicker:

Zotero/Scenes/Detail/Items/Models/ItemsAction.swift

@@ -28,7 +28,6 @@ enum ItemsAction {
     case observingFailed
     case removeDownloads(Set<String>)
     case restoreItems(Set<String>)
-    case processNoteSaveResult(NoteEditorSaveResult)
     case search(String)
     case selectItem(String)
     case setSortField(ItemsSortType.Field)

Zotero/Scenes/Detail/Items/ViewModels/ItemsActionHandler.swift

@@ -96,9 +96,6 @@ struct ItemsActionHandler: ViewModelActionHandler, BackgroundDbProcessingActionH
                 state.error = .dataLoading
             }
 
-        case .processNoteSaveResult(let result):
-            processNoteSaveResult(result: result, in: viewModel)
-
         case .search(let text):
             self.search(for: (text.isEmpty ? nil : text), ignoreOriginal: false, in: viewModel)
 
@@ -501,22 +498,6 @@ struct ItemsActionHandler: ViewModelActionHandler, BackgroundDbProcessingActionH
         Defaults.shared.itemsSortType = sortType
     }
 
-    private func processNoteSaveResult(result: NoteEditorSaveResult, in viewModel: ViewModel<ItemsActionHandler>) {
-        switch result {
-        case .success:
-            break
-
-        case .failure(let error):
-            DispatchQueue.main.async { [weak viewModel] in
-                DDLogError("ItemsStore: can't save note: \(error)")
-                guard let viewModel else { return }
-                update(viewModel: viewModel) { state in
-                    state.error = .noteSaving
-                }
-            }
-        }
-    }
-
     private func addAttachments(urls: [URL], in viewModel: ViewModel<ItemsActionHandler>) {
         let libraryId = viewModel.state.library.identifier
         var attachments: [Attachment] = []

Zotero/Scenes/Detail/Items/Views/ItemsViewController.swift

@@ -208,9 +208,7 @@ final class ItemsViewController: UIViewController {
             guard let note = Note(item: item) else { return }
             let tags = Array(item.tags.map({ Tag(tag: $0) }))
             let library = self.viewModel.state.library
-            coordinatorDelegate?.showNote(library: library, kind: .edit(key: note.key), text: note.text, tags: tags, parentTitleData: nil, title: note.title) { [weak self] _, result in
-                self?.viewModel.process(action: .processNoteSaveResult(result))
-            }
+            coordinatorDelegate?.showNote(library: library, kind: .edit(key: note.key), text: note.text, tags: tags, parentTitleData: nil, title: note.title, saveCallback: nil)
         }
     }
 

Zotero/Scenes/General/Models/NoteEditorAction.swift

@@ -9,11 +9,12 @@
 import Foundation
 
 enum NoteEditorAction {
-    case setup
+    case deleteResource([String: Any])
+    case importImages([String: Any])
+    case loadResource([String: Any])
     case save
+    case saveBeforeClosing
     case setTags([Tag])
     case setText(String)
-    case loadResource([String: Any])
-    case deleteResource([String: Any])
-    case importImages([String: Any])
+    case setup
 }

Zotero/Scenes/General/Models/NoteEditorState.swift

@@ -51,9 +51,11 @@ struct NoteEditorState: ViewModelState {
         let rawValue: UInt8
 
         static let tags = Changes(rawValue: 1 << 0)
-        static let save = Changes(rawValue: 1 << 1)
+        static let shouldSave = Changes(rawValue: 1 << 1)
         static let kind = Changes(rawValue: 1 << 3)
         static let title = Changes(rawValue: 1 << 4)
+        static let saved = Changes(rawValue: 1 << 5)
+        static let closing = Changes(rawValue: 1 << 6)
     }
 
     struct TitleData {
@@ -77,6 +79,8 @@ struct NoteEditorState: ViewModelState {
     var createdImages: [CreatedImage]
     var changes: Changes
     var title: String?
+    var isClosing: Bool
+    var error: Swift.Error?
 
     init(kind: Kind, library: Library, parentTitleData: TitleData?, text: String, tags: [Tag], title: String?) {
         self.kind = kind
@@ -85,6 +89,7 @@ struct NoteEditorState: ViewModelState {
         self.library = library
         self.parentTitleData = parentTitleData
         self.title = title
+        isClosing = false
         pendingResources = [:]
         changes = []
         createdImages = []
@@ -94,5 +99,6 @@ struct NoteEditorState: ViewModelState {
         downloadedResource = nil
         changes = []
         createdImages = []
+        error = nil
     }
 }

Zotero/Scenes/General/NoteEditorCoordinator.swift

@@ -11,18 +11,13 @@ import SafariServices
 
 import CocoaLumberjackSwift
 
-typealias NoteEditorSaveResult = NoteEditorCoordinator.SaveResult
-typealias NoteEditorSaveCallback = NoteEditorCoordinator.SaveCallback
-
 protocol NoteEditorCoordinatorDelegate: AnyObject {
     func show(url: URL)
     func showTagPicker(libraryId: LibraryIdentifier, selected: Set<String>, picked: @escaping ([Tag]) -> Void)
+    func show(error: Error)
 }
 
 final class NoteEditorCoordinator: NSObject, Coordinator {
-    typealias SaveResult = Result<(note: Note, isCreated: Bool), Error>
-    typealias SaveCallback = (_ key: String?, _ result: SaveResult) -> Void
-
     weak var parentCoordinator: Coordinator?
     var childCoordinators: [Coordinator]
     private var transitionDelegate: EmptyTransitioningDelegate?
@@ -34,17 +29,19 @@ final class NoteEditorCoordinator: NSObject, Coordinator {
     private let parentTitleData: NoteEditorState.TitleData?
     private let title: String?
     private let library: Library
-    private let saveCallback: NoteEditorSaveCallback
     private unowned let controllers: Controllers
 
+    var viewModel: ViewModel<NoteEditorActionHandler>? {
+        (navigationController?.viewControllers.first as? NoteEditorViewController)?.viewModel
+    }
+
     init(
         library: Library,
         kind: NoteEditorKind,
         text: String,
         tags: [Tag],
         parentTitleData: NoteEditorState.TitleData?,
         title: String?,
-        saveCallback: @escaping NoteEditorSaveCallback,
         navigationController: NavigationViewController,
         controllers: Controllers
     ) {
@@ -54,7 +51,6 @@ final class NoteEditorCoordinator: NSObject, Coordinator {
         self.parentTitleData = parentTitleData
         self.title = title
         self.library = library
-        self.saveCallback = saveCallback
         self.navigationController = navigationController
         self.controllers = controllers
         childCoordinators = []
@@ -78,8 +74,7 @@ final class NoteEditorCoordinator: NSObject, Coordinator {
             dbStorage: dbStorage,
             fileStorage: controllers.fileStorage,
             schemaController: controllers.schemaController,
-            attachmentDownloader: fileDownloader,
-            saveCallback: saveCallback
+            attachmentDownloader: fileDownloader
         )
         let viewModel = ViewModel(initialState: state, handler: handler)
         let controller = NoteEditorViewController(viewModel: viewModel)
@@ -104,4 +99,10 @@ extension NoteEditorCoordinator: NoteEditorCoordinatorDelegate {
         guard let detailCoordinator = parentCoordinator as? DetailCoordinator else { return }
         detailCoordinator.show(url: url)
     }
+
+    func show(error: any Error) {
+        let controller = UIAlertController(title: L10n.error, message: error.localizedDescription, preferredStyle: .alert)
+        controller.addAction(UIAlertAction(title: L10n.ok, style: .cancel))
+        navigationController?.present(controller, animated: true)
+    }
 }