Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PDFAnnotationsViewController edge case bug #1048

Merged
merged 3 commits into from
Jan 6, 2025
Merged

Fix PDFAnnotationsViewController edge case bug #1048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7740d13
Fix PDFAnnotationsViewController edge case bug
mvasilak Jan 4, 2025
079e494
Improve datasource reconfigure items logic
mvasilak Jan 5, 2025
9b0a611
Improve ExpandableCollectionsCollectionViewHandler data source handling
mvasilak Jan 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions Zotero/Scenes/Detail/PDF/Views/PDFAnnotationsViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,10 @@ final class PDFAnnotationsViewController: UIViewController {

func reconfigureSelectedCellIfAny() {
guard let key = viewModel.state.selectedAnnotationKey else { return }
var snapshot = dataSource.snapshot()
guard snapshot.itemIdentifiers.contains(where: { $0 == key }) 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()
Expand All @@ -164,7 +165,7 @@ final class PDFAnnotationsViewController: UIViewController {
var snapshot = NSDiffableDataSourceSnapshot<Int, PDFReaderState.AnnotationKey>()
snapshot.appendSections([0])
snapshot.appendItems(state.sortedKeys)
if let keys = state.updatedAnnotationKeys {
if let keys = state.updatedAnnotationKeys?.filter({ snapshot.itemIdentifiers.contains($0) }), !keys.isEmpty {
snapshot.reconfigureItems(keys)
}
dataSource.apply(snapshot, animatingDifferences: animatedDifferences, completion: completion)
Expand Down Expand Up @@ -242,10 +243,12 @@ final class PDFAnnotationsViewController: UIViewController {
if let key = state.selectedAnnotationKey, !keys.contains(key) {
keys.append(key)
}
var snapshot = dataSource.snapshot()
// Filter any items identifiers not existing already in the snapshot. These will be added in a subsequent update.
keys = keys.filter({ snapshot.itemIdentifiers.contains($0) })
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 }
Expand Down
5 changes: 2 additions & 3 deletions Zotero/Scenes/Detail/PDF/Views/PDFThumbnailsViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,10 @@ class PDFThumbnailsViewController: UICollectionViewController {
return
}

if let index = state.loadedThumbnail, index < dataSource.snapshot().itemIdentifiers.count {
var snapshot = dataSource.snapshot()
if let index = state.loadedThumbnail, index < snapshot.itemIdentifiers.count {
updateQueue.async { [weak self] in
guard let self else { return }
var snapshot = dataSource.snapshot()
let label = dataSource.snapshot().itemIdentifiers[index]
snapshot.reconfigureItems([label])
dataSource.apply(snapshot)
Expand All @@ -119,7 +119,6 @@ class PDFThumbnailsViewController: UICollectionViewController {
}

// The following updates should be ignored if the collection hasn't loaded yet for the first time.
var snapshot = dataSource.snapshot()
guard snapshot.numberOfSections > 0 else { return }

if state.changes.contains(.userInterface) || state.changes.contains(.reload) {
Expand Down
5 changes: 4 additions & 1 deletion Zotero/Scenes/Master/Collections/Views/ExpandableCollectionsCollectionViewHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ final class ExpandableCollectionsCollectionViewHandler: NSObject {
collectionView.delegate = self
collectionView.dropDelegate = self
dataSource = createDataSource(for: collectionView)
collectionView.dataSource = dataSource
updateQueue.async { [weak self] in
guard let self else { return }
var snapshot = NSDiffableDataSourceSnapshot<Int, Collection>()
Expand Down Expand Up @@ -74,6 +75,7 @@ final class ExpandableCollectionsCollectionViewHandler: NSObject {
}
}

guard let dataSource = dataSource else { return }
let snapshot = dataSource.snapshot(for: collectionsSection)

if !snapshot.items.contains(where: { $0.identifier == collectionId }) {
Expand All @@ -89,7 +91,7 @@ final class ExpandableCollectionsCollectionViewHandler: NSObject {
return
}

if let index = dataSource.snapshot(for: collectionsSection).visibleItems.firstIndex(where: { $0.identifier == collectionId }) {
if let index = snapshot.visibleItems.firstIndex(where: { $0.identifier == collectionId }) {
let indexPath = IndexPath(item: index, section: 0)
guard selectedIndexPaths.first != indexPath else { return }
collectionView.selectItem(at: indexPath, animated: false, scrollPosition: scrollToPosition ? .centeredVertically : [])
Expand All @@ -99,6 +101,7 @@ final class ExpandableCollectionsCollectionViewHandler: NSObject {
}

func update(with tree: CollectionTree, selectedId: CollectionIdentifier, animated: Bool, completion: (() -> Void)? = nil) {
guard let dataSource = dataSource else { return }
let newSnapshot = tree.createSnapshot(selectedId: selectedId)

if dataSource.snapshot(for: collectionsSection).items.count == newSnapshot.items.count {
Expand Down