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

Don't open metadata on cell tap #751

Merged
merged 1 commit into from
Aug 24, 2023
Merged

Don't open metadata on cell tap #751

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 18 additions & 3 deletions Zotero/Scenes/Detail/Items/Views/ItemsTableViewHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ protocol ItemsTableViewHandlerDelegate: AnyObject {
final class ItemsTableViewHandler: NSObject {
enum TapAction {
case metadata(RItem)
case note(RItem)
case attachment(attachment: Attachment, parentKey: String?)
case doi(String)
case url(URL)
}
Expand Down Expand Up @@ -358,14 +360,20 @@ extension ItemsTableViewHandler: UITableViewDelegate {
tableView.deselectRow(at: indexPath, animated: true)

guard let accessory = self.viewModel.state.itemAccessories[item.key] else {
self.tapObserver.on(.next(.metadata(item)))
switch item.rawType {
case ItemTypes.note:
self.tapObserver.on(.next(.note(item)))

default:
break
}
return
}

switch accessory {
case .attachment(let attachment):
let parentKey = item.key == attachment.key ? nil : item.key
self.viewModel.process(action: .openAttachment(attachment: attachment, parentKey: parentKey))
self.tapObserver.on(.next(.attachment(attachment: attachment, parentKey: parentKey)))

case .doi(let doi):
self.tapObserver.on(.next(.doi(doi)))
Expand All @@ -377,7 +385,14 @@ extension ItemsTableViewHandler: UITableViewDelegate {

func tableView(_ tableView: UITableView, accessoryButtonTappedForRowWith indexPath: IndexPath) {
guard let item = self.snapshot?[indexPath.row] else { return }
self.tapObserver.on(.next(.metadata(item)))

switch item.rawType {
case ItemTypes.note:
self.tapObserver.on(.next(.note(item)))

default:
self.tapObserver.on(.next(.metadata(item)))
}
}

func tableView(_ tableView: UITableView, didDeselectRowAt indexPath: IndexPath) {
Expand Down
227 changes: 121 additions & 106 deletions Zotero/Scenes/Detail/Items/Views/ItemsViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,14 @@ final class ItemsViewController: UIViewController {
override func viewDidLoad() {
super.viewDidLoad()

self.tableViewHandler = ItemsTableViewHandler(tableView: self.tableView,
viewModel: self.viewModel,
delegate: self,
dragDropController: self.controllers.dragDropController,
fileDownloader: self.controllers.userControllers?.fileDownloader,
schemaController: self.controllers.schemaController)
self.tableViewHandler = ItemsTableViewHandler(
tableView: self.tableView,
viewModel: self.viewModel,
delegate: self,
dragDropController: self.controllers.dragDropController,
fileDownloader: self.controllers.userControllers?.fileDownloader,
schemaController: self.controllers.schemaController
)
self.toolbarController = ItemsToolbarController(viewController: self, initialState: self.viewModel.state, delegate: self)
self.navigationController?.toolbar.barTintColor = UIColor(dynamicProvider: { traitCollection in
return traitCollection.userInterfaceStyle == .dark ? .black : .white
Expand All @@ -102,29 +104,22 @@ final class ItemsViewController: UIViewController {
self.startObserving(results: results)
}

self.tableViewHandler.tapObserver
.observe(on: MainScheduler.instance)
.subscribe(with: self, onNext: { `self`, action in
switch action {
case .metadata(let item):
self.showItemDetail(for: item)
self.resetActiveSearch()

case .doi(let doi):
self.coordinatorDelegate?.show(doi: doi)

case .url(let url):
self.coordinatorDelegate?.show(url: url)
}
})
.disposed(by: self.disposeBag)
self.tableViewHandler
.tapObserver
.observe(on: MainScheduler.instance)
.subscribe(with: self, onNext: { `self`, action in
self.resetActiveSearch()
self.handle(action: action)
})
.disposed(by: self.disposeBag)

self.viewModel.stateObservable
.observe(on: MainScheduler.instance)
.subscribe(with: self, onNext: { `self`, state in
self.update(state: state)
})
.disposed(by: self.disposeBag)
self.viewModel
.stateObservable
.observe(on: MainScheduler.instance)
.subscribe(with: self, onNext: { `self`, state in
self.update(state: state)
})
.disposed(by: self.disposeBag)
}

override func viewWillAppear(_ animated: Bool) {
Expand Down Expand Up @@ -215,7 +210,12 @@ final class ItemsViewController: UIViewController {
}

if let key = state.itemKeyToDuplicate {
self.coordinatorDelegate?.showItemDetail(for: .duplication(itemKey: key, collectionKey: self.viewModel.state.collection.identifier.key), library: self.viewModel.state.library, scrolledToKey: nil, animated: true)
self.coordinatorDelegate?.showItemDetail(
for: .duplication(itemKey: key, collectionKey: self.viewModel.state.collection.identifier.key),
library: self.viewModel.state.library,
scrolledToKey: nil,
animated: true
)
}

if state.processingBibliography {
Expand Down Expand Up @@ -255,6 +255,38 @@ final class ItemsViewController: UIViewController {

// MARK: - Actions

private func handle(action: ItemsTableViewHandler.TapAction) {
switch action {
case .metadata(let item):
self.coordinatorDelegate?.showItemDetail(for: .preview(key: item.key), library: self.viewModel.state.library, scrolledToKey: nil, animated: true)

case .attachment(let attachment, let parentKey):
self.viewModel.process(action: .openAttachment(attachment: attachment, parentKey: parentKey))

case .doi(let doi):
self.coordinatorDelegate?.show(doi: doi)

case .url(let url):
self.coordinatorDelegate?.show(url: url)

case .note(let item):
guard let note = Note(item: item) else { return }
let tags = Array(item.tags.map({ Tag(tag: $0) }))
let library = self.viewModel.state.library
self.coordinatorDelegate?.showNote(
with: note.text,
tags: tags,
title: nil,
key: note.key,
libraryId: library.identifier,
readOnly: !library.metadataEditable,
save: { [weak self] newText, newTags in
self?.viewModel.process(action: .saveNote(note.key, newText, newTags))
}
)
}
}

private func updateTagFilter(with state: ItemsState) {
self.tagFilterDelegate?.itemsDidChange(filters: state.filters, collectionId: state.collection.identifier, libraryId: state.library.identifier)
}
Expand Down Expand Up @@ -328,7 +360,12 @@ final class ItemsViewController: UIViewController {
default: break
}

self.coordinatorDelegate?.showItemDetail(for: .creation(type: ItemTypes.document, child: attachment, collectionKey: collectionKey), library: self.viewModel.state.library, scrolledToKey: nil, animated: true)
self.coordinatorDelegate?.showItemDetail(
for: .creation(type: ItemTypes.document, child: attachment, collectionKey: collectionKey),
library: self.viewModel.state.library,
scrolledToKey: nil,
animated: true
)

case .delete:
guard !selectedKeys.isEmpty else { return }
Expand Down Expand Up @@ -408,29 +445,6 @@ final class ItemsViewController: UIViewController {
}
}

private func showItemDetail(for item: RItem) {
switch item.rawType {
case ItemTypes.note:
guard let note = Note(item: item) else { return }
let tags = Array(item.tags.map({ Tag(tag: $0) }))
let library = self.viewModel.state.library
self.coordinatorDelegate?.showNote(
with: note.text,
tags: tags,
title: nil,
key: note.key,
libraryId: library.identifier,
readOnly: !library.metadataEditable,
save: { [weak self] newText, newTags in
self?.viewModel.process(action: .saveNote(note.key, newText, newTags))
}
)

default:
self.coordinatorDelegate?.showItemDetail(for: .preview(key: item.key), library: self.viewModel.state.library, scrolledToKey: nil, animated: true)
}
}

private func startObserving(results: Results<RItem>) {
self.resultsToken = results.observe(keyPaths: RItem.observableKeypathsForItemList, { [weak self] changes in
guard let self = self else { return }
Expand Down Expand Up @@ -498,59 +512,57 @@ final class ItemsViewController: UIViewController {

private func setupAppStateObserver() {
NotificationCenter.default
.rx
.notification(.willEnterForeground)
.observe(on: MainScheduler.instance)
.subscribe(onNext: { [weak self] _ in
guard let self = self else { return }
if self.searchBarNeedsReset {
self.resetSearchBar()
self.searchBarNeedsReset = false
}
})
.disposed(by: self.disposeBag)
.rx
.notification(.willEnterForeground)
.observe(on: MainScheduler.instance)
.subscribe(onNext: { [weak self] _ in
guard let self = self, self.searchBarNeedsReset else { return }
self.resetSearchBar()
self.searchBarNeedsReset = false
})
.disposed(by: self.disposeBag)
}

private func setupFileObservers() {
NotificationCenter.default
.rx
.notification(.attachmentFileDeleted)
.observe(on: MainScheduler.instance)
.subscribe(onNext: { [weak self] notification in
if let notification = notification.object as? AttachmentFileDeletedNotification {
self?.viewModel.process(action: .updateAttachments(notification))
}
})
.disposed(by: self.disposeBag)
.rx
.notification(.attachmentFileDeleted)
.observe(on: MainScheduler.instance)
.subscribe(onNext: { [weak self] notification in
if let notification = notification.object as? AttachmentFileDeletedNotification {
self?.viewModel.process(action: .updateAttachments(notification))
}
})
.disposed(by: self.disposeBag)

guard let downloader = self.controllers.userControllers?.fileDownloader else { return }

downloader.observable
.observe(on: MainScheduler.instance)
.subscribe(with: self, onNext: { [weak downloader] `self`, update in
if let downloader = downloader {
let (progress, remainingCount, totalCount) = downloader.batchData
let batchData = progress.flatMap({ ItemsState.DownloadBatchData(progress: $0, remaining: remainingCount, total: totalCount) })
self.viewModel.process(action: .updateDownload(update: update, batchData: batchData))
}
.observe(on: MainScheduler.instance)
.subscribe(with: self, onNext: { [weak downloader] `self`, update in
if let downloader = downloader {
let (progress, remainingCount, totalCount) = downloader.batchData
let batchData = progress.flatMap({ ItemsState.DownloadBatchData(progress: $0, remaining: remainingCount, total: totalCount) })
self.viewModel.process(action: .updateDownload(update: update, batchData: batchData))
}

if case .progress = update.kind { return }
if case .progress = update.kind { return }

guard self.viewModel.state.attachmentToOpen == update.key else { return }
guard self.viewModel.state.attachmentToOpen == update.key else { return }

self.viewModel.process(action: .attachmentOpened(update.key))
self.viewModel.process(action: .attachmentOpened(update.key))

switch update.kind {
case .ready:
self.coordinatorDelegate?.showAttachment(key: update.key, parentKey: update.parentKey, libraryId: update.libraryId)
switch update.kind {
case .ready:
self.coordinatorDelegate?.showAttachment(key: update.key, parentKey: update.parentKey, libraryId: update.libraryId)

case .failed(let error):
self.coordinatorDelegate?.showAttachmentError(error)
case .failed(let error):
self.coordinatorDelegate?.showAttachmentError(error)

default: break
}
})
.disposed(by: self.disposeBag)
default: break
}
})
.disposed(by: self.disposeBag)
}

private func setupTitle() {
Expand All @@ -559,7 +571,9 @@ final class ItemsViewController: UIViewController {
}

private func updateEmptyTrashButton(toEnabled enabled: Bool) {
guard self.viewModel.state.collection.identifier.isTrash, let item = self.navigationItem.rightBarButtonItems?.first(where: { button in RightBarButtonItem(rawValue: button.tag) == .emptyTrash }) else { return }
guard self.viewModel.state.collection.identifier.isTrash,
let item = self.navigationItem.rightBarButtonItems?.first(where: { button in RightBarButtonItem(rawValue: button.tag) == .emptyTrash })
else { return }
item.isEnabled = enabled
}

Expand Down Expand Up @@ -708,13 +722,14 @@ final class ItemsViewController: UIViewController {
/// - parameter searchBar: `searchBar` to setup and observe.
private func setup(searchBar: UISearchBar) {
searchBar.placeholder = L10n.Items.searchTitle
searchBar.rx.text.observe(on: MainScheduler.instance)
.skip(1)
.debounce(.milliseconds(150), scheduler: MainScheduler.instance)
.subscribe(onNext: { [weak self] text in
self?.viewModel.process(action: .search(text ?? ""))
})
.disposed(by: self.disposeBag)
searchBar.rx
.text.observe(on: MainScheduler.instance)
.skip(1)
.debounce(.milliseconds(150), scheduler: MainScheduler.instance)
.subscribe(onNext: { [weak self] text in
self?.viewModel.process(action: .search(text ?? ""))
})
.disposed(by: self.disposeBag)
}

/// Removes `searchBar` from all positions.
Expand All @@ -739,12 +754,12 @@ final class ItemsViewController: UIViewController {
private func setupSyncObserving() {
guard let scheduler = self.controllers.userControllers?.syncScheduler else { return }
scheduler.syncController
.progressObservable
.observe(on: MainScheduler.instance)
.subscribe(onNext: { [weak self] progress in
self?.update(progress: progress)
})
.disposed(by: self.disposeBag)
.progressObservable
.observe(on: MainScheduler.instance)
.subscribe(onNext: { [weak self] progress in
self?.update(progress: progress)
})
.disposed(by: self.disposeBag)
}

private func setupOverlay() {
Expand Down