Fix datasource updates

Zotero/Scenes/Detail/PDF/Models/PDFDatabaseAnnotation.swift

@@ -142,7 +142,7 @@ struct PDFDatabaseAnnotation {
             if !library.metadataEditable {
                 return .notEditable
             }
-            return isAuthor(currentUserId: currentUserId) ? .editable : .deletable
+            return isAuthor(currentUserId: currentUserId) ? .editable : .notEditable
         }
     }
 

Zotero/Scenes/Detail/PDF/Views/Annotation View/AnnotationView.swift

@@ -145,7 +145,6 @@ final class AnnotationView: UIView {
         )
         setup(comment: comment, canEdit: canEdit)
         setup(tags: annotation.tags, canEdit: canEdit, accessibilityEnabled: selected)
-        setupObserving()
 
         let commentButtonIsHidden = commentTextView.isHidden
         let highlightContentIsHidden = highlightContent?.isHidden ?? true
@@ -283,7 +282,7 @@ final class AnnotationView: UIView {
         header.menuTap.flatMap({ Observable.just(Action.options($0)) }).bind(to: actionPublisher)
     }
 
-    private func setupObserving() {
+    func setupObserving() {
         var disposables: [Disposable] = buildDisposables()
         if let doneTap = header.doneTap {
             disposables.append(doneTap.flatMap({ Observable.just(Action.done) }).bind(to: actionPublisher))

Zotero/Scenes/Detail/PDF/Views/AnnotationCell.swift

@@ -103,6 +103,7 @@ final class AnnotationCell: UITableViewCell {
             annotationView.resignFirstResponder()
         }
 
+        let reconfiguringForSameAnnotation = key == annotation.key
         key = annotation.key
         selectionView.layer.borderWidth = selected ? PDFReaderLayout.cellSelectionLineWidth : 0
         let availableWidth = availableWidth - (PDFReaderLayout.annotationLayout.horizontalInset * 2)
@@ -121,6 +122,10 @@ final class AnnotationCell: UITableViewCell {
             pdfAnnotationsCoordinatorDelegate: pdfAnnotationsCoordinatorDelegate,
             state: state
         )
+        if !reconfiguringForSameAnnotation {
+            annotationView.setupObserving()
+        }
+        // Otherwise, reconfigured cells do not have their prepareForReuse method called, so observing is already set up.
 
         setupAccessibility(
             isAuthor: annotation.isAuthor(currentUserId: currentUserId),

Zotero/Scenes/Detail/PDF/Views/PDFAnnotationsViewController.swift

@@ -69,15 +69,9 @@ final class PDFAnnotationsViewController: UIViewController {
         super.viewIsAppearing(animated)
 
         tableView.setEditing(viewModel.state.sidebarEditingEnabled, animated: false)
-        updateQueue.async { [weak self] in
-            guard let self else { return }
-            var snapshot = NSDiffableDataSourceSnapshot<Int, PDFReaderState.AnnotationKey>()
-            snapshot.appendSections([0])
-            snapshot.appendItems(viewModel.state.sortedKeys)
-            dataSource.apply(snapshot, animatingDifferences: false) { [weak self] in
-                guard let self, let key = viewModel.state.focusSidebarKey, let indexPath = dataSource.indexPath(for: key) else { return }
-                tableView.selectRow(at: indexPath, animated: false, scrollPosition: .middle)
-            }
+        updateUI(state: viewModel.state, animatedDifferences: false) { [weak self] in
+            guard let self, let key = viewModel.state.focusSidebarKey, let indexPath = dataSource.indexPath(for: key) else { return }
+            tableView.selectRow(at: indexPath, animated: false, scrollPosition: .middle)
         }
 
         viewModel.stateObservable
@@ -143,14 +137,38 @@ final class PDFAnnotationsViewController: UIViewController {
             viewModel.process(action: .setComment(key: annotation.key, comment: comment))
 
         case .reloadHeight:
-            updateCellHeight()
-            focusSelectedCell()
+            reconfigureSelectedCellIfAny()
 
         case .setCommentActive(let isActive):
             viewModel.process(action: .setCommentActive(isActive))
 
         case .done: break // Done button doesn't appear here
         }
+
+        func reconfigureSelectedCellIfAny() {
+            guard let key = viewModel.state.selectedAnnotationKey else { return }
+            updateQueue.async { [weak self] in
+                guard let self else { return }
+                var snapshot = dataSource.snapshot()
+                snapshot.reconfigureItems([key])
+                dataSource.apply(snapshot, animatingDifferences: false) { [weak self] in
+                    self?.focusSelectedCell()
+                }
+            }
+        }
+    }
+
+    private func updateUI(state: PDFReaderState, animatedDifferences: Bool = true, completion: (() -> Void)? = nil) {
+        updateQueue.async { [weak self] in
+            guard let self else { return }
+            var snapshot = NSDiffableDataSourceSnapshot<Int, PDFReaderState.AnnotationKey>()
+            snapshot.appendSections([0])
+            snapshot.appendItems(state.sortedKeys)
+            if let keys = state.updatedAnnotationKeys {
+                snapshot.reconfigureItems(keys)
+            }
+            dataSource.apply(snapshot, animatingDifferences: animatedDifferences, completion: completion)
+        }
     }
 
     private func update(state: PDFReaderState) {
@@ -160,117 +178,99 @@ final class PDFAnnotationsViewController: UIViewController {
             emptyLabel.isHidden = !tableView.isHidden
         }
 
-        reloadIfNeeded(for: state) { [weak self] in
+        let isVisible = parentDelegate?.isSidebarVisible ?? false
+        reloadIfNeeded(for: state, isVisible: isVisible) { [weak self] in
             guard let self else { return }
 
             if let keys = state.loadedPreviewImageAnnotationKeys {
-                updatePreviewsIfVisible(for: keys)
+                updatePreviewsIfVisible(state: state, tableView: tableView, for: keys)
             }
 
             if let key = state.focusSidebarKey, let indexPath = dataSource.indexPath(for: key) {
-                let isVisible = parentDelegate?.isSidebarVisible ?? false
                 tableView.selectRow(at: indexPath, animated: isVisible, scrollPosition: .middle)
             }
-
             if state.changes.contains(.sidebarEditingSelection) {
                 deleteBarButton?.isEnabled = state.deletionEnabled
                 mergeBarButton?.isEnabled = state.mergingEnabled
             }
-
             if state.changes.contains(.filter) || state.changes.contains(.annotations) || state.changes.contains(.sidebarEditing) {
                 setupToolbar(to: state)
             }
         }
-    }
 
-    /// Updates `UIImage` of `SquareAnnotation` preview if the cell is currently on screen.
-    /// - parameter keys: Set of keys to update.
-    private func updatePreviewsIfVisible(for keys: Set<String>) {
-        let cells = tableView.visibleCells.compactMap({ $0 as? AnnotationCell }).filter({ keys.contains($0.key) })
-
-        for cell in cells {
-            let image = viewModel.state.previewCache.object(forKey: (cell.key as NSString))
-            cell.updatePreview(image: image)
-        }
-    }
-
-    /// Reloads tableView if needed, based on new state. Calls completion either when reloading finished or when there was no reload.
-    /// - parameter state: Current state.
-    /// - parameter completion: Called after reload was performed or even if there was no reload.
-    private func reloadIfNeeded(for state: PDFReaderState, completion: @escaping () -> Void) {
-        if state.document.pageCount == 0 {
-            DDLogWarn("AnnotationsViewController: trying to reload empty document")
-            completion()
-            return
-        }
-
-        let isVisible = parentDelegate?.isSidebarVisible ?? false
+        /// Reloads tableView if needed, based on new state. Calls completion either when reloading finished or when there was no reload.
+        /// - parameter state: Current state.
+        /// - parameter completion: Called after reload was performed or even if there was no reload.
+        func reloadIfNeeded(for state: PDFReaderState, isVisible: Bool, completion: @escaping () -> Void) {
+            if state.document.pageCount == 0 {
+                DDLogWarn("AnnotationsViewController: trying to reload empty document")
+                completion()
+                return
+            }
 
-        if state.changes.contains(.annotations) {
             if state.changes.contains(.sidebarEditing) {
-                tableView.setEditing(state.sidebarEditingEnabled, animated: false)
+                tableView.setEditing(state.sidebarEditingEnabled, animated: isVisible)
             }
-            updateQueue.async { [weak self] in
-                guard let self else { return }
-                var snapshot = NSDiffableDataSourceSnapshot<Int, PDFReaderState.AnnotationKey>()
-                snapshot.appendSections([0])
-                snapshot.appendItems(state.sortedKeys)
-                if let keys = state.updatedAnnotationKeys {
-                    snapshot.reloadItems(keys)
+
+            if state.changes.contains(.library) {
+                updateQueue.async { [weak self] in
+                    guard let self else { return }
+                    var snapshot = NSDiffableDataSourceSnapshot<Int, PDFReaderState.AnnotationKey>()
+                    snapshot.reloadSections([0])
+                    dataSource.apply(snapshot, animatingDifferences: isVisible, completion: completion)
                 }
-                dataSource.apply(snapshot, animatingDifferences: isVisible, completion: completion)
+                return
             }
-            return
-        }
 
-        if state.changes.contains(.interfaceStyle) {
-            updateQueue.async { [weak self] in
-                guard let self else { return }
-                var snapshot = dataSource.snapshot()
-                guard !snapshot.sectionIdentifiers.isEmpty else { return }
-                snapshot.reloadSections([0])
-                dataSource.apply(snapshot, animatingDifferences: false, completion: completion)
+            if state.changes.contains(.annotations) {
+                updateUI(state: state, animatedDifferences: isVisible, completion: completion)
+                return
             }
-            return
-        }
 
-        if state.changes.contains(.selection) || state.changes.contains(.activeComment) {
-            if let keys = state.updatedAnnotationKeys {
-                updateQueue.sync { [unowned self] in
+            if state.changes.contains(.interfaceStyle) && dataSource.snapshot().numberOfItems > 0 {
+                updateQueue.async { [weak self] in
+                    guard let self else { return }
                     var snapshot = dataSource.snapshot()
-                    snapshot.reloadItems(keys)
-                    dataSource.apply(snapshot, animatingDifferences: false)
+                    snapshot.reconfigureItems(snapshot.itemIdentifiers)
+                    dataSource.apply(snapshot, animatingDifferences: false, completion: completion)
                 }
+                return
             }
 
-            updateCellHeight()
-            focusSelectedCell()
-
-            if state.changes.contains(.sidebarEditing) {
-                tableView.setEditing(state.sidebarEditingEnabled, animated: isVisible)
+            if state.changes.contains(.selection) || state.changes.contains(.activeComment) {
+                var keys = state.updatedAnnotationKeys ?? []
+                if let key = state.selectedAnnotationKey, !keys.contains(key) {
+                    keys.append(key)
+                }
+                if !keys.isEmpty {
+                    updateQueue.async { [weak self] in
+                        guard let self else { return }
+                        var snapshot = dataSource.snapshot()
+                        snapshot.reconfigureItems(keys)
+                        dataSource.apply(snapshot, animatingDifferences: false) { [weak self] in
+                            guard let self else { return }
+                            focusSelectedCell()
+                            completion()
+                        }
+                    }
+                    return
+                }
             }
 
             completion()
-            return
-        }
-
-        if state.changes.contains(.library) {
-            tableView.reloadData()
         }
 
-        if state.changes.contains(.sidebarEditing) {
-            tableView.setEditing(state.sidebarEditingEnabled, animated: true)
+        /// Updates `UIImage` of `SquareAnnotation` preview if the cell is currently on screen.
+        /// - parameter state: Current state.
+        /// - parameter tableView: Table view
+        /// - parameter keys: Set of keys to update.
+        func updatePreviewsIfVisible(state: PDFReaderState, tableView: UITableView, for keys: Set<String>) {
+            let cells = tableView.visibleCells.compactMap({ $0 as? AnnotationCell }).filter({ keys.contains($0.key) })
+            for cell in cells {
+                let image = state.previewCache.object(forKey: (cell.key as NSString))
+                cell.updatePreview(image: image)
+            }
         }
-
-        completion()
-    }
-
-    /// Updates tableView layout in case any cell changed height.
-    private func updateCellHeight() {
-        UIView.setAnimationsEnabled(false)
-        tableView.beginUpdates()
-        tableView.endUpdates()
-        UIView.setAnimationsEnabled(true)
     }
 
     /// Scrolls to selected cell if it's not visible.
@@ -323,6 +323,7 @@ final class PDFAnnotationsViewController: UIViewController {
         }
 
         guard let boundingBoxConverter, let pdfAnnotationsCoordinatorDelegate = coordinatorDelegate else { return }
+        let reconfiguringForSameAnnotation = annotation.key == cell.key
         cell.setup(
             with: annotation,
             text: text,
@@ -339,10 +340,13 @@ final class PDFAnnotationsViewController: UIViewController {
             pdfAnnotationsCoordinatorDelegate: pdfAnnotationsCoordinatorDelegate,
             state: state
         )
-        let actionSubscription = cell.actionPublisher.subscribe(onNext: { [weak self] action in
-            self?.perform(action: action, annotation: annotation)
-        })
-        _ = cell.disposeBag?.insert(actionSubscription)
+        if !reconfiguringForSameAnnotation {
+            let actionSubscription = cell.actionPublisher.subscribe(onNext: { [weak self] action in
+                self?.perform(action: action, annotation: annotation)
+            })
+            _ = cell.disposeBag?.insert(actionSubscription)
+        }
+        // Otherwise, reconfigured cells do not have their prepareForReuse method called, so observing is already set up.
 
         func loadPreview(for annotation: PDFAnnotation, state: PDFReaderState) -> UIImage? {
             let preview = state.previewCache.object(forKey: (annotation.key as NSString))

Zotero/Scenes/Master/Collections/Views/ExpandableCollectionsCollectionViewHandler.swift

@@ -47,21 +47,18 @@ final class ExpandableCollectionsCollectionViewHandler: NSObject {
         collectionView.delegate = self
         collectionView.dropDelegate = self
         dataSource = createDataSource(for: collectionView)
+        updateQueue.async { [weak self] in
+            guard let self else { return }
+            var snapshot = NSDiffableDataSourceSnapshot<Int, Collection>()
+            snapshot.appendSections([collectionsSection])
+            dataSource.apply(snapshot, animatingDifferences: false)
+        }
 
         func createDataSource(for collectionView: UICollectionView) -> UICollectionViewDiffableDataSource<Int, Collection> {
             let registration = cellRegistration
-
-            let dataSource = UICollectionViewDiffableDataSource<Int, Collection>(collectionView: collectionView, cellProvider: { collectionView, indexPath, collection in
+            return UICollectionViewDiffableDataSource<Int, Collection>(collectionView: collectionView, cellProvider: { collectionView, indexPath, collection in
                 return collectionView.dequeueConfiguredReusableCell(using: registration, for: indexPath, item: collection)
             })
-
-            updateQueue.sync {
-                var snapshot = NSDiffableDataSourceSnapshot<Int, Collection>()
-                snapshot.appendSections([collectionsSection])
-                dataSource.apply(snapshot, animatingDifferences: false)
-            }
-
-            return dataSource
         }
     }