From e414183d298837e4f66b0e9df02b4908e7f31c49 Mon Sep 17 00:00:00 2001 From: Miltiadis Vasilakis Date: Fri, 22 Sep 2023 12:52:37 +0300 Subject: [PATCH 1/5] Add support for multiple open items Improve AppCoordinator code Keep track of open items count in items navigation bar Add ability to restore most recently opened item Implement state restoration for open items Allow switch to another open item via bar button menu Use instant presenter to switch between open items Maintain user order of open items Add support for notes in open items Validate open items when set on app launch Observe open items for deletions Add support for different open items per session Improve DetailCoordinator presented item replacement for multiple items Set user activity when new note is actually created Show actions submenu for current item Improve ItemsViewController right bar button items Add icons item type icons to open items menu Add close all action to open items menu Simplify NoteEditorViewController open items button creation Add PDFReaderViewController open items observer Add close other items action to submenu for current item Add getSessionIdentifier convenience property to UIViewController Save user activity when open items change w/o current item change Open items in their respective scene, if already open Update copy Improve NSUserActivity extension Simplify DetailCoordinator Improve open items bar button image creation Improve open items bar button creation Properly close PDF Reader when switching current open item Properly close Note Editor when switching current open item Improve note editor save callback for new notes Improve note editor activity title update Improve NoteEditorActionHandler Refactor PDFReaderState Improve PDFReaderActionHandler Pass note title when showing note editor Clarify NoteEditorState parameter names --- Zotero/Assets/en.lproj/Localizable.strings | 8 + .../Architecture/Coordinator.swift | 2 +- Zotero/Controllers/Controllers.swift | 2 + .../Requests/ReadItemsDbRequest.swift | 12 + Zotero/Controllers/OpenItemsController.swift | 384 ++++++++++++++++++ Zotero/Extensions/Localizable.swift | 16 + .../NSUserActivity+Activities.swift | 8 +- .../UIViewController+Extensions.swift | 4 + Zotero/Models/Predicates.swift | 4 + Zotero/SceneDelegate.swift | 7 +- Zotero/Scenes/AppCoordinator.swift | 85 +--- Zotero/Scenes/Detail/DetailCoordinator.swift | 150 +++++-- .../Detail/Items/Models/ItemsAction.swift | 1 + .../Detail/Items/Models/ItemsState.swift | 6 +- .../Items/ViewModels/ItemsActionHandler.swift | 8 + .../Items/Views/BaseItemsViewController.swift | 40 +- .../Items/Views/ItemsViewController.swift | 49 ++- .../Detail/PDF/Models/PDFReaderAction.swift | 1 + .../Detail/PDF/Models/PDFReaderState.swift | 7 +- Zotero/Scenes/Detail/PDF/PDFCoordinator.swift | 18 +- .../ViewModels/PDFReaderActionHandler.swift | 7 + .../PDF/Views/PDFReaderViewController.swift | 62 ++- .../Detail/Trash/Models/TrashAction.swift | 1 + .../Detail/Trash/Models/TrashState.swift | 5 +- .../Trash/ViewModels/TrashActionHandler.swift | 8 + .../Trash/Views/TrashViewController.swift | 46 ++- .../General/Models/NoteEditorAction.swift | 1 + .../General/Models/NoteEditorState.swift | 5 +- .../General/NoteEditorCoordinator.swift | 33 +- .../ViewModels/NoteEditorActionHandler.swift | 7 + .../Views/NavigationViewController.swift | 22 + .../Views/NoteEditorViewController.swift | 144 +++++-- .../Main/Views/MainViewController.swift | 14 +- 33 files changed, 984 insertions(+), 183 deletions(-) diff --git a/Zotero/Assets/en.lproj/Localizable.strings b/Zotero/Assets/en.lproj/Localizable.strings index 9de246b58..622d48274 100644 --- a/Zotero/Assets/en.lproj/Localizable.strings +++ b/Zotero/Assets/en.lproj/Localizable.strings @@ -147,6 +147,7 @@ "items.generating_bib" = "Generating Bibliography"; "items.creator_summary.and" = "%@ and %@"; "items.creator_summary.etal" = "%@ et al."; +"items.restore_open" = "Restore Open Items"; "lookup.title" = "Enter ISBNs, DOls, PMIDs, arXiv IDs, or ADS Bibcodes to add to your library:"; @@ -579,3 +580,10 @@ "accessibility.pdf.undo" = "Undo"; "accessibility.pdf.toggle_annotation_toolbar" = "Toggle annotation toolbar"; "accessibility.pdf.show_more_tools" = "Show more"; +"accessibility.pdf.open_items" = "Open Items"; +"accessibility.pdf.current_item" = "Current Item"; +"accessibility.pdf.current_item_close" = "Close"; +"accessibility.pdf.current_item_move_to_start" = "Move to start"; +"accessibility.pdf.current_item_move_to end" = "Move to end"; +"accessibility.pdf.close_all_open_items" = "Close all"; +"accessibility.pdf.close_other_open_items" = "Close other items"; diff --git a/Zotero/Controllers/Architecture/Coordinator.swift b/Zotero/Controllers/Architecture/Coordinator.swift index 8bf3dd36a..65bfba09c 100644 --- a/Zotero/Controllers/Architecture/Coordinator.swift +++ b/Zotero/Controllers/Architecture/Coordinator.swift @@ -16,7 +16,7 @@ enum SourceView { protocol Coordinator: AnyObject { var parentCoordinator: Coordinator? { get } var childCoordinators: [Coordinator] { get set } - var navigationController: UINavigationController? { get } + var navigationController: UINavigationController? { get set } func start(animated: Bool) func childDidFinish(_ child: Coordinator) diff --git a/Zotero/Controllers/Controllers.swift b/Zotero/Controllers/Controllers.swift index d70b24b52..62f2cbf9b 100644 --- a/Zotero/Controllers/Controllers.swift +++ b/Zotero/Controllers/Controllers.swift @@ -314,6 +314,7 @@ final class UserControllers { let webDavController: WebDavController let customUrlController: CustomURLController let fullSyncDebugger: FullSyncDebugger + let openItemsController: OpenItemsController private let isFirstLaunch: Bool private let lastBuildNumber: Int? private unowned let translatorsAndStylesController: TranslatorsAndStylesController @@ -407,6 +408,7 @@ final class UserControllers { fullSyncDebugger = FullSyncDebugger(syncScheduler: syncScheduler, debugLogging: controllers.debugLogging, sessionController: controllers.sessionController) idleTimerController = controllers.idleTimerController customUrlController = CustomURLController(dbStorage: dbStorage, fileStorage: controllers.fileStorage) + openItemsController = OpenItemsController(dbStorage: dbStorage, fileStorage: controllers.fileStorage) lastBuildNumber = controllers.lastBuildNumber disposeBag = DisposeBag() } diff --git a/Zotero/Controllers/Database/Requests/ReadItemsDbRequest.swift b/Zotero/Controllers/Database/Requests/ReadItemsDbRequest.swift index 9ae2a5252..1ce039370 100644 --- a/Zotero/Controllers/Database/Requests/ReadItemsDbRequest.swift +++ b/Zotero/Controllers/Database/Requests/ReadItemsDbRequest.swift @@ -86,3 +86,15 @@ struct ReadItemsWithKeysDbRequest: DbResponseRequest { return database.objects(RItem.self).filter(.keys(self.keys, in: self.libraryId)) } } + +struct ReadItemsWithKeysFromMultipleLibrariesDbRequest: DbResponseRequest { + typealias Response = Results + + let keysByLibraryIdentifier: [LibraryIdentifier: Set] + + var needsWrite: Bool { return false } + + func process(in database: Realm) throws -> Results { + database.objects(RItem.self).filter(.keysByLibraryIdentifier(keysByLibraryIdentifier)) + } +} diff --git a/Zotero/Controllers/OpenItemsController.swift b/Zotero/Controllers/OpenItemsController.swift index a06b3e036..6ed8bb6a1 100644 --- a/Zotero/Controllers/OpenItemsController.swift +++ b/Zotero/Controllers/OpenItemsController.swift @@ -7,6 +7,9 @@ // import UIKit +import RxSwift +import RealmSwift +import CocoaLumberjackSwift typealias OpenItem = OpenItemsController.Item typealias ItemPresentation = OpenItemsController.Presentation @@ -98,4 +101,385 @@ final class OpenItemsController { case pdf(library: Library, key: String, parentKey: String?, url: URL) case note(library: Library, key: String, text: String, tags: [Tag], parentTitleData: NoteEditorState.TitleData?, title: String) } + + // MARK: Properties + private unowned let dbStorage: DbStorage + private unowned let fileStorage: FileStorage + // TODO: Use a better data structure, such as an ordered set + private var itemsBySessionIdentifier: [String: [Item]] = [:] + private var sessionIdentifierByItemKind: [Item.Kind: String] = [:] + private var itemsTokenBySessionIdentifier: [String: NotificationToken] = [:] + private var observableBySessionIdentifier: [String: PublishSubject<[Item]>] = [:] + private let disposeBag: DisposeBag + + // MARK: Object Lifecycle + init(dbStorage: DbStorage, fileStorage: FileStorage) { + self.dbStorage = dbStorage + self.fileStorage = fileStorage + disposeBag = DisposeBag() + } + + // MARK: Actions + func observable(for sessionIdentifier: String) -> PublishSubject<[Item]> { + if let observable = observableBySessionIdentifier[sessionIdentifier] { + return observable + } + let observable = PublishSubject<[Item]>() + observableBySessionIdentifier[sessionIdentifier] = observable + return observable + } + + func getItems(for sessionIdentifier: String) -> [Item] { + itemsBySessionIdentifier[sessionIdentifier, default: []] + } + + func setItems(_ items: [Item], for sessionIdentifier: String, validate: Bool) { + DDLogInfo("OpenItemsController: setting items \(items) for \(sessionIdentifier)") + let existingItems = getItems(for: sessionIdentifier) + let newItems = validate ? filterValidItems(items) : items + guard newItems != existingItems else { return } + // Invalidate previous observer first. + itemsTokenBySessionIdentifier[sessionIdentifier]?.invalidate() + itemsTokenBySessionIdentifier[sessionIdentifier] = nil + // Update itemsBySessionIdentifier. + itemsBySessionIdentifier[sessionIdentifier] = newItems + // Update sessionIdentifierByItemKind. Recompute for all session identifier, to remove any closed items. + var newSessionIdentifierByItemKind: [Item.Kind: String] = [:] + itemsBySessionIdentifier.forEach { (sessionIdentifier, items) in + items.forEach { item in + newSessionIdentifierByItemKind[item.kind] = sessionIdentifier + } + } + sessionIdentifierByItemKind = newSessionIdentifierByItemKind + // Register observer for newly set items. + itemsTokenBySessionIdentifier[sessionIdentifier] = registerObserver(for: newItems) + observable(for: sessionIdentifier).on(.next(newItems)) + + func registerObserver(for items: [Item]) -> NotificationToken? { + var token: NotificationToken? + var keysByLibraryIdentifier: [LibraryIdentifier: Set] = [:] + for item in items { + let libraryId = item.kind.libraryId + let key = item.kind.key + var keys = keysByLibraryIdentifier[libraryId, default: .init()] + keys.insert(key) + keysByLibraryIdentifier[libraryId] = keys + } + do { + let objects = try dbStorage.perform(request: ReadItemsWithKeysFromMultipleLibrariesDbRequest(keysByLibraryIdentifier: keysByLibraryIdentifier), on: .main) + token = objects.observe { [weak self] changes in + switch changes { + case .initial: + break + + case .update(_, let deletions, _, _): + if !deletions.isEmpty, let self { + // Observed items have been deleted, call setItems to validate and register new observer. + let existingItems = getItems(for: sessionIdentifier) + setItems(existingItems, for: sessionIdentifier, validate: true) + } + + case .error(let error): + DDLogError("OpenItemsController: register observer error - \(error)") + } + } + } catch let error { + DDLogError("OpenItemsController: can't register items observer - \(error)") + } + return token + } + } + + private func setItemsSortedByUserIndex(_ items: [Item], for sessionIdentifier: String, validate: Bool) { + var newItems = items + for i in 0.. String? { + sessionIdentifierByItemKind[kind] + } + + func open(_ kind: Item.Kind, for sessionIdentifier: String) { + DDLogInfo("OpenItemsController: opened item \(kind) for \(sessionIdentifier)") + var existingItems = getItems(for: sessionIdentifier) + if let index = existingItems.firstIndex(where: { $0.kind == kind }) { + existingItems[index].lastOpened = .now + // No need to call setItems, to register a new items observer, as only items metadata were updated. + itemsBySessionIdentifier[sessionIdentifier] = existingItems + DDLogInfo("OpenItemsController: already opened item \(kind) became most recent for \(sessionIdentifier)") + observable(for: sessionIdentifier).on(.next(existingItems)) + } else { + DDLogInfo("OpenItemsController: newly opened item \(kind) set as most recent for \(sessionIdentifier)") + let item = Item(kind: kind, userIndex: existingItems.count) + let newItems = existingItems + [item] + // setItems will produce next observable event + setItems(newItems, for: sessionIdentifier, validate: false) + } + } + + func close(_ kind: Item.Kind, for sessionIdentifier: String) { + DDLogInfo("OpenItemsController: closed open item \(kind) for \(sessionIdentifier)") + var existingItems = itemsSortedByUserOrder(for: sessionIdentifier) + guard let index = existingItems.firstIndex(where: { $0.kind == kind }) else { + DDLogWarn("OpenItemsController: item was already closed") + return + } + existingItems.remove(at: index) + setItemsSortedByUserIndex(existingItems, for: sessionIdentifier, validate: false) + } + + func move(_ kind: Item.Kind, to userIndex: Int, for sessionIdentifier: String) { + DDLogInfo("OpenItemsController: moved open item \(kind) to user index \(userIndex) for \(sessionIdentifier)") + var existingItems = itemsSortedByUserOrder(for: sessionIdentifier) + let userIndex = min(existingItems.count, max(0, userIndex)) + guard let index = existingItems.firstIndex(where: { $0.kind == kind }) else { + DDLogWarn("OpenItemsController: item was not open") + return + } + existingItems.move(fromOffsets: IndexSet(integer: index), toOffset: userIndex) + setItemsSortedByUserIndex(existingItems, for: sessionIdentifier, validate: false) + } + + @discardableResult + func restore(_ item: Item, using presenter: OpenItemsPresenter) -> Bool { + guard let presentation = loadPresentation(for: item) else { return false } + presentItem(with: presentation, using: presenter) + return true + } + + @discardableResult + func restoreMostRecentlyOpenedItem(using presenter: OpenItemsPresenter, sessionIdentifier: String) -> Item? { + // Will restore most recent opened item still present, or none if all fail + var existingItems = getItems(for: sessionIdentifier) + DDLogInfo("OpenItemsController: restoring most recently opened item using presenter \(presenter) for \(sessionIdentifier)") + var itemsChanged: Bool = false + defer { + if itemsChanged { + // setItems will produce next observable event + setItems(existingItems, for: sessionIdentifier, validate: false) + } + } + var item: Item? + var presentation: Presentation? + let existingItemsSortedByLastOpen = itemsSortedByLastOpen(for: sessionIdentifier) + for _item in existingItemsSortedByLastOpen { + if let _presentation = loadPresentation(for: _item) { + item = _item + presentation = _presentation + break + } + DDLogWarn("OpenItemsController: removing not loaded item \(_item) for \(sessionIdentifier)") + existingItems.removeAll(where: { $0 == _item }) + itemsChanged = true + } + guard let item, let presentation else { return nil } + presentItem(with: presentation, using: presenter) + return item + } + + func deferredOpenItemsMenuElement( + for sessionIdentifier: String, + showMenuForCurrentItem: Bool, + openItemPresenterProvider: @escaping () -> OpenItemsPresenter?, + completion: @escaping (_ changedCurrentItem: Bool, _ openItemsChanged: Bool) -> Void + ) -> UIDeferredMenuElement { + UIDeferredMenuElement.uncached { [weak self] elementProvider in + guard let self else { + elementProvider([]) + return + } + var elements: [UIMenuElement] = [] + let openItem: Item? = showMenuForCurrentItem ? itemsSortedByLastOpen(for: sessionIdentifier).first : nil + let existingItemsSortedByLastOpen = itemsSortedByUserOrder(for: sessionIdentifier) + let itemTuples: [(Item, RItem)] = filterValidItemsWithRItem(existingItemsSortedByLastOpen) + let itemsCount = itemTuples.count + for (index, (item, rItem)) in itemTuples.enumerated() { + if item == openItem { + var currentItemActions: [UIAction] = [] + let closeAction = UIAction(title: L10n.Accessibility.Pdf.currentItemClose, image: .init(systemName: "xmark.circle")) { [weak self] _ in + guard let self else { return } + close(item.kind, for: sessionIdentifier) + guard let presenter = openItemPresenterProvider() else { return } + if restoreMostRecentlyOpenedItem(using: presenter, sessionIdentifier: sessionIdentifier) == nil { + DDLogInfo("OpenItemsController: no open item to restore after close") + presenter.showItem(with: nil) + } + completion(true, true) + } + currentItemActions.append(closeAction) + if index > 0 { + let moveToTopAction = UIAction(title: L10n.Accessibility.Pdf.currentItemMoveToStart, image: .init(systemName: "arrowshape.up.circle")) { [weak self] _ in + guard let self else { return } + move(item.kind, to: 0, for: sessionIdentifier) + completion(false, true) + } + currentItemActions.append(moveToTopAction) + } + if index < itemsCount - 1 { + let moveToBottomAction = UIAction(title: L10n.Accessibility.Pdf.currentItemMoveToEnd, image: .init(systemName: "arrowshape.down.circle")) { [weak self] _ in + guard let self else { return } + move(item.kind, to: itemsCount, for: sessionIdentifier) + completion(false, true) + } + currentItemActions.append(moveToBottomAction) + } + if itemsCount > 1 { + let closeOtherAction = UIAction(title: L10n.Accessibility.Pdf.closeOtherOpenItems, image: .init(systemName: "checkmark.circle.badge.xmark")) { [weak self] _ in + guard let self else { return } + setItems([item], for: sessionIdentifier, validate: false) + completion(false, true) + } + currentItemActions.append(closeOtherAction) + } + let currentItemMenu = UIMenu(title: L10n.Accessibility.Pdf.currentItem, options: [.displayInline], children: currentItemActions) + let currentItemElement = UIMenu(title: rItem.displayTitle, image: item.kind.icon, children: [currentItemMenu]) + elements.append(currentItemElement) + } else { + let itemAction = UIAction(title: rItem.displayTitle, image: item.kind.icon) { [weak self] _ in + guard let self, let presenter = openItemPresenterProvider() else { return } + restore(item, using: presenter) + completion(true, false) + } + elements.append(itemAction) + } + } + + let closeAllAction = UIAction(title: L10n.Accessibility.Pdf.closeAllOpenItems, image: .init(systemName: "xmark.square")) { [weak self] _ in + guard let self else { return } + setItems([], for: sessionIdentifier, validate: false) + openItemPresenterProvider()?.showItem(with: nil) + completion(true, true) + } + let closeAllElement = UIMenu(options: [.displayInline], children: [closeAllAction]) + elements.append(closeAllElement) + + elementProvider(elements) + } + } + + // MARK: Helper Methods + private func itemsSortedByUserOrder(for sessionIdentifier: String) -> [Item] { + getItems(for: sessionIdentifier).sorted(by: { $0.userIndex < $1.userIndex }) + } + + private func itemsSortedByLastOpen(for sessionIdentifier: String) -> [Item] { + getItems(for: sessionIdentifier).sorted(by: { $0.lastOpened > $1.lastOpened }) + } + + private func filterValidItemsWithRItem(_ items: [Item]) -> [(Item, RItem)] { + var itemTuples: [(Item, RItem)] = [] + do { + try dbStorage.perform(on: .main) { coordinator in + for item in items { + switch item.kind { + case .pdf(let libraryId, let key), .note(let libraryId, let key): + do { + let rItem = try coordinator.perform(request: ReadItemDbRequest(libraryId: libraryId, key: key)) + itemTuples.append((item, rItem)) + } catch let itemError { + DDLogError("OpenItemsController: can't load item \(item) - \(itemError)") + } + } + } + } + } catch let error { + DDLogError("OpenItemsController: can't load multiple items - \(error)") + } + return itemTuples + } + + private func filterValidItems(_ items: [Item]) -> [Item] { + filterValidItemsWithRItem(items).map { $0.0 } + } + + private func loadPresentation(for item: Item) -> Presentation? { + var presentation: Presentation? + do { + try dbStorage.perform(on: .main) { coordinator in + switch item.kind { + case .pdf(let libraryId, let key): + presentation = try loadPDFPresentation(key: key, libraryId: libraryId, coordinator: coordinator) + + case .note(let libraryId, let key): + presentation = try loadNotePresentation(key: key, libraryId: libraryId, coordinator: coordinator) + } + } + } catch let error { + DDLogError("OpenItemsController: can't load item \(item) - \(error)") + } + return presentation + + func loadPDFPresentation(key: String, libraryId: LibraryIdentifier, coordinator: DbCoordinator) throws -> Presentation? { + let library: Library = try coordinator.perform(request: ReadLibraryDbRequest(libraryId: libraryId)) + let rItem = try coordinator.perform(request: ReadItemDbRequest(libraryId: libraryId, key: key)) + let parentKey = rItem.parent?.key + guard let attachment = AttachmentCreator.attachment(for: rItem, fileStorage: fileStorage, urlDetector: nil) else { return nil } + var url: URL? + switch attachment.type { + case .file(let filename, let contentType, let location, _, _): + switch location { + case .local, .localAndChangedRemotely: + let file = Files.attachmentFile(in: libraryId, key: key, filename: filename, contentType: contentType) + url = file.createUrl() + + case .remote, .remoteMissing: + break + } + + case .url: + break + } + guard let url else { return nil } + return .pdf(library: library, key: key, parentKey: parentKey, url: url) + } + + func loadNotePresentation(key: String, libraryId: LibraryIdentifier, coordinator: DbCoordinator) throws -> Presentation? { + let library = try coordinator.perform(request: ReadLibraryDbRequest(libraryId: libraryId)) + let rItem = try coordinator.perform(request: ReadItemDbRequest(libraryId: libraryId, key: key)) + let note = Note(item: rItem) + let parentTitleData: NoteEditorState.TitleData? = rItem.parent.flatMap { .init(type: $0.rawType, title: $0.displayTitle) } + guard let note else { return nil } + return .note(library: library, key: note.key, text: note.text, tags: note.tags, parentTitleData: parentTitleData, title: note.title) + } + } + + private func presentItem(with presentation: Presentation, using presenter: OpenItemsPresenter) { + presenter.showItem(with: presentation) + DDLogInfo("OpenItemsController: presented item with presentation \(presentation)") + } +} + +extension OpenItemsController { + func openItemsUserActivity(for sessionIdentifier: String?, libraryId: LibraryIdentifier, collectionId: CollectionIdentifier? = nil) -> NSUserActivity { + var items: [Item] = [] + if let sessionIdentifier { + items = getItems(for: sessionIdentifier) + } + return items.isEmpty ? .mainActivity(with: []) : .contentActivity(with: items, libraryId: libraryId, collectionId: collectionId ?? Defaults.shared.selectedCollectionId) + } + + func setOpenItemsUserActivity(from viewController: UIViewController, libraryId: LibraryIdentifier, collectionId: CollectionIdentifier? = nil, title: String? = nil) { + let activity = openItemsUserActivity(for: viewController.sessionIdentifier, libraryId: libraryId, collectionId: collectionId).set(title: title) + viewController.set(userActivity: activity) + } +} + +extension UIImage { + static func openItemsImage(count: Int) -> UIImage? { + let count = max(0, count) + return count <= 50 ? UIImage(systemName: "\(count).square") : UIImage(systemName: "square.grid.3x3.square") + } +} + +extension UIBarButtonItem { + static func openItemsBarButtonItem() -> UIBarButtonItem { + let barButtonItem = UIBarButtonItem(image: .openItemsImage(count: 0), style: .plain, target: nil, action: nil) + barButtonItem.isEnabled = true + barButtonItem.accessibilityLabel = L10n.Accessibility.Pdf.openItems + barButtonItem.title = L10n.Accessibility.Pdf.openItems + return barButtonItem + } } diff --git a/Zotero/Extensions/Localizable.swift b/Zotero/Extensions/Localizable.swift index fc14c0643..8ef0d44cf 100644 --- a/Zotero/Extensions/Localizable.swift +++ b/Zotero/Extensions/Localizable.swift @@ -204,10 +204,22 @@ internal enum L10n { internal static let annotationHint = L10n.tr("Localizable", "accessibility.pdf.annotation_hint", fallback: "Double tap to select and edit") /// Author internal static let author = L10n.tr("Localizable", "accessibility.pdf.author", fallback: "Author") + /// Close all + internal static let closeAllOpenItems = L10n.tr("Localizable", "accessibility.pdf.close_all_open_items", fallback: "Close all") + /// Close other items + internal static let closeOtherOpenItems = L10n.tr("Localizable", "accessibility.pdf.close_other_open_items", fallback: "Close other items") /// Color picker internal static let colorPicker = L10n.tr("Localizable", "accessibility.pdf.color_picker", fallback: "Color picker") /// Comment internal static let comment = L10n.tr("Localizable", "accessibility.pdf.comment", fallback: "Comment") + /// Current Item + internal static let currentItem = L10n.tr("Localizable", "accessibility.pdf.current_item", fallback: "Current Item") + /// Close + internal static let currentItemClose = L10n.tr("Localizable", "accessibility.pdf.current_item_close", fallback: "Close") + /// Move to end + internal static let currentItemMoveToEnd = L10n.tr("Localizable", "accessibility.pdf.current_item_move_to end", fallback: "Move to end") + /// Move to start + internal static let currentItemMoveToStart = L10n.tr("Localizable", "accessibility.pdf.current_item_move_to_start", fallback: "Move to start") /// Edit annotation internal static let editAnnotation = L10n.tr("Localizable", "accessibility.pdf.edit_annotation", fallback: "Edit annotation") /// Eraser @@ -236,6 +248,8 @@ internal enum L10n { internal static let noteAnnotation = L10n.tr("Localizable", "accessibility.pdf.note_annotation", fallback: "Note annotation") /// Create note annotation internal static let noteAnnotationTool = L10n.tr("Localizable", "accessibility.pdf.note_annotation_tool", fallback: "Create note annotation") + /// Open Items + internal static let openItems = L10n.tr("Localizable", "accessibility.pdf.open_items", fallback: "Open Items") /// Open text reader internal static let openReader = L10n.tr("Localizable", "accessibility.pdf.open_reader", fallback: "Open text reader") /// Redo @@ -839,6 +853,8 @@ internal enum L10n { } /// Remove from Collection internal static let removeFromCollectionTitle = L10n.tr("Localizable", "items.remove_from_collection_title", fallback: "Remove from Collection") + /// Restore Open Items + internal static let restoreOpen = L10n.tr("Localizable", "items.restore_open", fallback: "Restore Open Items") /// Search Items internal static let searchTitle = L10n.tr("Localizable", "items.search_title", fallback: "Search Items") /// Select All diff --git a/Zotero/Extensions/NSUserActivity+Activities.swift b/Zotero/Extensions/NSUserActivity+Activities.swift index 14ef174d9..110b5dfd9 100644 --- a/Zotero/Extensions/NSUserActivity+Activities.swift +++ b/Zotero/Extensions/NSUserActivity+Activities.swift @@ -14,8 +14,8 @@ struct RestoredStateData { let openItems: [OpenItem] let restoreMostRecentlyOpenedItem: Bool - static func myLibrary() -> Self { - .init(libraryId: .custom(.myLibrary), collectionId: .custom(.all), openItems: [], restoreMostRecentlyOpenedItem: false) + static func myLibrary(openItems: [OpenItem] = []) -> Self { + .init(libraryId: .custom(.myLibrary), collectionId: .custom(.all), openItems: openItems, restoreMostRecentlyOpenedItem: false) } } @@ -29,9 +29,9 @@ extension NSUserActivity { private static let openItemsKey = "openItems" private static let restoreMostRecentlyOpenedItemKey = "restoreMostRecentlyOpenedItem" - static func mainActivity() -> NSUserActivity { + static func mainActivity(with openItems: [OpenItem]) -> NSUserActivity { return NSUserActivity(activityType: self.mainId) - .addUserInfoEntries(openItems: []) + .addUserInfoEntries(openItems: openItems) .addUserInfoEntries(restoreMostRecentlyOpened: false) } diff --git a/Zotero/Extensions/UIViewController+Extensions.swift b/Zotero/Extensions/UIViewController+Extensions.swift index 29cf268f9..7ba73a8cb 100644 --- a/Zotero/Extensions/UIViewController+Extensions.swift +++ b/Zotero/Extensions/UIViewController+Extensions.swift @@ -69,4 +69,8 @@ extension UIViewController { // Parent also didn't return a scene. Trying presenting view controller. return presentingViewController?.scene } + + var sessionIdentifier: String? { + scene?.session.persistentIdentifier + } } diff --git a/Zotero/Models/Predicates.swift b/Zotero/Models/Predicates.swift index f90f7c737..f4c5ab032 100644 --- a/Zotero/Models/Predicates.swift +++ b/Zotero/Models/Predicates.swift @@ -43,6 +43,10 @@ extension NSPredicate { .library(with: libraryId)]) } + static func keysByLibraryIdentifier(_ keysByLibraryIdentifier: [LibraryIdentifier: Set]) -> NSPredicate { + NSCompoundPredicate(orPredicateWithSubpredicates: keysByLibraryIdentifier.map({ .keys($0.value, in: $0.key) })) + } + static func key(notIn keys: [String], in libraryId: LibraryIdentifier) -> NSPredicate { return NSCompoundPredicate(andPredicateWithSubpredicates: [.library(with: libraryId), .key(notIn: keys)]) } diff --git a/Zotero/SceneDelegate.swift b/Zotero/SceneDelegate.swift index 4c9dc9116..aa9c40b2b 100644 --- a/Zotero/SceneDelegate.swift +++ b/Zotero/SceneDelegate.swift @@ -83,7 +83,8 @@ final class SceneDelegate: UIResponder, UIWindowSceneDelegate { func windowScene(_ windowScene: UIWindowScene, performActionFor shortcutItem: UIApplicationShortcutItem, completionHandler: @escaping (Bool) -> Void) { if shortcutItem.type == NSUserActivity.mainId { - completionHandler(coordinator.showMainScreen(with: .myLibrary(), session: windowScene.session)) + let openItems: [OpenItem] = windowScene.userActivity?.restoredStateData?.openItems ?? [] + completionHandler(coordinator.showMainScreen(with: .myLibrary(openItems: openItems), session: windowScene.session)) } completionHandler(false) } @@ -117,4 +118,8 @@ final class SceneDelegate: UIResponder, UIWindowSceneDelegate { func stateRestorationActivity(for scene: UIScene) -> NSUserActivity? { return scene.userActivity } + + func scene(_ scene: UIScene, continue userActivity: NSUserActivity) { + coordinator.continueUserActivity(userActivity, for: scene.session.persistentIdentifier) + } } diff --git a/Zotero/Scenes/AppCoordinator.swift b/Zotero/Scenes/AppCoordinator.swift index 93275a318..137dc0550 100644 --- a/Zotero/Scenes/AppCoordinator.swift +++ b/Zotero/Scenes/AppCoordinator.swift @@ -19,6 +19,7 @@ protocol AppDelegateCoordinatorDelegate: AnyObject { func didRotate(to size: CGSize) func show(customUrl: CustomURLController.Kind, animated: Bool) func showMainScreen(with data: RestoredStateData, session: UISceneSession) -> Bool + func continueUserActivity(_ userActivity: NSUserActivity, for sessionIdentifier: String) } protocol AppOnboardingCoordinatorDelegate: AnyObject { @@ -138,7 +139,7 @@ extension AppCoordinator: AppDelegateCoordinatorDelegate { DDLogInfo("AppCoordinator: show main screen logged \(isLoggedIn ? "in" : "out"); animated=\(animated)") show(viewController: viewController, in: window, animated: animated) { - process(urlContext: urlContext, data: data) + process(urlContext: urlContext, data: data, sessionIdentifier: session.persistentIdentifier) } func show(viewController: UIViewController?, in window: UIWindow, animated: Bool = false, completion: @escaping () -> Void) { @@ -157,8 +158,9 @@ extension AppCoordinator: AppDelegateCoordinatorDelegate { var userActivity: NSUserActivity? var data: RestoredStateData? if connectionOptions.shortcutItem?.type == NSUserActivity.mainId { - userActivity = .mainActivity() - data = .myLibrary() + let openItems: [OpenItem] = session.stateRestorationActivity?.restoredStateData?.openItems ?? [] + userActivity = .mainActivity(with: openItems) + data = .myLibrary(openItems: openItems) } else { userActivity = connectionOptions.userActivities.first ?? session.stateRestorationActivity data = userActivity?.restoredStateData @@ -168,11 +170,12 @@ extension AppCoordinator: AppDelegateCoordinatorDelegate { DDLogInfo("AppCoordinator: Preprocessing restored state - \(data)") Defaults.shared.selectedLibrary = data.libraryId Defaults.shared.selectedCollectionId = data.collectionId + controllers.userControllers?.openItemsController.setItems(data.openItems, for: session.persistentIdentifier, validate: true) } return (urlContext, data) } - func process(urlContext: UIOpenURLContext?, data: RestoredStateData?) { + func process(urlContext: UIOpenURLContext?, data: RestoredStateData?, sessionIdentifier: String) { if let urlContext, let urlController = controllers.userControllers?.customUrlController { // If scene was started from custom URL let sourceApp = urlContext.options.sourceApplication ?? "unknown" @@ -187,10 +190,11 @@ extension AppCoordinator: AppDelegateCoordinatorDelegate { if let data { DDLogInfo("AppCoordinator: Processing restored state - \(data)") // If scene had state stored, restore state - showRestoredState(for: data) + showRestoredState(for: data, sessionIdentifier: sessionIdentifier) } - func showRestoredState(for data: RestoredStateData) { + func showRestoredState(for data: RestoredStateData, sessionIdentifier: String) { + guard let openItemsController = controllers.userControllers?.openItemsController else { return } DDLogInfo("AppCoordinator: show restored state") guard let mainController = window.rootViewController as? MainViewController else { DDLogWarn("AppCoordinator: show restored state aborted - invalid root view controller") @@ -207,8 +211,8 @@ extension AppCoordinator: AppDelegateCoordinatorDelegate { collection = Collection(custom: .all) } mainController.showItems(for: collection, in: data.libraryId) - guard data.restoreMostRecentlyOpenedItem, let item = data.openItems.first else { return } - restoreMostRecentlyOpenedItem(using: self, item: item) + guard data.restoreMostRecentlyOpenedItem else { return } + openItemsController.restoreMostRecentlyOpenedItem(using: self, sessionIdentifier: sessionIdentifier) func loadRestoredStateData(libraryId: LibraryIdentifier, collectionId: CollectionIdentifier) -> Collection? { guard let dbStorage = controllers.userControllers?.dbStorage else { return nil } @@ -224,63 +228,6 @@ extension AppCoordinator: AppDelegateCoordinatorDelegate { return collection } - - func restoreMostRecentlyOpenedItem(using presenter: OpenItemsPresenter, item: OpenItem) { - guard let presentation = loadPresentation(for: item) else { return } - presenter.showItem(with: presentation) - - func loadPresentation(for item: OpenItem) -> ItemPresentation? { - guard let dbStorage = controllers.userControllers?.dbStorage else { return nil } - var presentation: ItemPresentation? - do { - try dbStorage.perform(on: .main) { coordinator in - switch item.kind { - case .pdf(let libraryId, let key): - presentation = try loadPDFPresentation(key: key, libraryId: libraryId, coordinator: coordinator) - - case .note(let libraryId, let key): - presentation = try loadNotePresentation(key: key, libraryId: libraryId, coordinator: coordinator) - } - } - } catch let error { - DDLogError("OpenItemsController: can't load item \(item) - \(error)") - } - return presentation - - func loadPDFPresentation(key: String, libraryId: LibraryIdentifier, coordinator: DbCoordinator) throws -> ItemPresentation? { - let library: Library = try coordinator.perform(request: ReadLibraryDbRequest(libraryId: libraryId)) - let rItem = try coordinator.perform(request: ReadItemDbRequest(libraryId: libraryId, key: key)) - let parentKey = rItem.parent?.key - guard let attachment = AttachmentCreator.attachment(for: rItem, fileStorage: controllers.fileStorage, urlDetector: nil) else { return nil } - var url: URL? - switch attachment.type { - case .file(let filename, let contentType, let location, _, _): - switch location { - case .local, .localAndChangedRemotely: - let file = Files.attachmentFile(in: libraryId, key: key, filename: filename, contentType: contentType) - url = file.createUrl() - - case .remote, .remoteMissing: - break - } - - case .url: - break - } - guard let url else { return nil } - return .pdf(library: library, key: key, parentKey: parentKey, url: url) - } - - func loadNotePresentation(key: String, libraryId: LibraryIdentifier, coordinator: DbCoordinator) throws -> ItemPresentation? { - let library = try coordinator.perform(request: ReadLibraryDbRequest(libraryId: libraryId)) - let rItem = try coordinator.perform(request: ReadItemDbRequest(libraryId: libraryId, key: key)) - let note = Note(item: rItem) - let parentTitleData: NoteEditorState.TitleData? = rItem.parent.flatMap { .init(type: $0.rawType, title: $0.displayTitle) } - guard let note else { return nil } - return .note(library: library, key: note.key, text: note.text, tags: note.tags, parentTitleData: parentTitleData, title: note.title) - } - } - } } } } @@ -398,11 +345,19 @@ extension AppCoordinator: AppDelegateCoordinatorDelegate { func showMainScreen(with data: RestoredStateData, session: UISceneSession) -> Bool { guard let window, let mainController = window.rootViewController as? MainViewController else { return false } + controllers.userControllers?.openItemsController.setItems(data.openItems, for: session.persistentIdentifier, validate: true) mainController.dismiss(animated: false) { mainController.masterCoordinator?.showCollections(for: data.libraryId, preselectedCollection: data.collectionId, animated: false) } return true } + + func continueUserActivity(_ userActivity: NSUserActivity, for sessionIdentifier: String) { + guard userActivity.activityType == NSUserActivity.contentContainerId, let window, let mainController = window.rootViewController as? MainViewController else { return } + mainController.getDetailCoordinator { [weak self] coordinator in + self?.controllers.userControllers?.openItemsController.restoreMostRecentlyOpenedItem(using: coordinator, sessionIdentifier: sessionIdentifier) + } + } } extension AppCoordinator: MFMailComposeViewControllerDelegate { diff --git a/Zotero/Scenes/Detail/DetailCoordinator.swift b/Zotero/Scenes/Detail/DetailCoordinator.swift index b82a8068e..62109d61f 100644 --- a/Zotero/Scenes/Detail/DetailCoordinator.swift +++ b/Zotero/Scenes/Detail/DetailCoordinator.swift @@ -86,10 +86,12 @@ final class DetailCoordinator: Coordinator { private var transitionDelegate: EmptyTransitioningDelegate? weak var itemsTagFilterDelegate: ItemsTagFilterDelegate? weak var navigationController: UINavigationController? + var presentedRestoredControllerWindow: UIWindow? let collection: Collection let libraryId: LibraryIdentifier let searchItemKeys: [String]? + let sessionIdentifier: String private unowned let controllers: Controllers private let disposeBag: DisposeBag @@ -99,6 +101,7 @@ final class DetailCoordinator: Coordinator { searchItemKeys: [String]?, navigationController: UINavigationController, itemsTagFilterDelegate: ItemsTagFilterDelegate?, + sessionIdentifier: String, controllers: Controllers ) { self.libraryId = libraryId @@ -106,6 +109,7 @@ final class DetailCoordinator: Coordinator { self.searchItemKeys = searchItemKeys self.navigationController = navigationController self.itemsTagFilterDelegate = itemsTagFilterDelegate + self.sessionIdentifier = sessionIdentifier self.controllers = controllers self.childCoordinators = [] self.disposeBag = DisposeBag() @@ -142,7 +146,14 @@ final class DetailCoordinator: Coordinator { let searchTerm = searchItemKeys?.joined(separator: " ") let sortType = Defaults.shared.itemsSortType let downloadBatchData = ItemsState.DownloadBatchData(batchData: userControllers.fileDownloader.batchData) - let state = TrashState(libraryId: libraryId, sortType: sortType, searchTerm: searchTerm, filters: [], downloadBatchData: downloadBatchData) + let state = TrashState( + libraryId: libraryId, + sortType: sortType, + searchTerm: searchTerm, + filters: [], + downloadBatchData: downloadBatchData, + openItemsCount: userControllers.openItemsController.getItems(for: sessionIdentifier).count + ) let handler = TrashActionHandler( dbStorage: userControllers.dbStorage, schemaController: controllers.schemaController, @@ -152,7 +163,7 @@ final class DetailCoordinator: Coordinator { htmlAttributedStringConverter: controllers.htmlAttributedStringConverter, fileCleanupController: userControllers.fileCleanupController ) - let controller = TrashViewController(viewModel: ViewModel(initialState: state, handler: handler), controllers: controllers, coordinatorDelegate: self) + let controller = TrashViewController(viewModel: ViewModel(initialState: state, handler: handler), controllers: controllers, coordinatorDelegate: self, presenter: self) controller.tagFilterDelegate = itemsTagFilterDelegate itemsTagFilterDelegate?.delegate = controller return controller @@ -181,7 +192,8 @@ final class DetailCoordinator: Coordinator { downloadBatchData: downloadBatchData, remoteDownloadBatchData: remoteDownloadBatchData, identifierLookupBatchData: identifierLookupBatchData, - error: nil + error: nil, + openItemsCount: userControllers.openItemsController.getItems(for: sessionIdentifier).count ) let handler = ItemsActionHandler( dbStorage: userControllers.dbStorage, @@ -194,7 +206,7 @@ final class DetailCoordinator: Coordinator { syncScheduler: userControllers.syncScheduler, htmlAttributedStringConverter: controllers.htmlAttributedStringConverter ) - let controller = ItemsViewController(viewModel: ViewModel(initialState: state, handler: handler), controllers: controllers, coordinatorDelegate: self) + let controller = ItemsViewController(viewModel: ViewModel(initialState: state, handler: handler), controllers: controllers, coordinatorDelegate: self, presenter: self) controller.tagFilterDelegate = itemsTagFilterDelegate itemsTagFilterDelegate?.delegate = controller return controller @@ -306,6 +318,19 @@ final class DetailCoordinator: Coordinator { navigationController.present(controller, animated: true, completion: nil) } + private func showDetail(presentedBy presenter: UIViewController, detailControllerProvider: () -> DetailNavigationViewController) { + if let presentedViewController = presenter.presentedViewController { + if let presentedDetailNavigationController = presentedViewController as? DetailNavigationViewController { + presentedDetailNavigationController.replaceContents(with: detailControllerProvider(), animated: false) + return + } + guard let window = presentedViewController.view.window else { return } + show(viewControllerProvider: detailControllerProvider, by: presenter, in: window, animated: false) + return + } + presenter.present(detailControllerProvider(), animated: true) + } + func createPDFController( key: String, parentKey: String?, @@ -314,8 +339,8 @@ final class DetailCoordinator: Coordinator { page: Int? = nil, preselectedAnnotationKey: String? = nil, previewRects: [CGRect]? = nil - ) -> NavigationViewController { - let navigationController = NavigationViewController() + ) -> DetailNavigationViewController { + let navigationController = DetailNavigationViewController() navigationController.modalPresentationStyle = .fullScreen let coordinator = PDFCoordinator( @@ -327,18 +352,47 @@ final class DetailCoordinator: Coordinator { preselectedAnnotationKey: preselectedAnnotationKey, previewRects: previewRects, navigationController: navigationController, + sessionIdentifier: sessionIdentifier, controllers: controllers ) + navigationController.coordinator = coordinator coordinator.parentCoordinator = self childCoordinators.append(coordinator) coordinator.start(animated: false) return navigationController } - + private func showPDF(at url: URL, key: String, parentKey: String?, libraryId: LibraryIdentifier) { - let controller = createPDFController(key: key, parentKey: parentKey, libraryId: libraryId, url: url) - navigationController?.present(controller, animated: true, completion: nil) + guard let navigationController, let openItemsController = controllers.userControllers?.openItemsController else { return } + let kind: OpenItem.Kind = .pdf(libraryId: libraryId, key: key) + if let existingSessionIdentifier = openItemsController.sessionIdentifier(for: kind), existingSessionIdentifier != sessionIdentifier { + show(kind, collectionId: collection.id, targetSessionIdentifier: existingSessionIdentifier, sourceSessionIdentifier: sessionIdentifier, openItemsController: openItemsController) + return + } + openItemsController.open(kind, for: sessionIdentifier) + + showDetail(presentedBy: navigationController) { + self.createPDFController(key: key, parentKey: parentKey, libraryId: libraryId, url: url) + } + } + + private func show(_ kind: OpenItem.Kind, collectionId: CollectionIdentifier, targetSessionIdentifier: String, sourceSessionIdentifier: String, openItemsController: OpenItemsController) { + let application = UIApplication.shared + guard let itemSession = application.openSessions.first(where: { $0.persistentIdentifier == targetSessionIdentifier }) else { return } + openItemsController.open(kind, for: targetSessionIdentifier) + let userActivity = openItemsController.openItemsUserActivity(for: targetSessionIdentifier, libraryId: kind.libraryId, collectionId: collectionId) + let options = UIScene.ActivationRequestOptions() + options.requestingScene = application.connectedScenes.first(where: { $0.session.persistentIdentifier == sourceSessionIdentifier }) + let errorHandler: (any Error) -> Void = { error in + DDLogError("DetailCoordinator: failed to activate scene session: \(itemSession) - \(error)") + } + if #available(iOS 17.0, *) { + let request = UISceneSessionActivationRequest(session: itemSession, userActivity: userActivity, options: options) + application.activateSceneSession(for: request, errorHandler: errorHandler) + } else { + application.requestSceneSessionActivation(itemSession, userActivity: userActivity, options: options, errorHandler: errorHandler) + } } private func showWebView(for url: URL) { @@ -395,8 +449,8 @@ final class DetailCoordinator: Coordinator { extension DetailCoordinator: DetailItemsCoordinatorDelegate { var displayTitle: String { - collection.name - } + collection.name + } func showAddActions(viewModel: ViewModel, button: UIBarButtonItem) { let controller = UIAlertController(title: nil, message: nil, preferredStyle: .actionSheet) @@ -429,7 +483,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), saveCallback: nil) + showNote(library: viewModel.state.library, kind: .standaloneCreation(collection: viewModel.state.collection)) })) if viewModel.state.library.metadataAndFilesEditable { @@ -512,8 +566,8 @@ extension DetailCoordinator: DetailItemsCoordinatorDelegate { tags: [Tag], parentTitleData: NoteEditorState.TitleData?, title: String? - ) -> NavigationViewController { - return createNoteController(library: library, kind: kind, text: text, tags: tags, parentTitleData: parentTitleData, title: title).0 + ) -> DetailNavigationViewController { + createNoteController(library: library, kind: kind, text: text, tags: tags, parentTitleData: parentTitleData, title: title).0 } private func createNoteController( @@ -523,8 +577,8 @@ extension DetailCoordinator: DetailItemsCoordinatorDelegate { tags: [Tag], parentTitleData: NoteEditorState.TitleData?, title: String? - ) -> (NavigationViewController, ViewModel) { - let navigationController = NavigationViewController() + ) -> (DetailNavigationViewController, ViewModel) { + let navigationController = DetailNavigationViewController() navigationController.modalPresentationStyle = .fullScreen navigationController.isModalInPresentation = true @@ -536,8 +590,10 @@ extension DetailCoordinator: DetailItemsCoordinatorDelegate { parentTitleData: parentTitleData, title: title, navigationController: navigationController, + sessionIdentifier: sessionIdentifier, controllers: controllers ) + navigationController.coordinator = coordinator coordinator.parentCoordinator = self childCoordinators.append(coordinator) coordinator.start(animated: false) @@ -1007,27 +1063,47 @@ extension DetailCoordinator: DetailNoteEditorCoordinatorDelegate { tags: [Tag] = [], parentTitleData: NoteEditorState.TitleData? = nil, title: String? = nil, - saveCallback: ((Note) -> Void)? + saveCallback: ((Note) -> Void)? = nil ) { guard let navigationController else { return } + var creationCallback: ((Note) -> Void)? switch kind { case .itemCreation, .standaloneCreation: DDLogInfo("DetailCoordinator: show note creation") + creationCallback = { [weak self] note in + guard let self, let openItemsController = controllers.userControllers?.openItemsController else { return } + openItemsController.open(.note(libraryId: library.identifier, key: note.key), for: sessionIdentifier) + } case .edit(let key), .readOnly(let key): DDLogInfo("DetailCoordinator: show note \(key)") + guard let openItemsController = controllers.userControllers?.openItemsController else { return } + let kind: OpenItem.Kind = .note(libraryId: library.identifier, key: key) + if let existingSessionIdentifier = openItemsController.sessionIdentifier(for: kind), existingSessionIdentifier != sessionIdentifier { + show(kind, collectionId: collection.id, targetSessionIdentifier: existingSessionIdentifier, sourceSessionIdentifier: sessionIdentifier, openItemsController: openItemsController) + return + } + openItemsController.open(kind, for: sessionIdentifier) } - 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: { state in - guard 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) + + showDetail(presentedBy: navigationController) { + let (controller, viewModel) = self.createNoteController(library: library, kind: kind, text: text, tags: tags, parentTitleData: parentTitleData, title: title) + + if saveCallback != nil || creationCallback != nil { + viewModel.stateObservable + .observe(on: MainScheduler.instance) + .subscribe(onNext: { state in + guard state.changes.contains(.saved), case .edit(let key) = state.kind else { return } + let note = Note(key: key, text: state.text, tags: state.tags) + if state.changes.contains(.kind) { + creationCallback?(note) + } + saveCallback?(note) + }) + .disposed(by: disposeBag) + } + + return controller } } } @@ -1060,3 +1136,21 @@ extension DetailCoordinator: DetailCitationCoordinatorDelegate { } extension DetailCoordinator: DetailCopyBibliographyCoordinatorDelegate { } + +extension DetailCoordinator: OpenItemsPresenter { + func showItem(with presentation: ItemPresentation?) { + switch presentation { + case .pdf(let library, let key, let parentKey, let url): + showPDF(at: url, key: key, parentKey: parentKey, libraryId: library.identifier) + + 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) + showNote(library: library, kind: kind, text: text, tags: tags, parentTitleData: parentTitleData, title: title) + + case .none: + navigationController?.dismiss(animated: true) + } + } +} + +extension DetailCoordinator: InstantPresenter { } diff --git a/Zotero/Scenes/Detail/Items/Models/ItemsAction.swift b/Zotero/Scenes/Detail/Items/Models/ItemsAction.swift index 461b20b79..0b0145ba6 100644 --- a/Zotero/Scenes/Detail/Items/Models/ItemsAction.swift +++ b/Zotero/Scenes/Detail/Items/Models/ItemsAction.swift @@ -42,4 +42,5 @@ enum ItemsAction { case openAttachment(attachment: Attachment, parentKey: String?) case attachmentOpened(String) case updateKeys(items: Results, deletions: [Int], insertions: [Int], modifications: [Int]) + case updateOpenItems(items: [OpenItem]) } diff --git a/Zotero/Scenes/Detail/Items/Models/ItemsState.swift b/Zotero/Scenes/Detail/Items/Models/ItemsState.swift index 5c45b1656..08ccf35f0 100644 --- a/Zotero/Scenes/Detail/Items/Models/ItemsState.swift +++ b/Zotero/Scenes/Detail/Items/Models/ItemsState.swift @@ -24,6 +24,7 @@ struct ItemsState: ViewModelState { static let filters = Changes(rawValue: 1 << 5) static let batchData = Changes(rawValue: 1 << 6) static let library = Changes(rawValue: 1 << 7) + static let openItems = Changes(rawValue: 1 << 8) } struct DownloadBatchData: Equatable { @@ -111,6 +112,7 @@ struct ItemsState: ViewModelState { var itemTitleFont: UIFont { return UIFont.preferredFont(for: .headline, weight: .regular) } + var openItemsCount: Int init( collection: Collection, @@ -121,7 +123,8 @@ struct ItemsState: ViewModelState { downloadBatchData: DownloadBatchData?, remoteDownloadBatchData: DownloadBatchData?, identifierLookupBatchData: IdentifierLookupBatchData, - error: ItemsError? + error: ItemsError?, + openItemsCount: Int ) { self.collection = collection self.filters = [] @@ -138,6 +141,7 @@ struct ItemsState: ViewModelState { self.identifierLookupBatchData = identifierLookupBatchData self.searchTerm = searchTerm self.itemTitles = [:] + self.openItemsCount = openItemsCount switch libraryId { case .custom: diff --git a/Zotero/Scenes/Detail/Items/ViewModels/ItemsActionHandler.swift b/Zotero/Scenes/Detail/Items/ViewModels/ItemsActionHandler.swift index 956284f62..bfdbb0efc 100644 --- a/Zotero/Scenes/Detail/Items/ViewModels/ItemsActionHandler.swift +++ b/Zotero/Scenes/Detail/Items/ViewModels/ItemsActionHandler.swift @@ -179,6 +179,14 @@ final class ItemsActionHandler: BaseItemsActionHandler, ViewModelActionHandler { self.update(viewModel: viewModel) { state in state.itemTitles = [:] } + + case .updateOpenItems(let items): + update(viewModel: viewModel) { state in + if state.openItemsCount != items.count { + state.openItemsCount = items.count + state.changes = .openItems + } + } } } diff --git a/Zotero/Scenes/Detail/Items/Views/BaseItemsViewController.swift b/Zotero/Scenes/Detail/Items/Views/BaseItemsViewController.swift index e1abfac5b..b92c5bb07 100644 --- a/Zotero/Scenes/Detail/Items/Views/BaseItemsViewController.swift +++ b/Zotero/Scenes/Detail/Items/Views/BaseItemsViewController.swift @@ -21,6 +21,7 @@ class BaseItemsViewController: UIViewController { case deselectAll case add case emptyTrash + case restoreOpenItems } private static let itemBatchingLimit = 150 @@ -33,10 +34,12 @@ class BaseItemsViewController: UIViewController { var handler: ItemsTableViewHandler? weak var tagFilterDelegate: ItemsTagFilterDelegate? weak var coordinatorDelegate: (DetailItemsCoordinatorDelegate & DetailNoteEditorCoordinatorDelegate)? + weak var presenter: OpenItemsPresenter? - init(controllers: Controllers, coordinatorDelegate: (DetailItemsCoordinatorDelegate & DetailNoteEditorCoordinatorDelegate)) { + init(controllers: Controllers, coordinatorDelegate: (DetailItemsCoordinatorDelegate & DetailNoteEditorCoordinatorDelegate), presenter: OpenItemsPresenter) { self.controllers = controllers self.coordinatorDelegate = coordinatorDelegate + self.presenter = presenter self.disposeBag = DisposeBag() super.init(nibName: nil, bundle: nil) } @@ -190,7 +193,16 @@ class BaseItemsViewController: UIViewController { func tagSelectionDidChange(selected: Set) {} - func process(barButtonItemAction: RightBarButtonItem, sender: UIBarButtonItem) {} + func process(barButtonItemAction: RightBarButtonItem, sender: UIBarButtonItem) { + switch barButtonItemAction { + case .select, .done, .selectAll, .deselectAll, .add, .emptyTrash: + break + + case .restoreOpenItems: + guard let presenter, let controller = controllers.userControllers?.openItemsController, let sessionIdentifier else { return } + controller.restoreMostRecentlyOpenedItem(using: presenter, sessionIdentifier: sessionIdentifier) + } + } func downloadsFilterDidChange(enabled: Bool) {} @@ -226,6 +238,7 @@ class BaseItemsViewController: UIViewController { var image: UIImage? var title: String? let accessibilityLabel: String + var menu: UIMenu? switch type { case .deselectAll: @@ -252,13 +265,34 @@ class BaseItemsViewController: UIViewController { case .emptyTrash: title = L10n.Collections.emptyTrash accessibilityLabel = L10n.Collections.emptyTrash + + case .restoreOpenItems: + image = .openItemsImage(count: 0) + accessibilityLabel = L10n.Items.restoreOpen + if let controller = controllers.userControllers?.openItemsController, let sessionIdentifier { + let deferredOpenItemsMenuElement = controller.deferredOpenItemsMenuElement( + for: sessionIdentifier, + showMenuForCurrentItem: false, + openItemPresenterProvider: { [weak self] in + self?.presenter + }, + completion: { [weak self] _, openItemsChanged in + guard let self, openItemsChanged else { return } + set(userActivity: .mainActivity(with: controllers.userControllers?.openItemsController.getItems(for: sessionIdentifier) ?? []) + .set(title: coordinatorDelegate?.displayTitle) + ) + } + ) + let openItemsMenu = UIMenu(title: L10n.Accessibility.Pdf.openItems, options: [.displayInline], children: [deferredOpenItemsMenuElement]) + menu = UIMenu(children: [openItemsMenu]) + } } let primaryAction = UIAction { [weak self] action in guard let self, let sender = action.sender as? UIBarButtonItem else { return } process(barButtonItemAction: type, sender: sender) } - let item = UIBarButtonItem(title: title, image: image, primaryAction: primaryAction) + let item = UIBarButtonItem(title: title, image: image, primaryAction: primaryAction, menu: menu) item.tag = type.rawValue item.accessibilityLabel = accessibilityLabel return item diff --git a/Zotero/Scenes/Detail/Items/Views/ItemsViewController.swift b/Zotero/Scenes/Detail/Items/Views/ItemsViewController.swift index 9063204ae..4c2fb6bcd 100644 --- a/Zotero/Scenes/Detail/Items/Views/ItemsViewController.swift +++ b/Zotero/Scenes/Detail/Items/Views/ItemsViewController.swift @@ -31,9 +31,14 @@ final class ItemsViewController: BaseItemsViewController { return toolbarData(from: viewModel.state) } - init(viewModel: ViewModel, controllers: Controllers, coordinatorDelegate: (DetailItemsCoordinatorDelegate & DetailNoteEditorCoordinatorDelegate)) { + init( + viewModel: ViewModel, + controllers: Controllers, + coordinatorDelegate: (DetailItemsCoordinatorDelegate & DetailNoteEditorCoordinatorDelegate), + presenter: OpenItemsPresenter + ) { self.viewModel = viewModel - super.init(controllers: controllers, coordinatorDelegate: coordinatorDelegate) + super.init(controllers: controllers, coordinatorDelegate: coordinatorDelegate, presenter: presenter) viewModel.process(action: .loadInitialState) } @@ -50,6 +55,7 @@ final class ItemsViewController: BaseItemsViewController { setupRightBarButtonItems(expectedItems: rightBarButtonItemTypes(for: viewModel.state)) setupFileObservers() setupAppStateObserver() + setupOpenItemsObserving() if let term = viewModel.state.searchTerm, !term.isEmpty { navigationItem.searchController?.searchBar.text = term @@ -65,6 +71,16 @@ final class ItemsViewController: BaseItemsViewController { self?.update(state: state) }) .disposed(by: disposeBag) + + func setupOpenItemsObserving() { + guard let controller = controllers.userControllers?.openItemsController, let sessionIdentifier else { return } + controller.observable(for: sessionIdentifier) + .observe(on: MainScheduler.instance) + .subscribe(onNext: { [weak self] items in + self?.viewModel.process(action: .updateOpenItems(items: items)) + }) + .disposed(by: disposeBag) + } } deinit { @@ -105,6 +121,10 @@ final class ItemsViewController: BaseItemsViewController { if state.changes.contains(.filters) || state.changes.contains(.batchData) { toolbarController?.reloadToolbarItems(for: toolbarData(from: state)) } + + if state.changes.contains(.openItems) { + setupRightBarButtonItems(expectedItems: rightBarButtonItemTypes(for: state)) + } if let key = state.itemKeyToDuplicate { coordinatorDelegate?.showItemDetail( @@ -233,6 +253,9 @@ final class ItemsViewController: BaseItemsViewController { case .select: viewModel.process(action: .startEditing) + + case .restoreOpenItems: + super.process(barButtonItemAction: barButtonItemAction, sender: sender) } } @@ -359,9 +382,27 @@ final class ItemsViewController: BaseItemsViewController { .disposed(by: disposeBag) } + override func setupRightBarButtonItems(expectedItems: [RightBarButtonItem]) { + defer { + updateRestoreOpenItemsButton(withCount: viewModel.state.openItemsCount) + } + super.setupRightBarButtonItems(expectedItems: expectedItems) + + func updateRestoreOpenItemsButton(withCount count: Int) { + guard let item = navigationItem.rightBarButtonItems?.first(where: { button in RightBarButtonItem(rawValue: button.tag) == .restoreOpenItems }) else { return } + item.image = .openItemsImage(count: count) + } + } + private func rightBarButtonItemTypes(for state: ItemsState) -> [RightBarButtonItem] { - let selectItems = rightBarButtonSelectItemTypes(for: state) - return state.library.metadataEditable ? [.add] + selectItems : selectItems + var items = rightBarButtonSelectItemTypes(for: state) + if state.library.metadataEditable { + items = [.add] + items + } + if state.openItemsCount > 0 { + items = [.restoreOpenItems] + items + } + return items func rightBarButtonSelectItemTypes(for state: ItemsState) -> [RightBarButtonItem] { if !state.isEditing { diff --git a/Zotero/Scenes/Detail/PDF/Models/PDFReaderAction.swift b/Zotero/Scenes/Detail/PDF/Models/PDFReaderAction.swift index 33666ccc5..71f840d37 100644 --- a/Zotero/Scenes/Detail/PDF/Models/PDFReaderAction.swift +++ b/Zotero/Scenes/Detail/PDF/Models/PDFReaderAction.swift @@ -59,4 +59,5 @@ enum PDFReaderAction { case changeFilter(AnnotationsFilter?) case submitPendingPage(Int) case unlock(String) + case updateOpenItems(items: [OpenItem]) } diff --git a/Zotero/Scenes/Detail/PDF/Models/PDFReaderState.swift b/Zotero/Scenes/Detail/PDF/Models/PDFReaderState.swift index daa385d3a..51c5bedc7 100644 --- a/Zotero/Scenes/Detail/PDF/Models/PDFReaderState.swift +++ b/Zotero/Scenes/Detail/PDF/Models/PDFReaderState.swift @@ -52,6 +52,7 @@ struct PDFReaderState: ViewModelState { static let activeFontSize = Changes(rawValue: 1 << 15) static let library = Changes(rawValue: 1 << 16) static let md5 = Changes(rawValue: 1 << 17) + static let openItems = Changes(rawValue: 1 << 18) } enum Error: Swift.Error { @@ -129,6 +130,8 @@ struct PDFReaderState: ViewModelState { var previewRects: [CGRect]? var unlockSuccessful: Bool? + var openItemsCount: Int + init( url: URL, key: String, @@ -142,7 +145,8 @@ struct PDFReaderState: ViewModelState { userId: Int, username: String, displayName: String, - interfaceStyle: UIUserInterfaceStyle + interfaceStyle: UIUserInterfaceStyle, + openItemsCount: Int ) { self.key = key self.parentKey = parentKey @@ -183,6 +187,7 @@ struct PDFReaderState: ViewModelState { self.activeFontSize = CGFloat(Defaults.shared.activeFontSize) self.deletionEnabled = false self.mergingEnabled = false + self.openItemsCount = openItemsCount self.previewCache.totalCostLimit = 1024 * 1024 * 10 // Cache object limit - 10 MB diff --git a/Zotero/Scenes/Detail/PDF/PDFCoordinator.swift b/Zotero/Scenes/Detail/PDF/PDFCoordinator.swift index f8b9c53d5..f087003ef 100644 --- a/Zotero/Scenes/Detail/PDF/PDFCoordinator.swift +++ b/Zotero/Scenes/Detail/PDF/PDFCoordinator.swift @@ -75,6 +75,7 @@ final class PDFCoordinator: Coordinator { private let page: Int? private let preselectedAnnotationKey: String? private let previewRects: [CGRect]? + private let sessionIdentifier: String private unowned let controllers: Controllers private let disposeBag: DisposeBag @@ -87,6 +88,7 @@ final class PDFCoordinator: Coordinator { preselectedAnnotationKey: String?, previewRects: [CGRect]?, navigationController: NavigationViewController, + sessionIdentifier: String, controllers: Controllers ) { self.key = key @@ -97,6 +99,7 @@ final class PDFCoordinator: Coordinator { self.preselectedAnnotationKey = preselectedAnnotationKey self.previewRects = previewRects self.navigationController = navigationController + self.sessionIdentifier = sessionIdentifier self.controllers = controllers self.childCoordinators = [] self.disposeBag = DisposeBag() @@ -116,7 +119,8 @@ final class PDFCoordinator: Coordinator { guard let dbStorage = self.controllers.userControllers?.dbStorage, let userId = self.controllers.sessionController.sessionData?.userId, !username.isEmpty, - let parentNavigationController = self.parentCoordinator?.navigationController + let parentNavigationController = self.parentCoordinator?.navigationController, + let openItemsController = controllers.userControllers?.openItemsController else { return } let settings = Defaults.shared.pdfSettings @@ -150,11 +154,13 @@ final class PDFCoordinator: Coordinator { userId: userId, username: username, displayName: displayName, - interfaceStyle: settings.appearanceMode == .automatic ? parentNavigationController.view.traitCollection.userInterfaceStyle : settings.appearanceMode.userInterfaceStyle + interfaceStyle: settings.appearanceMode == .automatic ? parentNavigationController.view.traitCollection.userInterfaceStyle : settings.appearanceMode.userInterfaceStyle, + openItemsCount: openItemsController.getItems(for: sessionIdentifier).count ) let controller = PDFReaderViewController( viewModel: ViewModel(initialState: state, handler: handler), - compactSize: UIDevice.current.isCompactWidth(size: parentNavigationController.view.frame.size) + compactSize: UIDevice.current.isCompactWidth(size: parentNavigationController.view.frame.size), + openItemsController: openItemsController ) controller.coordinatorDelegate = self handler.delegate = controller @@ -666,3 +672,9 @@ extension PDFCoordinator: DetailCitationCoordinatorDelegate { } extension PDFCoordinator: DetailCopyBibliographyCoordinatorDelegate { } + +extension PDFCoordinator: OpenItemsPresenter { + func showItem(with presentation: ItemPresentation?) { + (parentCoordinator as? OpenItemsPresenter)?.showItem(with: presentation) + } +} diff --git a/Zotero/Scenes/Detail/PDF/ViewModels/PDFReaderActionHandler.swift b/Zotero/Scenes/Detail/PDF/ViewModels/PDFReaderActionHandler.swift index ff3a6e72a..a81d7dc0d 100644 --- a/Zotero/Scenes/Detail/PDF/ViewModels/PDFReaderActionHandler.swift +++ b/Zotero/Scenes/Detail/PDF/ViewModels/PDFReaderActionHandler.swift @@ -266,6 +266,13 @@ final class PDFReaderActionHandler: ViewModelActionHandler, BackgroundDbProcessi update(viewModel: viewModel) { state in state.unlockSuccessful = result } + + case .updateOpenItems(let items): + guard viewModel.state.openItemsCount != items.count else { return } + update(viewModel: viewModel) { state in + state.openItemsCount = items.count + state.changes = .openItems + } } } diff --git a/Zotero/Scenes/Detail/PDF/Views/PDFReaderViewController.swift b/Zotero/Scenes/Detail/PDF/Views/PDFReaderViewController.swift index 82e87feb8..856ee358b 100644 --- a/Zotero/Scenes/Detail/PDF/Views/PDFReaderViewController.swift +++ b/Zotero/Scenes/Detail/PDF/Views/PDFReaderViewController.swift @@ -60,7 +60,8 @@ class PDFReaderViewController: UIViewController { var isToolbarVisible: Bool { return toolbarState.visible } var key: String { return viewModel.state.key } - weak var coordinatorDelegate: (PdfReaderCoordinatorDelegate & PdfAnnotationsCoordinatorDelegate)? + private unowned let openItemsController: OpenItemsController + weak var coordinatorDelegate: (PdfReaderCoordinatorDelegate & PdfAnnotationsCoordinatorDelegate & OpenItemsPresenter)? private lazy var shareButton: UIBarButtonItem = { var menuChildren: [UIMenuElement] = [] @@ -108,6 +109,29 @@ class PDFReaderViewController: UIViewController { share.menu = UIMenu(children: [deferredMenu]) return share }() + private lazy var openItemsButton: UIBarButtonItem = { + let openItems = UIBarButtonItem.openItemsBarButtonItem() + if let sessionIdentifier { + let deferredOpenItemsMenuElement = openItemsController.deferredOpenItemsMenuElement( + for: sessionIdentifier, + showMenuForCurrentItem: true, + openItemPresenterProvider: { [weak self] in + self?.coordinatorDelegate + }, + completion: { [weak self] changedCurrentItem, openItemsChanged in + guard let self else { return } + if changedCurrentItem { + close(dismiss: false) + } else if openItemsChanged { + openItemsController.setOpenItemsUserActivity(from: self, libraryId: viewModel.state.library.identifier, title: viewModel.state.title) + } + } + ) + let openItemsMenu = UIMenu(title: L10n.Accessibility.Pdf.openItems, options: [.displayInline], children: [deferredOpenItemsMenuElement]) + openItems.menu = UIMenu(children: [openItemsMenu]) + } + return openItems + }() private lazy var settingsButton: UIBarButtonItem = { let settings = UIBarButtonItem(image: UIImage(systemName: "gearshape"), style: .plain, target: nil, action: nil) settings.isEnabled = !viewModel.state.document.isLocked @@ -194,9 +218,10 @@ class PDFReaderViewController: UIViewController { return false } - init(viewModel: ViewModel, compactSize: Bool) { + init(viewModel: ViewModel, compactSize: Bool, openItemsController: OpenItemsController) { self.viewModel = viewModel isCompactWidth = compactSize + self.openItemsController = openItemsController disposeBag = DisposeBag() super.init(nibName: nil, bundle: nil) } @@ -208,10 +233,7 @@ class PDFReaderViewController: UIViewController { override func viewDidLoad() { super.viewDidLoad() - let openItem = OpenItem(kind: .pdf(libraryId: viewModel.state.library.identifier, key: viewModel.state.key), userIndex: 0) - set(userActivity: .contentActivity(with: [openItem], libraryId: viewModel.state.library.identifier, collectionId: Defaults.shared.selectedCollectionId) - .set(title: viewModel.state.title) - ) + openItemsController.setOpenItemsUserActivity(from: self, libraryId: viewModel.state.library.identifier, title: viewModel.state.title) viewModel.process(action: .changeIdleTimerDisabled(true)) view.backgroundColor = .systemGray6 // Create intraDocumentNavigationHandler before setting up views, as it may be called by a child view controller, before view has finished loading. @@ -316,7 +338,7 @@ class PDFReaderViewController: UIViewController { let closeButton = UIBarButtonItem(image: UIImage(systemName: "chevron.left"), style: .plain, target: nil, action: nil) closeButton.title = L10n.close closeButton.accessibilityLabel = L10n.close - closeButton.rx.tap.subscribe(onNext: { [weak self] _ in self?.close() }).disposed(by: disposeBag) + closeButton.rx.tap.subscribe(onNext: { [weak self] _ in self?.close(dismiss: true) }).disposed(by: disposeBag) let readerButton = UIBarButtonItem(image: Asset.Images.pdfRawReader.image, style: .plain, target: nil, action: nil) readerButton.isEnabled = !viewModel.state.document.isLocked @@ -330,7 +352,7 @@ class PDFReaderViewController: UIViewController { .disposed(by: disposeBag) navigationItem.leftBarButtonItems = [closeButton, sidebarButton, readerButton] - navigationItem.rightBarButtonItems = createRightBarButtonItems() + navigationItem.rightBarButtonItems = createRightBarButtonItems(for: viewModel.state) } func setupObserving() { @@ -341,6 +363,15 @@ class PDFReaderViewController: UIViewController { }) .disposed(by: disposeBag) + if let sessionIdentifier { + openItemsController.observable(for: sessionIdentifier) + .observe(on: MainScheduler.instance) + .subscribe(onNext: { [weak self] items in + self?.viewModel.process(action: .updateOpenItems(items: items)) + }) + .disposed(by: disposeBag) + } + NotificationCenter.default.rx .notification(UIApplication.didBecomeActiveNotification) .observe(on: MainScheduler.instance) @@ -429,7 +460,7 @@ class PDFReaderViewController: UIViewController { private func update(state: PDFReaderState) { if state.changes.contains(.md5) { coordinatorDelegate?.showDocumentChangedAlert { [weak self] in - self?.close() + self?.close(dismiss: true) } return } @@ -477,7 +508,9 @@ class PDFReaderViewController: UIViewController { } annotationToolbarHandler.set(hidden: hidden, animated: true) (toolbarButton.customView as? CheckboxButton)?.isSelected = toolbarState.visible - navigationItem.rightBarButtonItems = createRightBarButtonItems() + } + if state.changes.contains(.library) || state.changes.contains(.openItems) { + navigationItem.rightBarButtonItems = createRightBarButtonItems(for: state) } if let tool = state.changedColorForTool, documentController.pdfController?.annotationStateManager.state == tool, let color = state.toolColors[tool] { @@ -660,11 +693,12 @@ class PDFReaderViewController: UIViewController { .disposed(by: disposeBag) } - private func close() { + private func close(dismiss: Bool) { if let page = documentController?.pdfController?.pageIndex { viewModel.process(action: .submitPendingPage(Int(page))) } viewModel.process(action: .clearTmpData) + guard dismiss else { return } navigationController?.presentingViewController?.dismiss(animated: true, completion: nil) } @@ -692,10 +726,12 @@ class PDFReaderViewController: UIViewController { button.title = isSidebarVisible ? L10n.Accessibility.Pdf.sidebarClose : L10n.Accessibility.Pdf.sidebarOpen } - private func createRightBarButtonItems() -> [UIBarButtonItem] { + private func createRightBarButtonItems(for state: PDFReaderState) -> [UIBarButtonItem] { var buttons = [settingsButton, shareButton, searchButton] + buttons.insert(openItemsButton, at: 1) + openItemsButton.image = .openItemsImage(count: state.openItemsCount) - if viewModel.state.library.metadataEditable { + if state.library.metadataEditable { buttons.append(toolbarButton) } diff --git a/Zotero/Scenes/Detail/Trash/Models/TrashAction.swift b/Zotero/Scenes/Detail/Trash/Models/TrashAction.swift index 1a0aa3cb4..6e824ce1a 100644 --- a/Zotero/Scenes/Detail/Trash/Models/TrashAction.swift +++ b/Zotero/Scenes/Detail/Trash/Models/TrashAction.swift @@ -30,4 +30,5 @@ enum TrashAction { case toggleSelectionState case updateAttachments(AttachmentFileDeletedNotification) case updateDownload(update: AttachmentDownloader.Update, batchData: ItemsState.DownloadBatchData?) + case updateOpenItems(items: [OpenItem]) } diff --git a/Zotero/Scenes/Detail/Trash/Models/TrashState.swift b/Zotero/Scenes/Detail/Trash/Models/TrashState.swift index f3279ce6e..a12323da7 100644 --- a/Zotero/Scenes/Detail/Trash/Models/TrashState.swift +++ b/Zotero/Scenes/Detail/Trash/Models/TrashState.swift @@ -81,6 +81,7 @@ struct TrashState: ViewModelState { static let batchData = Changes(rawValue: 1 << 5) static let attachmentsRemoved = Changes(rawValue: 1 << 6) static let library = Changes(rawValue: 1 << 7) + static let openItems = Changes(rawValue: 1 << 8) } enum Error: Swift.Error { @@ -105,8 +106,9 @@ struct TrashState: ViewModelState { var titleFont: UIFont { return UIFont.preferredFont(for: .headline, weight: .regular) } + var openItemsCount: Int - init(libraryId: LibraryIdentifier, sortType: ItemsSortType, searchTerm: String?, filters: [ItemsFilter], downloadBatchData: ItemsState.DownloadBatchData?) { + init(libraryId: LibraryIdentifier, sortType: ItemsSortType, searchTerm: String?, filters: [ItemsFilter], downloadBatchData: ItemsState.DownloadBatchData?, openItemsCount: Int) { snapshot = .empty itemDataCache = [:] self.sortType = sortType @@ -116,6 +118,7 @@ struct TrashState: ViewModelState { changes = [] selectedItems = [] self.downloadBatchData = downloadBatchData + self.openItemsCount = openItemsCount switch libraryId { case .custom: diff --git a/Zotero/Scenes/Detail/Trash/ViewModels/TrashActionHandler.swift b/Zotero/Scenes/Detail/Trash/ViewModels/TrashActionHandler.swift index 490319907..39de5495b 100644 --- a/Zotero/Scenes/Detail/Trash/ViewModels/TrashActionHandler.swift +++ b/Zotero/Scenes/Detail/Trash/ViewModels/TrashActionHandler.swift @@ -142,6 +142,14 @@ final class TrashActionHandler: BaseItemsActionHandler, ViewModelActionHandler { case .cacheItemDataIfNeeded(let key): cacheItemData(key: key, viewModel: viewModel) + + case .updateOpenItems(let items): + update(viewModel: viewModel) { state in + if state.openItemsCount != items.count { + state.openItemsCount = items.count + state.changes = .openItems + } + } } } diff --git a/Zotero/Scenes/Detail/Trash/Views/TrashViewController.swift b/Zotero/Scenes/Detail/Trash/Views/TrashViewController.swift index f8c5f0ed7..6e60ab7aa 100644 --- a/Zotero/Scenes/Detail/Trash/Views/TrashViewController.swift +++ b/Zotero/Scenes/Detail/Trash/Views/TrashViewController.swift @@ -24,9 +24,14 @@ final class TrashViewController: BaseItemsViewController { return toolbarData(from: viewModel.state) } - init(viewModel: ViewModel, controllers: Controllers, coordinatorDelegate: (DetailItemsCoordinatorDelegate & DetailNoteEditorCoordinatorDelegate)) { + init( + viewModel: ViewModel, + controllers: Controllers, + coordinatorDelegate: (DetailItemsCoordinatorDelegate & DetailNoteEditorCoordinatorDelegate), + presenter: OpenItemsPresenter + ) { self.viewModel = viewModel - super.init(controllers: controllers, coordinatorDelegate: coordinatorDelegate) + super.init(controllers: controllers, coordinatorDelegate: coordinatorDelegate, presenter: presenter) viewModel.process(action: .loadData) } @@ -42,6 +47,7 @@ final class TrashViewController: BaseItemsViewController { toolbarController = ItemsToolbarController(viewController: self, data: toolbarData, collection: collection, library: library, delegate: self) setupRightBarButtonItems(expectedItems: rightBarButtonItemTypes(for: viewModel.state)) setupDownloadObserver() + setupOpenItemsObserving() dataSource.apply(snapshot: viewModel.state.snapshot) updateTagFilter(filters: viewModel.state.filters, collectionId: .custom(.trash), libraryId: viewModel.state.library.identifier) @@ -84,6 +90,16 @@ final class TrashViewController: BaseItemsViewController { }) .disposed(by: self.disposeBag) } + + func setupOpenItemsObserving() { + guard let controller = controllers.userControllers?.openItemsController, let sessionIdentifier else { return } + controller.observable(for: sessionIdentifier) + .observe(on: MainScheduler.instance) + .subscribe(onNext: { [weak self] items in + self?.viewModel.process(action: .updateOpenItems(items: items)) + }) + .disposed(by: disposeBag) + } } // MARK: - Actions @@ -121,6 +137,10 @@ final class TrashViewController: BaseItemsViewController { toolbarController?.reloadToolbarItems(for: toolbarData(from: state)) } + if state.changes.contains(.openItems) { + setupRightBarButtonItems(expectedItems: rightBarButtonItemTypes(for: state)) + } + if let error = state.error { process(error: error, state: state) } @@ -205,6 +225,9 @@ final class TrashViewController: BaseItemsViewController { case .select: viewModel.process(action: .startEditing) + + case .restoreOpenItems: + super.process(barButtonItemAction: barButtonItemAction, sender: sender) } } @@ -222,9 +245,24 @@ final class TrashViewController: BaseItemsViewController { ) } + override func setupRightBarButtonItems(expectedItems: [RightBarButtonItem]) { + defer { + updateRestoreOpenItemsButton(withCount: viewModel.state.openItemsCount) + } + super.setupRightBarButtonItems(expectedItems: expectedItems) + + func updateRestoreOpenItemsButton(withCount count: Int) { + guard let item = navigationItem.rightBarButtonItems?.first(where: { button in RightBarButtonItem(rawValue: button.tag) == .restoreOpenItems }) else { return } + item.image = .openItemsImage(count: count) + } + } + private func rightBarButtonItemTypes(for state: TrashState) -> [RightBarButtonItem] { - let selectItems = rightBarButtonSelectItemTypes(for: state) - return selectItems + [.emptyTrash] + var items = rightBarButtonSelectItemTypes(for: state) + [.emptyTrash] + if state.openItemsCount > 0 { + items = [.restoreOpenItems] + items + } + return items func rightBarButtonSelectItemTypes(for state: TrashState) -> [RightBarButtonItem] { if !state.isEditing { diff --git a/Zotero/Scenes/General/Models/NoteEditorAction.swift b/Zotero/Scenes/General/Models/NoteEditorAction.swift index 7858715d9..9257b0537 100644 --- a/Zotero/Scenes/General/Models/NoteEditorAction.swift +++ b/Zotero/Scenes/General/Models/NoteEditorAction.swift @@ -16,4 +16,5 @@ enum NoteEditorAction { case saveBeforeClosing case setTags([Tag]) case setText(String) + case updateOpenItems(items: [OpenItem]) } diff --git a/Zotero/Scenes/General/Models/NoteEditorState.swift b/Zotero/Scenes/General/Models/NoteEditorState.swift index 96bd18dd4..afa04ab5c 100644 --- a/Zotero/Scenes/General/Models/NoteEditorState.swift +++ b/Zotero/Scenes/General/Models/NoteEditorState.swift @@ -45,6 +45,7 @@ struct NoteEditorState: ViewModelState { static let tags = Changes(rawValue: 1 << 0) static let shouldSave = Changes(rawValue: 1 << 1) + static let openItems = Changes(rawValue: 1 << 2) static let kind = Changes(rawValue: 1 << 3) static let title = Changes(rawValue: 1 << 4) static let saved = Changes(rawValue: 1 << 5) @@ -70,16 +71,18 @@ struct NoteEditorState: ViewModelState { var downloadedResource: Resource? var createdImages: [CreatedImage] var changes: Changes + var openItemsCount: Int var title: String? var isClosing: Bool var error: Swift.Error? - init(kind: Kind, library: Library, parentTitleData: TitleData?, text: String, tags: [Tag], title: String?) { + init(kind: Kind, library: Library, parentTitleData: TitleData?, text: String, tags: [Tag], openItemsCount: Int, title: String?) { self.kind = kind self.text = text self.tags = tags self.library = library self.parentTitleData = parentTitleData + self.openItemsCount = openItemsCount self.title = title isClosing = false changes = [] diff --git a/Zotero/Scenes/General/NoteEditorCoordinator.swift b/Zotero/Scenes/General/NoteEditorCoordinator.swift index 59206cec1..193e10f78 100644 --- a/Zotero/Scenes/General/NoteEditorCoordinator.swift +++ b/Zotero/Scenes/General/NoteEditorCoordinator.swift @@ -32,6 +32,7 @@ final class NoteEditorCoordinator: NSObject, Coordinator { private let parentTitleData: NoteEditorState.TitleData? private let title: String? private let library: Library + private let sessionIdentifier: String private unowned let controllers: Controllers var viewModel: ViewModel? { @@ -46,6 +47,7 @@ final class NoteEditorCoordinator: NSObject, Coordinator { parentTitleData: NoteEditorState.TitleData?, title: String?, navigationController: NavigationViewController, + sessionIdentifier: String, controllers: Controllers ) { self.kind = kind @@ -55,6 +57,7 @@ final class NoteEditorCoordinator: NSObject, Coordinator { self.title = title self.library = library self.navigationController = navigationController + self.sessionIdentifier = sessionIdentifier self.controllers = controllers childCoordinators = [] @@ -71,21 +74,29 @@ final class NoteEditorCoordinator: NSObject, Coordinator { } func start(animated: Bool) { - guard let dbStorage = controllers.userControllers?.dbStorage, let fileDownloader = controllers.userControllers?.fileDownloader else { return } - let state = NoteEditorState(kind: kind, library: library, parentTitleData: parentTitleData, text: initialText, tags: initialTags, title: title) - let handler = NoteEditorActionHandler( - dbStorage: dbStorage, - fileStorage: controllers.fileStorage, - schemaController: controllers.schemaController, - attachmentDownloader: fileDownloader + guard let dbStorage = controllers.userControllers?.dbStorage, + let fileDownloader = controllers.userControllers?.fileDownloader, + let openItemsController = controllers.userControllers?.openItemsController + else { return } + + let state = NoteEditorState( + kind: kind, + library: library, + parentTitleData: parentTitleData, + text: initialText, + tags: initialTags, + openItemsCount: openItemsController.getItems(for: sessionIdentifier).count, + title: title ) + let handler = NoteEditorActionHandler(dbStorage: dbStorage, fileStorage: controllers.fileStorage, schemaController: controllers.schemaController, attachmentDownloader: fileDownloader) let viewModel = ViewModel(initialState: state, handler: handler) let controller = NoteEditorViewController( viewModel: viewModel, htmlAttributedStringConverter: controllers.htmlAttributedStringConverter, dbStorage: dbStorage, fileStorage: controllers.fileStorage, - uriConverter: controllers.uriConverter + uriConverter: controllers.uriConverter, + openItemsController: openItemsController ) controller.coordinatorDelegate = self navigationController?.setViewControllers([controller], animated: animated) @@ -159,3 +170,9 @@ extension NoteEditorCoordinator: NoteEditorCoordinatorDelegate { } } } + +extension NoteEditorCoordinator: OpenItemsPresenter { + func showItem(with presentation: ItemPresentation?) { + (parentCoordinator as? OpenItemsPresenter)?.showItem(with: presentation) + } +} diff --git a/Zotero/Scenes/General/ViewModels/NoteEditorActionHandler.swift b/Zotero/Scenes/General/ViewModels/NoteEditorActionHandler.swift index f3a88df7c..6d5dc9401 100644 --- a/Zotero/Scenes/General/ViewModels/NoteEditorActionHandler.swift +++ b/Zotero/Scenes/General/ViewModels/NoteEditorActionHandler.swift @@ -67,6 +67,13 @@ struct NoteEditorActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc guard let viewModel else { return } importImages(data: data, in: viewModel) } + + case .updateOpenItems(let items): + guard viewModel.state.openItemsCount != items.count else { return } + update(viewModel: viewModel) { state in + state.openItemsCount = items.count + state.changes = .openItems + } } } diff --git a/Zotero/Scenes/General/Views/NavigationViewController.swift b/Zotero/Scenes/General/Views/NavigationViewController.swift index 43aeb21d4..de164f285 100644 --- a/Zotero/Scenes/General/Views/NavigationViewController.swift +++ b/Zotero/Scenes/General/Views/NavigationViewController.swift @@ -20,3 +20,25 @@ class NavigationViewController: UINavigationController { dismissHandler?() } } + +class DetailNavigationViewController: NavigationViewController { + weak var coordinator: Coordinator? + public func replaceContents(with replacement: DetailNavigationViewController, animated: Bool) { + // Set replacement properties to self. + // Swap coordinators and dismiss handlers, so that the original coordinator is properly deinitialized, along with the original view controllers. + // Swap also the navigation controller property of the two coordinators. + // Store original + let originalCoordinator = coordinator + let originalDismissHandler = dismissHandler + // Swap replacement to original + coordinator = replacement.coordinator + coordinator?.navigationController = self + dismissHandler = replacement.dismissHandler + statusBarVisible = replacement.statusBarVisible + setViewControllers(replacement.viewControllers, animated: animated) + // Swap original to replacement + replacement.coordinator = originalCoordinator + replacement.coordinator?.navigationController = replacement + replacement.dismissHandler = originalDismissHandler + } +} diff --git a/Zotero/Scenes/General/Views/NoteEditorViewController.swift b/Zotero/Scenes/General/Views/NoteEditorViewController.swift index 2bf130fcb..ee9a48f0c 100644 --- a/Zotero/Scenes/General/Views/NoteEditorViewController.swift +++ b/Zotero/Scenes/General/Views/NoteEditorViewController.swift @@ -18,6 +18,11 @@ final class NoteEditorViewController: UIViewController { case messageHandler case logHandler } + private enum RightBarButtonItem: Int { + case done + case closing + case restoreOpenItems + } @IBOutlet private weak var webView: WKWebView! @IBOutlet private weak var webViewBottom: NSLayoutConstraint! @@ -33,20 +38,23 @@ final class NoteEditorViewController: UIViewController { private let disposeBag: DisposeBag private var debounceDisposeBag: DisposeBag? - weak var coordinatorDelegate: NoteEditorCoordinatorDelegate? + private unowned let openItemsController: OpenItemsController + weak var coordinatorDelegate: (NoteEditorCoordinatorDelegate & OpenItemsPresenter)? init( viewModel: ViewModel, htmlAttributedStringConverter: HtmlAttributedStringConverter, dbStorage: DbStorage, fileStorage: FileStorage, - uriConverter: ZoteroURIConverter + uriConverter: ZoteroURIConverter, + openItemsController: OpenItemsController ) { self.viewModel = viewModel self.htmlAttributedStringConverter = htmlAttributedStringConverter self.dbStorage = dbStorage self.fileStorage = fileStorage self.uriConverter = uriConverter + self.openItemsController = openItemsController disposeBag = DisposeBag() super.init(nibName: "NoteEditorViewController", bundle: nil) } @@ -58,24 +66,16 @@ final class NoteEditorViewController: UIViewController { override func viewDidLoad() { super.viewDidLoad() - switch viewModel.state.kind { - case .edit(let key), .readOnly(let key): - let openItem = OpenItem(kind: .note(libraryId: viewModel.state.library.identifier, key: key), userIndex: 0) - set(userActivity: .contentActivity(with: [openItem], libraryId: viewModel.state.library.identifier, collectionId: Defaults.shared.selectedCollectionId) - .set(title: viewModel.state.title) - ) - - case.itemCreation, .standaloneCreation: - break - } + openItemsController.setOpenItemsUserActivity(from: self, libraryId: viewModel.state.library.identifier, title: viewModel.state.title) if let parentTitleData = viewModel.state.parentTitleData { navigationItem.titleView = NoteEditorTitleView(type: parentTitleData.type, title: htmlAttributedStringConverter.convert(text: parentTitleData.title).string) } - setupNavbarItems(isClosing: false) + setupNavbarItems(for: viewModel.state, isClosing: false) setupKeyboard() setupWebView() + setupOpenItemsObserving() update(tags: viewModel.state.tags) viewModel.stateObservable @@ -99,6 +99,16 @@ final class NoteEditorViewController: UIViewController { webView.loadFileURL(url, allowingReadAccessTo: url.deletingLastPathComponent()) } + func setupOpenItemsObserving() { + guard let sessionIdentifier else { return } + openItemsController.observable(for: sessionIdentifier) + .observe(on: MainScheduler.instance) + .subscribe(onNext: { [weak self] items in + self?.viewModel.process(action: .updateOpenItems(items: items)) + }) + .disposed(by: disposeBag) + } + func setupKeyboard() { NotificationCenter.default .keyboardWillShow @@ -134,33 +144,73 @@ final class NoteEditorViewController: UIViewController { // MARK: - Actions - private func setupNavbarItems(isClosing: Bool) { - if isClosing { - let activityIndicator = UIActivityIndicatorView(style: .medium) - activityIndicator.color = .gray - activityIndicator.startAnimating() - navigationItem.rightBarButtonItem = UIBarButtonItem(customView: activityIndicator) - return + private func setupNavbarItems(for state: NoteEditorState, isClosing: Bool) { + defer { + updateRestoreOpenItemsButton(withCount: state.openItemsCount) + } + let currentItems = (self.navigationItem.rightBarButtonItems ?? []).compactMap({ RightBarButtonItem(rawValue: $0.tag) }) + let expectedItems = rightBarButtonItemTypes(for: state, isClosing: isClosing) + guard currentItems != expectedItems else { return } + navigationItem.rightBarButtonItems = expectedItems.map({ createRightBarButtonItem($0) }).reversed() + + func rightBarButtonItemTypes(for state: NoteEditorState, isClosing: Bool) -> [RightBarButtonItem] { + var items: [RightBarButtonItem] = [isClosing ? .closing : .done] + if state.openItemsCount > 0 { + items = [.restoreOpenItems] + items + } + return items } - let done = UIBarButtonItem(title: L10n.done, style: .done, target: nil, action: nil) - done.rx - .tap - .subscribe(onNext: { [weak self] _ in - guard let self else { return } - closeAndSaveIfNeeded(controller: self) - }) - .disposed(by: disposeBag) - navigationItem.rightBarButtonItem = done - - func closeAndSaveIfNeeded(controller: NoteEditorViewController) { - if controller.debounceDisposeBag == nil { - controller.close() - return + func createRightBarButtonItem(_ type: RightBarButtonItem) -> UIBarButtonItem { + let item: UIBarButtonItem + switch type { + case .done: + let done = UIBarButtonItem(title: L10n.done, style: .done, target: nil, action: nil) + done.rx.tap + .subscribe(onNext: { [weak self] _ in + guard let self else { return } + closeAndSaveIfNeeded() + }) + .disposed(by: disposeBag) + item = done + + case .closing: + let activityIndicator = UIActivityIndicatorView(style: .medium) + activityIndicator.color = .gray + activityIndicator.startAnimating() + item = UIBarButtonItem(customView: activityIndicator) + + case .restoreOpenItems: + let openItems = UIBarButtonItem.openItemsBarButtonItem() + if let sessionIdentifier { + let deferredOpenItemsMenuElement = openItemsController.deferredOpenItemsMenuElement( + for: sessionIdentifier, + showMenuForCurrentItem: true, + openItemPresenterProvider: { [weak self] in + self?.coordinatorDelegate + }, + completion: { [weak self] changedCurrentItem, openItemsChanged in + guard let self else { return } + if changedCurrentItem { + closeAndSaveIfNeeded() + } else if openItemsChanged { + openItemsController.setOpenItemsUserActivity(from: self, libraryId: viewModel.state.library.identifier, title: viewModel.state.title) + } + } + ) + let openItemsMenu = UIMenu(title: L10n.Accessibility.Pdf.openItems, options: [.displayInline], children: [deferredOpenItemsMenuElement]) + openItems.menu = UIMenu(children: [openItemsMenu]) + } + item = openItems } - controller.debounceDisposeBag = nil - controller.viewModel.process(action: .saveBeforeClosing) + item.tag = type.rawValue + return item + } + + func updateRestoreOpenItemsButton(withCount count: Int) { + guard let item = navigationItem.rightBarButtonItems?.first(where: { button in RightBarButtonItem(rawValue: button.tag) == .restoreOpenItems }) else { return } + item.image = .openItemsImage(count: count) } } @@ -181,21 +231,19 @@ final class NoteEditorViewController: UIViewController { if state.changes.contains(.shouldSave) { debounceSave() } + if state.changes.contains(.kind) || state.changes.contains(.title) { switch state.kind { - case .edit(let key), .readOnly(let key): - let openItem = OpenItem(kind: .note(libraryId: state.library.identifier, key: key), userIndex: 0) - set(userActivity: .contentActivity(with: [openItem], libraryId: state.library.identifier, collectionId: Defaults.shared.selectedCollectionId) - .set(title: state.title) - ) + case .edit, .readOnly: + openItemsController.setOpenItemsUserActivity(from: self, libraryId: state.library.identifier, title: state.title) case.itemCreation, .standaloneCreation: break } } - if state.changes.contains(.closing) { - setupNavbarItems(isClosing: state.isClosing) + if state.changes.contains(.openItems) || state.changes.contains(.closing) { + setupNavbarItems(for: state, isClosing: state.isClosing) } if !state.createdImages.isEmpty { @@ -340,6 +388,16 @@ final class NoteEditorViewController: UIViewController { self?.viewModel.process(action: .setTags(tags)) }) } + + private func closeAndSaveIfNeeded() { + if debounceDisposeBag == nil { + close() + return + } + + debounceDisposeBag = nil + viewModel.process(action: .saveBeforeClosing) + } } extension NoteEditorViewController: WKScriptMessageHandler { diff --git a/Zotero/Scenes/Main/Views/MainViewController.swift b/Zotero/Scenes/Main/Views/MainViewController.swift index b6f82949a..125a22790 100644 --- a/Zotero/Scenes/Main/Views/MainViewController.swift +++ b/Zotero/Scenes/Main/Views/MainViewController.swift @@ -32,7 +32,11 @@ final class MainViewController: UISplitViewController { private var detailCoordinator: DetailCoordinator? { didSet { guard let detailCoordinator else { return } - set(userActivity: .mainActivity().set(title: detailCoordinator.displayTitle)) + var openItems: [OpenItem] = [] + if let openItemsController = controllers.userControllers?.openItemsController, let sessionIdentifier { + openItems = openItemsController.getItems(for: sessionIdentifier) + } + set(userActivity: .mainActivity(with: openItems).set(title: detailCoordinator.displayTitle)) if let detailCoordinatorGetter { detailCoordinatorGetter(detailCoordinator) self.detailCoordinatorGetter = nil @@ -89,7 +93,11 @@ final class MainViewController: UISplitViewController { override func viewDidAppear(_ animated: Bool) { super.viewDidAppear(animated) guard let detailCoordinator else { return } - set(userActivity: .mainActivity().set(title: detailCoordinator.displayTitle)) + var openItems: [OpenItem] = [] + if let openItemsController = controllers.userControllers?.openItemsController, let sessionIdentifier { + openItems = openItemsController.getItems(for: sessionIdentifier) + } + set(userActivity: .mainActivity(with: openItems).set(title: detailCoordinator.displayTitle)) } func getDetailCoordinator(completed: @escaping (DetailCoordinator) -> Void) { @@ -101,6 +109,7 @@ final class MainViewController: UISplitViewController { } private func showItems(for collection: Collection, in libraryId: LibraryIdentifier, searchItemKeys: [String]?) { + guard let sessionIdentifier else { return } let navigationController = UINavigationController() let tagFilterController = (self.viewControllers.first as? MasterContainerViewController)?.bottomController as? ItemsTagFilterDelegate @@ -110,6 +119,7 @@ final class MainViewController: UISplitViewController { searchItemKeys: searchItemKeys, navigationController: navigationController, itemsTagFilterDelegate: tagFilterController, + sessionIdentifier: sessionIdentifier, controllers: self.controllers ) coordinator.start(animated: false) From 7fdc0429e445fd9e017b429a1afa7c177188a4aa Mon Sep 17 00:00:00 2001 From: Miltiadis Vasilakis Date: Tue, 27 Aug 2024 14:01:21 +0300 Subject: [PATCH 2/5] Make OpenItemsController restoration methods asynchronous --- Zotero/Controllers/OpenItemsController.swift | 182 ++++++++++-------- Zotero/Scenes/AppCoordinator.swift | 12 +- .../Items/Views/BaseItemsViewController.swift | 6 +- 3 files changed, 120 insertions(+), 80 deletions(-) diff --git a/Zotero/Controllers/OpenItemsController.swift b/Zotero/Controllers/OpenItemsController.swift index 6ed8bb6a1..87a360abc 100644 --- a/Zotero/Controllers/OpenItemsController.swift +++ b/Zotero/Controllers/OpenItemsController.swift @@ -243,41 +243,56 @@ final class OpenItemsController { setItemsSortedByUserIndex(existingItems, for: sessionIdentifier, validate: false) } - @discardableResult - func restore(_ item: Item, using presenter: OpenItemsPresenter) -> Bool { - guard let presentation = loadPresentation(for: item) else { return false } - presentItem(with: presentation, using: presenter) - return true + func restore(_ item: Item, using presenter: OpenItemsPresenter, completion: @escaping (Bool) -> Void) { + loadPresentation(for: item) { [weak presenter] presentation in + guard let presenter, let presentation else { + completion(false) + return + } + presenter.showItem(with: presentation) + DDLogInfo("OpenItemsController: presenter \(presenter) presented item with presentation \(presentation)") + completion(true) + } } - @discardableResult - func restoreMostRecentlyOpenedItem(using presenter: OpenItemsPresenter, sessionIdentifier: String) -> Item? { + func restoreMostRecentlyOpenedItem(using presenter: OpenItemsPresenter, sessionIdentifier: String, completion: @escaping (Item?) -> Void) { // Will restore most recent opened item still present, or none if all fail var existingItems = getItems(for: sessionIdentifier) DDLogInfo("OpenItemsController: restoring most recently opened item using presenter \(presenter) for \(sessionIdentifier)") - var itemsChanged: Bool = false - defer { - if itemsChanged { + let existingItemsSortedByLastOpen = itemsSortedByLastOpen(for: sessionIdentifier) + loadFirstAvailablePresentation(from: existingItemsSortedByLastOpen, indexOffset: 0) { [weak self, weak presenter] item, presentation, foundIndex in + if let self, foundIndex > 0 { + for item in existingItemsSortedByLastOpen[0..")") + } + completion(item) } - var item: Item? - var presentation: Presentation? - let existingItemsSortedByLastOpen = itemsSortedByLastOpen(for: sessionIdentifier) - for _item in existingItemsSortedByLastOpen { - if let _presentation = loadPresentation(for: _item) { - item = _item - presentation = _presentation - break + + func loadFirstAvailablePresentation(from items: [Item], indexOffset: Int, completion: @escaping (Item?, Presentation?, Int) -> Void ) { + guard !items.isEmpty else { + completion(nil, nil, indexOffset) + return + } + + var remainingItems = items + let currentItem = remainingItems.removeFirst() + + loadPresentation(for: currentItem) { presentation in + if let presentation { + completion(currentItem, presentation, indexOffset) + } else { + loadFirstAvailablePresentation(from: remainingItems, indexOffset: indexOffset + 1, completion: completion) + } } - DDLogWarn("OpenItemsController: removing not loaded item \(_item) for \(sessionIdentifier)") - existingItems.removeAll(where: { $0 == _item }) - itemsChanged = true } - guard let item, let presentation else { return nil } - presentItem(with: presentation, using: presenter) - return item } func deferredOpenItemsMenuElement( @@ -303,11 +318,12 @@ final class OpenItemsController { guard let self else { return } close(item.kind, for: sessionIdentifier) guard let presenter = openItemPresenterProvider() else { return } - if restoreMostRecentlyOpenedItem(using: presenter, sessionIdentifier: sessionIdentifier) == nil { - DDLogInfo("OpenItemsController: no open item to restore after close") - presenter.showItem(with: nil) + restoreMostRecentlyOpenedItem(using: presenter, sessionIdentifier: sessionIdentifier) { item in + if item == nil { + DDLogInfo("OpenItemsController: no open item to restore after close") + } + completion(true, true) } - completion(true, true) } currentItemActions.append(closeAction) if index > 0 { @@ -340,8 +356,9 @@ final class OpenItemsController { } else { let itemAction = UIAction(title: rItem.displayTitle, image: item.kind.icon) { [weak self] _ in guard let self, let presenter = openItemPresenterProvider() else { return } - restore(item, using: presenter) - completion(true, false) + restore(item, using: presenter) { restored in + completion(restored, false) + } } elements.append(itemAction) } @@ -395,61 +412,72 @@ final class OpenItemsController { filterValidItemsWithRItem(items).map { $0.0 } } - private func loadPresentation(for item: Item) -> Presentation? { - var presentation: Presentation? - do { - try dbStorage.perform(on: .main) { coordinator in - switch item.kind { - case .pdf(let libraryId, let key): - presentation = try loadPDFPresentation(key: key, libraryId: libraryId, coordinator: coordinator) + private func loadPresentation(for item: Item, completion: @escaping (Presentation?) -> Void) { + switch item.kind { + case .pdf(let libraryId, let key): + loadPDFPresentation(key: key, libraryId: libraryId, completion: completion) - case .note(let libraryId, let key): - presentation = try loadNotePresentation(key: key, libraryId: libraryId, coordinator: coordinator) - } - } - } catch let error { - DDLogError("OpenItemsController: can't load item \(item) - \(error)") + case .note(let libraryId, let key): + loadNotePresentation(key: key, libraryId: libraryId, completion: completion) } - return presentation - - func loadPDFPresentation(key: String, libraryId: LibraryIdentifier, coordinator: DbCoordinator) throws -> Presentation? { - let library: Library = try coordinator.perform(request: ReadLibraryDbRequest(libraryId: libraryId)) - let rItem = try coordinator.perform(request: ReadItemDbRequest(libraryId: libraryId, key: key)) - let parentKey = rItem.parent?.key - guard let attachment = AttachmentCreator.attachment(for: rItem, fileStorage: fileStorage, urlDetector: nil) else { return nil } - var url: URL? - switch attachment.type { - case .file(let filename, let contentType, let location, _, _): - switch location { - case .local, .localAndChangedRemotely: - let file = Files.attachmentFile(in: libraryId, key: key, filename: filename, contentType: contentType) - url = file.createUrl() - - case .remote, .remoteMissing: - break - } - case .url: - break + func loadPDFPresentation(key: String, libraryId: LibraryIdentifier, completion: @escaping (Presentation?) -> Void) { + do { + try dbStorage.perform(on: .main) { coordinator in + let library: Library = try coordinator.perform(request: ReadLibraryDbRequest(libraryId: libraryId)) + let rItem = try coordinator.perform(request: ReadItemDbRequest(libraryId: libraryId, key: key)) + let parentKey = rItem.parent?.key + guard let attachment = AttachmentCreator.attachment(for: rItem, fileStorage: fileStorage, urlDetector: nil) else { + completion(nil) + return + } + switch attachment.type { + case .file(let filename, let contentType, let location, _, _): + switch location { + case .local, .localAndChangedRemotely: + // TODO: Change .localAndChangedRemotely case to download first + let file = Files.attachmentFile(in: libraryId, key: key, filename: filename, contentType: contentType) + let url = file.createUrl() + completion(.pdf(library: library, key: key, parentKey: parentKey, url: url)) + + case .remote: + // TODO: Download first + completion(nil) + + case .remoteMissing: + DDLogError("OpenItemsController: can't load PDF item (key: \(key), library: \(libraryId)) - remote missing") + completion(nil) + } + + case .url: + DDLogError("OpenItemsController: can't load PDF item (key: \(key), library: \(libraryId)) - not a file attachment") + completion(nil) + } + } + } catch let error { + DDLogError("OpenItemsController: can't load PDF item (key: \(key), library: \(libraryId)) - \(error)") + completion(nil) } - guard let url else { return nil } - return .pdf(library: library, key: key, parentKey: parentKey, url: url) } - func loadNotePresentation(key: String, libraryId: LibraryIdentifier, coordinator: DbCoordinator) throws -> Presentation? { - let library = try coordinator.perform(request: ReadLibraryDbRequest(libraryId: libraryId)) - let rItem = try coordinator.perform(request: ReadItemDbRequest(libraryId: libraryId, key: key)) - let note = Note(item: rItem) - let parentTitleData: NoteEditorState.TitleData? = rItem.parent.flatMap { .init(type: $0.rawType, title: $0.displayTitle) } - guard let note else { return nil } - return .note(library: library, key: note.key, text: note.text, tags: note.tags, parentTitleData: parentTitleData, title: note.title) + func loadNotePresentation(key: String, libraryId: LibraryIdentifier, completion: @escaping (Presentation?) -> Void) { + do { + try dbStorage.perform(on: .main) { coordinator in + let library = try coordinator.perform(request: ReadLibraryDbRequest(libraryId: libraryId)) + let rItem = try coordinator.perform(request: ReadItemDbRequest(libraryId: libraryId, key: key)) + guard let note = Note(item: rItem) else { + completion(nil) + return + } + let parentTitleData: NoteEditorState.TitleData? = rItem.parent.flatMap { .init(type: $0.rawType, title: $0.displayTitle) } + completion(.note(library: library, key: note.key, text: note.text, tags: note.tags, parentTitleData: parentTitleData, title: note.title)) + } + } catch let error { + DDLogError("OpenItemsController: can't load note item (key: \(key), library: \(libraryId)) - \(error)") + completion(nil) + } } } - - private func presentItem(with presentation: Presentation, using presenter: OpenItemsPresenter) { - presenter.showItem(with: presentation) - DDLogInfo("OpenItemsController: presented item with presentation \(presentation)") - } } extension OpenItemsController { diff --git a/Zotero/Scenes/AppCoordinator.swift b/Zotero/Scenes/AppCoordinator.swift index 137dc0550..f64cbb280 100644 --- a/Zotero/Scenes/AppCoordinator.swift +++ b/Zotero/Scenes/AppCoordinator.swift @@ -212,7 +212,11 @@ extension AppCoordinator: AppDelegateCoordinatorDelegate { } mainController.showItems(for: collection, in: data.libraryId) guard data.restoreMostRecentlyOpenedItem else { return } - openItemsController.restoreMostRecentlyOpenedItem(using: self, sessionIdentifier: sessionIdentifier) + openItemsController.restoreMostRecentlyOpenedItem(using: self, sessionIdentifier: sessionIdentifier) { item in + if item == nil { + DDLogInfo("AppCoordinator: no open item to restore") + } + } func loadRestoredStateData(libraryId: LibraryIdentifier, collectionId: CollectionIdentifier) -> Collection? { guard let dbStorage = controllers.userControllers?.dbStorage else { return nil } @@ -355,7 +359,11 @@ extension AppCoordinator: AppDelegateCoordinatorDelegate { func continueUserActivity(_ userActivity: NSUserActivity, for sessionIdentifier: String) { guard userActivity.activityType == NSUserActivity.contentContainerId, let window, let mainController = window.rootViewController as? MainViewController else { return } mainController.getDetailCoordinator { [weak self] coordinator in - self?.controllers.userControllers?.openItemsController.restoreMostRecentlyOpenedItem(using: coordinator, sessionIdentifier: sessionIdentifier) + self?.controllers.userControllers?.openItemsController.restoreMostRecentlyOpenedItem(using: coordinator, sessionIdentifier: sessionIdentifier) { item in + if item == nil { + DDLogInfo("AppCoordinator: no open item to restore for continued user activity") + } + } } } } diff --git a/Zotero/Scenes/Detail/Items/Views/BaseItemsViewController.swift b/Zotero/Scenes/Detail/Items/Views/BaseItemsViewController.swift index b92c5bb07..43d7d282c 100644 --- a/Zotero/Scenes/Detail/Items/Views/BaseItemsViewController.swift +++ b/Zotero/Scenes/Detail/Items/Views/BaseItemsViewController.swift @@ -200,7 +200,11 @@ class BaseItemsViewController: UIViewController { case .restoreOpenItems: guard let presenter, let controller = controllers.userControllers?.openItemsController, let sessionIdentifier else { return } - controller.restoreMostRecentlyOpenedItem(using: presenter, sessionIdentifier: sessionIdentifier) + controller.restoreMostRecentlyOpenedItem(using: presenter, sessionIdentifier: sessionIdentifier) { item in + if item == nil { + DDLogInfo("ItemsViewController: no open item to restore") + } + } } } From a5f6b4b7106d511cceb6ea4628486b4f6f36a9d1 Mon Sep 17 00:00:00 2001 From: Miltiadis Vasilakis Date: Tue, 27 Aug 2024 16:12:12 +0300 Subject: [PATCH 3/5] In OpenItemsController download remote or local changed remotely PDFs --- Zotero/Controllers/Controllers.swift | 2 +- Zotero/Controllers/OpenItemsController.swift | 45 +++++++++++++++----- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/Zotero/Controllers/Controllers.swift b/Zotero/Controllers/Controllers.swift index 62f2cbf9b..229c0798b 100644 --- a/Zotero/Controllers/Controllers.swift +++ b/Zotero/Controllers/Controllers.swift @@ -408,7 +408,7 @@ final class UserControllers { fullSyncDebugger = FullSyncDebugger(syncScheduler: syncScheduler, debugLogging: controllers.debugLogging, sessionController: controllers.sessionController) idleTimerController = controllers.idleTimerController customUrlController = CustomURLController(dbStorage: dbStorage, fileStorage: controllers.fileStorage) - openItemsController = OpenItemsController(dbStorage: dbStorage, fileStorage: controllers.fileStorage) + openItemsController = OpenItemsController(dbStorage: dbStorage, fileStorage: controllers.fileStorage, attachmentDownloader: fileDownloader) lastBuildNumber = controllers.lastBuildNumber disposeBag = DisposeBag() } diff --git a/Zotero/Controllers/OpenItemsController.swift b/Zotero/Controllers/OpenItemsController.swift index 87a360abc..a95b7ad55 100644 --- a/Zotero/Controllers/OpenItemsController.swift +++ b/Zotero/Controllers/OpenItemsController.swift @@ -105,17 +105,20 @@ final class OpenItemsController { // MARK: Properties private unowned let dbStorage: DbStorage private unowned let fileStorage: FileStorage + private unowned let attachmentDownloader: AttachmentDownloader // TODO: Use a better data structure, such as an ordered set private var itemsBySessionIdentifier: [String: [Item]] = [:] private var sessionIdentifierByItemKind: [Item.Kind: String] = [:] private var itemsTokenBySessionIdentifier: [String: NotificationToken] = [:] private var observableBySessionIdentifier: [String: PublishSubject<[Item]>] = [:] private let disposeBag: DisposeBag + private var downloadDisposeBag: DisposeBag? // MARK: Object Lifecycle - init(dbStorage: DbStorage, fileStorage: FileStorage) { + init(dbStorage: DbStorage, fileStorage: FileStorage, attachmentDownloader: AttachmentDownloader) { self.dbStorage = dbStorage self.fileStorage = fileStorage + self.attachmentDownloader = attachmentDownloader disposeBag = DisposeBag() } @@ -434,15 +437,31 @@ final class OpenItemsController { switch attachment.type { case .file(let filename, let contentType, let location, _, _): switch location { - case .local, .localAndChangedRemotely: - // TODO: Change .localAndChangedRemotely case to download first - let file = Files.attachmentFile(in: libraryId, key: key, filename: filename, contentType: contentType) - let url = file.createUrl() - completion(.pdf(library: library, key: key, parentKey: parentKey, url: url)) - - case .remote: - // TODO: Download first - completion(nil) + case .local: + completion(createPDFPresentation(key: key, parentKey: parentKey, library: library, filename: filename, contentType: contentType)) + + case .localAndChangedRemotely, .remote: + let disposeBag = DisposeBag() + attachmentDownloader.observable + .observe(on: MainScheduler.instance) + .subscribe(onNext: { [weak self] update in + guard let self, update.libraryId == attachment.libraryId, update.key == attachment.key else { return } + switch update.kind { + case .ready: + completion(createPDFPresentation(key: key, parentKey: parentKey, library: library, filename: filename, contentType: contentType)) + downloadDisposeBag = nil + + case .cancelled, .failed: + completion(nil) + downloadDisposeBag = nil + + case .progress: + break + } + }) + .disposed(by: disposeBag) + downloadDisposeBag = disposeBag + attachmentDownloader.downloadIfNeeded(attachment: attachment, parentKey: parentKey) case .remoteMissing: DDLogError("OpenItemsController: can't load PDF item (key: \(key), library: \(libraryId)) - remote missing") @@ -458,6 +477,12 @@ final class OpenItemsController { DDLogError("OpenItemsController: can't load PDF item (key: \(key), library: \(libraryId)) - \(error)") completion(nil) } + + func createPDFPresentation(key: String, parentKey: String?, library: Library, filename: String, contentType: String) -> Presentation { + let file = Files.attachmentFile(in: library.identifier, key: key, filename: filename, contentType: contentType) + let url = file.createUrl() + return .pdf(library: library, key: key, parentKey: parentKey, url: url) + } } func loadNotePresentation(key: String, libraryId: LibraryIdentifier, completion: @escaping (Presentation?) -> Void) { From f6aea10280b6a02363f6ab908ae6ca75c6fa4c5c Mon Sep 17 00:00:00 2001 From: Miltiadis Vasilakis Date: Mon, 23 Sep 2024 12:31:19 +0300 Subject: [PATCH 4/5] Improve code --- Zotero/Controllers/OpenItemsController.swift | 19 ++++++++----------- Zotero/Scenes/AppCoordinator.swift | 12 ++++++++---- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/Zotero/Controllers/OpenItemsController.swift b/Zotero/Controllers/OpenItemsController.swift index a95b7ad55..2ce4afa09 100644 --- a/Zotero/Controllers/OpenItemsController.swift +++ b/Zotero/Controllers/OpenItemsController.swift @@ -136,7 +136,7 @@ final class OpenItemsController { itemsBySessionIdentifier[sessionIdentifier, default: []] } - func setItems(_ items: [Item], for sessionIdentifier: String, validate: Bool) { + func set(items: [Item], for sessionIdentifier: String, validate: Bool) { DDLogInfo("OpenItemsController: setting items \(items) for \(sessionIdentifier)") let existingItems = getItems(for: sessionIdentifier) let newItems = validate ? filterValidItems(items) : items @@ -179,7 +179,7 @@ final class OpenItemsController { if !deletions.isEmpty, let self { // Observed items have been deleted, call setItems to validate and register new observer. let existingItems = getItems(for: sessionIdentifier) - setItems(existingItems, for: sessionIdentifier, validate: true) + set(items: existingItems, for: sessionIdentifier, validate: true) } case .error(let error): @@ -198,7 +198,7 @@ final class OpenItemsController { for i in 0.. String? { @@ -219,7 +219,7 @@ final class OpenItemsController { let item = Item(kind: kind, userIndex: existingItems.count) let newItems = existingItems + [item] // setItems will produce next observable event - setItems(newItems, for: sessionIdentifier, validate: false) + set(items: newItems, for: sessionIdentifier, validate: false) } } @@ -270,7 +270,7 @@ final class OpenItemsController { existingItems.removeAll(where: { $0 == item }) } // setItems will produce next observable event - setItems(existingItems, for: sessionIdentifier, validate: false) + set(items: existingItems, for: sessionIdentifier, validate: false) } if let presenter { presenter.showItem(with: presentation) @@ -348,7 +348,7 @@ final class OpenItemsController { if itemsCount > 1 { let closeOtherAction = UIAction(title: L10n.Accessibility.Pdf.closeOtherOpenItems, image: .init(systemName: "checkmark.circle.badge.xmark")) { [weak self] _ in guard let self else { return } - setItems([item], for: sessionIdentifier, validate: false) + set(items: [item], for: sessionIdentifier, validate: false) completion(false, true) } currentItemActions.append(closeOtherAction) @@ -369,7 +369,7 @@ final class OpenItemsController { let closeAllAction = UIAction(title: L10n.Accessibility.Pdf.closeAllOpenItems, image: .init(systemName: "xmark.square")) { [weak self] _ in guard let self else { return } - setItems([], for: sessionIdentifier, validate: false) + set(items: [], for: sessionIdentifier, validate: false) openItemPresenterProvider()?.showItem(with: nil) completion(true, true) } @@ -507,10 +507,7 @@ final class OpenItemsController { extension OpenItemsController { func openItemsUserActivity(for sessionIdentifier: String?, libraryId: LibraryIdentifier, collectionId: CollectionIdentifier? = nil) -> NSUserActivity { - var items: [Item] = [] - if let sessionIdentifier { - items = getItems(for: sessionIdentifier) - } + let items = sessionIdentifier.flatMap({ getItems(for: $0) }) ?? [] return items.isEmpty ? .mainActivity(with: []) : .contentActivity(with: items, libraryId: libraryId, collectionId: collectionId ?? Defaults.shared.selectedCollectionId) } diff --git a/Zotero/Scenes/AppCoordinator.swift b/Zotero/Scenes/AppCoordinator.swift index f64cbb280..340f0a51e 100644 --- a/Zotero/Scenes/AppCoordinator.swift +++ b/Zotero/Scenes/AppCoordinator.swift @@ -170,7 +170,7 @@ extension AppCoordinator: AppDelegateCoordinatorDelegate { DDLogInfo("AppCoordinator: Preprocessing restored state - \(data)") Defaults.shared.selectedLibrary = data.libraryId Defaults.shared.selectedCollectionId = data.collectionId - controllers.userControllers?.openItemsController.setItems(data.openItems, for: session.persistentIdentifier, validate: true) + controllers.userControllers?.openItemsController.set(items: data.openItems, for: session.persistentIdentifier, validate: true) } return (urlContext, data) } @@ -213,7 +213,9 @@ extension AppCoordinator: AppDelegateCoordinatorDelegate { mainController.showItems(for: collection, in: data.libraryId) guard data.restoreMostRecentlyOpenedItem else { return } openItemsController.restoreMostRecentlyOpenedItem(using: self, sessionIdentifier: sessionIdentifier) { item in - if item == nil { + if let item { + DDLogInfo("AppCoordinator: restored open item - \(item)") + } else { DDLogInfo("AppCoordinator: no open item to restore") } } @@ -349,7 +351,7 @@ extension AppCoordinator: AppDelegateCoordinatorDelegate { func showMainScreen(with data: RestoredStateData, session: UISceneSession) -> Bool { guard let window, let mainController = window.rootViewController as? MainViewController else { return false } - controllers.userControllers?.openItemsController.setItems(data.openItems, for: session.persistentIdentifier, validate: true) + controllers.userControllers?.openItemsController.set(items: data.openItems, for: session.persistentIdentifier, validate: true) mainController.dismiss(animated: false) { mainController.masterCoordinator?.showCollections(for: data.libraryId, preselectedCollection: data.collectionId, animated: false) } @@ -360,7 +362,9 @@ extension AppCoordinator: AppDelegateCoordinatorDelegate { guard userActivity.activityType == NSUserActivity.contentContainerId, let window, let mainController = window.rootViewController as? MainViewController else { return } mainController.getDetailCoordinator { [weak self] coordinator in self?.controllers.userControllers?.openItemsController.restoreMostRecentlyOpenedItem(using: coordinator, sessionIdentifier: sessionIdentifier) { item in - if item == nil { + if let item { + DDLogInfo("AppCoordinator: restored open item for continued user activity - \(item)") + } else { DDLogInfo("AppCoordinator: no open item to restore for continued user activity") } } From a2e830a8dfa02de1c86853da9e42dd7d98b27ef1 Mon Sep 17 00:00:00 2001 From: Miltiadis Vasilakis Date: Mon, 23 Sep 2024 12:57:45 +0300 Subject: [PATCH 5/5] Improve code --- Zotero/Controllers/OpenItemsController.swift | 131 +++++++++---------- 1 file changed, 62 insertions(+), 69 deletions(-) diff --git a/Zotero/Controllers/OpenItemsController.swift b/Zotero/Controllers/OpenItemsController.swift index 2ce4afa09..b3a18dc7a 100644 --- a/Zotero/Controllers/OpenItemsController.swift +++ b/Zotero/Controllers/OpenItemsController.swift @@ -416,65 +416,65 @@ final class OpenItemsController { } private func loadPresentation(for item: Item, completion: @escaping (Presentation?) -> Void) { - switch item.kind { - case .pdf(let libraryId, let key): - loadPDFPresentation(key: key, libraryId: libraryId, completion: completion) + do { + try dbStorage.perform(on: .main) { coordinator in + switch item.kind { + case .pdf(let libraryId, let key): + try loadPDFPresentation(key: key, libraryId: libraryId, coordinator: coordinator, completion: completion) - case .note(let libraryId, let key): - loadNotePresentation(key: key, libraryId: libraryId, completion: completion) + case .note(let libraryId, let key): + try loadNotePresentation(key: key, libraryId: libraryId, coordinator: coordinator, completion: completion) + } + } + } catch let error { + DDLogError("OpenItemsController: can't load item \(item) - \(error)") + completion(nil) } - func loadPDFPresentation(key: String, libraryId: LibraryIdentifier, completion: @escaping (Presentation?) -> Void) { - do { - try dbStorage.perform(on: .main) { coordinator in - let library: Library = try coordinator.perform(request: ReadLibraryDbRequest(libraryId: libraryId)) - let rItem = try coordinator.perform(request: ReadItemDbRequest(libraryId: libraryId, key: key)) - let parentKey = rItem.parent?.key - guard let attachment = AttachmentCreator.attachment(for: rItem, fileStorage: fileStorage, urlDetector: nil) else { - completion(nil) - return - } - switch attachment.type { - case .file(let filename, let contentType, let location, _, _): - switch location { - case .local: - completion(createPDFPresentation(key: key, parentKey: parentKey, library: library, filename: filename, contentType: contentType)) - - case .localAndChangedRemotely, .remote: - let disposeBag = DisposeBag() - attachmentDownloader.observable - .observe(on: MainScheduler.instance) - .subscribe(onNext: { [weak self] update in - guard let self, update.libraryId == attachment.libraryId, update.key == attachment.key else { return } - switch update.kind { - case .ready: - completion(createPDFPresentation(key: key, parentKey: parentKey, library: library, filename: filename, contentType: contentType)) - downloadDisposeBag = nil - - case .cancelled, .failed: - completion(nil) - downloadDisposeBag = nil - - case .progress: - break - } - }) - .disposed(by: disposeBag) - downloadDisposeBag = disposeBag - attachmentDownloader.downloadIfNeeded(attachment: attachment, parentKey: parentKey) - - case .remoteMissing: - DDLogError("OpenItemsController: can't load PDF item (key: \(key), library: \(libraryId)) - remote missing") - completion(nil) - } - - case .url: - DDLogError("OpenItemsController: can't load PDF item (key: \(key), library: \(libraryId)) - not a file attachment") - completion(nil) - } + func loadPDFPresentation(key: String, libraryId: LibraryIdentifier, coordinator: DbCoordinator, completion: @escaping (Presentation?) -> Void) throws { + let library: Library = try coordinator.perform(request: ReadLibraryDbRequest(libraryId: libraryId)) + let rItem = try coordinator.perform(request: ReadItemDbRequest(libraryId: libraryId, key: key)) + let parentKey = rItem.parent?.key + guard let attachment = AttachmentCreator.attachment(for: rItem, fileStorage: fileStorage, urlDetector: nil) else { + completion(nil) + return + } + switch attachment.type { + case .file(let filename, let contentType, let location, _, _): + switch location { + case .local: + completion(createPDFPresentation(key: key, parentKey: parentKey, library: library, filename: filename, contentType: contentType)) + + case .localAndChangedRemotely, .remote: + let disposeBag = DisposeBag() + attachmentDownloader.observable + .observe(on: MainScheduler.instance) + .subscribe(onNext: { [weak self] update in + guard let self, update.libraryId == attachment.libraryId, update.key == attachment.key else { return } + switch update.kind { + case .ready: + completion(createPDFPresentation(key: key, parentKey: parentKey, library: library, filename: filename, contentType: contentType)) + downloadDisposeBag = nil + + case .cancelled, .failed: + completion(nil) + downloadDisposeBag = nil + + case .progress: + break + } + }) + .disposed(by: disposeBag) + downloadDisposeBag = disposeBag + attachmentDownloader.downloadIfNeeded(attachment: attachment, parentKey: parentKey) + + case .remoteMissing: + DDLogError("OpenItemsController: can't load PDF item (key: \(key), library: \(libraryId)) - remote missing") + completion(nil) } - } catch let error { - DDLogError("OpenItemsController: can't load PDF item (key: \(key), library: \(libraryId)) - \(error)") + + case .url: + DDLogError("OpenItemsController: can't load PDF item (key: \(key), library: \(libraryId)) - not a file attachment") completion(nil) } @@ -485,22 +485,15 @@ final class OpenItemsController { } } - func loadNotePresentation(key: String, libraryId: LibraryIdentifier, completion: @escaping (Presentation?) -> Void) { - do { - try dbStorage.perform(on: .main) { coordinator in - let library = try coordinator.perform(request: ReadLibraryDbRequest(libraryId: libraryId)) - let rItem = try coordinator.perform(request: ReadItemDbRequest(libraryId: libraryId, key: key)) - guard let note = Note(item: rItem) else { - completion(nil) - return - } - let parentTitleData: NoteEditorState.TitleData? = rItem.parent.flatMap { .init(type: $0.rawType, title: $0.displayTitle) } - completion(.note(library: library, key: note.key, text: note.text, tags: note.tags, parentTitleData: parentTitleData, title: note.title)) - } - } catch let error { - DDLogError("OpenItemsController: can't load note item (key: \(key), library: \(libraryId)) - \(error)") + func loadNotePresentation(key: String, libraryId: LibraryIdentifier, coordinator: DbCoordinator, completion: @escaping (Presentation?) -> Void) throws { + let library = try coordinator.perform(request: ReadLibraryDbRequest(libraryId: libraryId)) + let rItem = try coordinator.perform(request: ReadItemDbRequest(libraryId: libraryId, key: key)) + guard let note = Note(item: rItem) else { completion(nil) + return } + let parentTitleData: NoteEditorState.TitleData? = rItem.parent.flatMap { .init(type: $0.rawType, title: $0.displayTitle) } + completion(.note(library: library, key: note.key, text: note.text, tags: note.tags, parentTitleData: parentTitleData, title: note.title)) } } }