Changed item sorting view to expand sort options when view is full screen (#1280)

Author: michalrentka

Zotero.xcodeproj/project.pbxproj

@@ -39,17 +39,17 @@ protocol DetailItemsCoordinatorDelegate: AnyObject {
     func showItemDetail(for type: ItemDetailState.DetailType, libraryId: LibraryIdentifier, scrolledToKey childKey: String?, animated: Bool)
     func showAttachmentError(_ error: Error)
     func showAddActions(viewModel: ViewModel<ItemsActionHandler>, button: UIBarButtonItem)
-    func showSortActions(sortType: ItemsSortType, button: UIBarButtonItem, changed: @escaping (ItemsSortType) -> Void)
     func show(url: URL)
     func show(doi: String)
-    func showFilters(filters: [ItemsFilter], filtersDelegate: BaseItemsViewController, button: UIBarButtonItem)
     func showDeletionQuestion(count: Int, confirmAction: @escaping () -> Void, cancelAction: @escaping () -> Void)
     func showRemoveFromCollectionQuestion(count: Int, confirmAction: @escaping () -> Void)
     func showCitation(using presenter: UIViewController?, for itemIds: Set<String>, libraryId: LibraryIdentifier, delegate: DetailCitationCoordinatorDelegate?)
     func copyBibliography(using presenter: UIViewController, for itemIds: Set<String>, libraryId: LibraryIdentifier, delegate: DetailCopyBibliographyCoordinatorDelegate?)
     func showCiteExport(for itemIds: Set<String>, libraryId: LibraryIdentifier)
     func showAttachment(key: String, parentKey: String?, libraryId: LibraryIdentifier, readerURL: URL?)
     func show(error: ItemsError)
+    func showFilters(filters: [ItemsFilter], filtersDelegate: BaseItemsViewController, button: UIBarButtonItem)
+    func dismissFilters()
     func showLookup()
 }
 
@@ -481,66 +481,6 @@ extension DetailCoordinator: DetailItemsCoordinatorDelegate {
         self.navigationController?.present(controller, animated: true, completion: nil)
     }
 
-    func showSortActions(sortType: ItemsSortType, button: UIBarButtonItem, changed: @escaping (ItemsSortType) -> Void) {
-        DDLogInfo("DetailCoordinator: show item sort popup")
-
-        let sortNavigationController = UINavigationController()
-        sortNavigationController.modalPresentationStyle = .popover
-        sortNavigationController.popoverPresentationController?.sourceItem = button
-        let view = ItemSortingView(
-            sortType: sortType,
-            changed: changed,
-            showPicker: { [weak sortNavigationController] view in
-                guard let sortNavigationController else { return }
-                showPicker(view: view, navigationController: sortNavigationController)
-            },
-            closePicker: { [weak sortNavigationController] in
-                sortNavigationController?.popViewController(animated: true)
-            }
-        )
-        let controller = createItemSortingController(for: view, navigationController: sortNavigationController)
-        sortNavigationController.setViewControllers([controller], animated: false)
-        navigationController?.present(sortNavigationController, animated: true, completion: nil)
-
-        func createItemSortingController(for view: ItemSortingView, navigationController: UINavigationController) -> UIHostingController<ItemSortingView> {
-            let controller = DisappearActionHostingController(rootView: view)
-            var size: CGSize?
-            controller.willAppear = { [weak controller, weak navigationController] in
-                guard let controller else { return }
-                let _size = size ?? controller.view.systemLayoutSizeFitting(CGSize(width: 400.0, height: .greatestFiniteMagnitude))
-                size = _size
-                controller.preferredContentSize = _size
-                navigationController?.preferredContentSize = _size
-            }
-
-            if UIDevice.current.userInterfaceIdiom == .phone {
-                controller.didLoad = { [weak self] viewController in
-                    guard let self else { return }
-                    let doneButton = UIBarButtonItem(title: L10n.done, style: .done, target: nil, action: nil)
-                    doneButton.rx.tap
-                        .subscribe({ [weak self] _ in
-                            self?.navigationController?.dismiss(animated: true)
-                        })
-                        .disposed(by: disposeBag)
-                    viewController.navigationItem.rightBarButtonItem = doneButton
-                }
-            }
-            return controller
-        }
-
-        func showPicker(view: ItemSortTypePickerView, navigationController: UINavigationController) {
-            let controller = UIHostingController(rootView: view)
-            controller.preferredContentSize = CGSize(width: 400, height: 600)
-            navigationController.preferredContentSize = controller.preferredContentSize
-            navigationController.pushViewController(controller, animated: true)
-        }
-    }
-
-    private func sortButtonTitles(for sortType: ItemsSortType) -> (field: String, order: String) {
-        let sortOrderTitle = sortType.ascending ? L10n.Items.ascending : L10n.Items.descending
-        return ("\(L10n.Items.sortBy): \(sortType.field.title)", "\(L10n.Items.sortOrder): \(sortOrderTitle)")
-    }
-
     func createNoteController(
         library: Library,
         kind: NoteEditorKind,
@@ -655,27 +595,6 @@ extension DetailCoordinator: DetailItemsCoordinatorDelegate {
         self.navigationController?.present(navigationController, animated: true, completion: nil)
     }
 
-    func showFilters(filters: [ItemsFilter], filtersDelegate: BaseItemsViewController, button: UIBarButtonItem) {
-        DDLogInfo("DetailCoordinator: show item filters")
-
-        let navigationController = NavigationViewController()
-        navigationController.modalPresentationStyle = .popover
-        navigationController.popoverPresentationController?.sourceItem = button
-
-        let coordinator = ItemsFilterCoordinator(
-            filters: filters,
-            filtersDelegate: filtersDelegate,
-            navigationController: navigationController,
-            mainCoordinatorDelegate: mainCoordinatorDelegate,
-            controllers: controllers
-        )
-        coordinator.parentCoordinator = self
-        childCoordinators.append(coordinator)
-        coordinator.start(animated: false)
-
-        self.navigationController?.present(navigationController, animated: true, completion: nil)
-    }
-
     func showDeletionQuestion(count: Int, confirmAction: @escaping () -> Void, cancelAction: @escaping () -> Void) {
         let question = L10n.Items.deleteQuestion(count)
         self.ask(question: question, title: L10n.delete, isDestructive: true, confirm: confirmAction, cancel: cancelAction)
@@ -754,6 +673,32 @@ extension DetailCoordinator: DetailItemsCoordinatorDelegate {
         self.navigationController?.present(containerController, animated: true, completion: nil)
     }
 
+    func showFilters(filters: [ItemsFilter], filtersDelegate: BaseItemsViewController, button: UIBarButtonItem) {
+        DDLogInfo("DetailCoordinator: show item filters")
+
+        let navigationController = NavigationViewController()
+        navigationController.modalPresentationStyle = .popover
+        navigationController.popoverPresentationController?.sourceItem = button
+
+        let coordinator = ItemsFilterCoordinator(
+            filters: filters,
+            filtersDelegate: filtersDelegate,
+            navigationController: navigationController,
+            mainCoordinatorDelegate: mainCoordinatorDelegate,
+            controllers: controllers
+        )
+        coordinator.parentCoordinator = self
+        childCoordinators.append(coordinator)
+        coordinator.start(animated: false)
+
+        self.navigationController?.present(navigationController, animated: true, completion: nil)
+    }
+
+    func dismissFilters() {
+        guard let coordinator = childCoordinators.first(where: { $0 is ItemsFilterCoordinator }) as? ItemsFilterCoordinator else { return }
+        coordinator.navigationController?.dismiss(animated: true)
+    }
+
     func show(error: ItemsError) {
         let message: String
         switch error {

Zotero/Scenes/Detail/DetailCoordinator.swift

@@ -14,13 +14,17 @@ import RxSwift
 protocol ItemsToolbarControllerDelegate: UITraitEnvironment {
     func process(action: ItemAction.Kind, button: UIBarButtonItem)
     func showLookup()
+    func showFilters(button: UIBarButtonItem)
+    func sortTypeChanged(_ sortType: ItemsSortType)
+    func downloadsFilterChanged(enabled: Bool)
 }
 
 final class ItemsToolbarController {
     struct Data {
         let isEditing: Bool
         let selectedItems: Set<AnyHashable>
         let filters: [ItemsFilter]
+        let sortType: ItemsSortType
         let allowsManualSort: Bool
         let downloadBatchData: ItemsState.DownloadBatchData?
         let remoteDownloadBatchData: ItemsState.DownloadBatchData?
@@ -33,7 +37,8 @@ final class ItemsToolbarController {
         case single
         case filter
         case title
-        
+        case sort
+
         var tag: Int {
             rawValue
         }
@@ -76,6 +81,10 @@ final class ItemsToolbarController {
         }
     }
 
+    private var isCompact: Bool {
+        delegate?.traitCollection.horizontalSizeClass == .compact || UIDevice.current.userInterfaceIdiom == .phone
+    }
+
     func willAppear() {
         viewController.navigationController?.setToolbarHidden(false, animated: false)
     }
@@ -91,6 +100,7 @@ final class ItemsToolbarController {
             viewController.toolbarItems = createNormalToolbarItems(for: filters)
             updateNormalToolbarItems(
                 for: filters,
+                sortType: data.sortType,
                 downloadBatchData: data.downloadBatchData,
                 remoteDownloadBatchData: data.remoteDownloadBatchData,
                 identifierLookupBatchData: data.identifierLookupBatchData,
@@ -161,12 +171,17 @@ final class ItemsToolbarController {
 
             let filterImageName = filters.isEmpty ? "line.horizontal.3.decrease.circle" : "line.horizontal.3.decrease.circle.fill"
             let filterButton = UIBarButtonItem(image: UIImage(systemName: filterImageName), style: .plain, target: nil, action: nil)
+            if isCompact {
+                filterButton.primaryAction = UIAction { [weak self, weak filterButton] _ in
+                    guard let filterButton else { return }
+                    self?.delegate?.showFilters(button: filterButton)
+                }
+            } else {
+                let downloadsFilterEnabled = data.filters.contains(where: { $0.isDownloadedFilesFilter })
+                filterButton.menu = createFilterMenu(downloadsFilterEnabled: downloadsFilterEnabled)
+            }
             filterButton.tag = ToolbarItem.filter.tag
             filterButton.accessibilityLabel = L10n.Accessibility.Items.filterItems
-            filterButton.rx.tap.subscribe(onNext: { [weak self] _ in
-                self?.delegate?.process(action: .filter, button: filterButton)
-            })
-            .disposed(by: disposeBag)
 
             let titleButton = UIBarButtonItem(customView: createTitleView())
             titleButton.tag = ToolbarItem.title.tag
@@ -175,13 +190,9 @@ final class ItemsToolbarController {
 
             if data.allowsManualSort {
                 let action = ItemAction(type: .sort)
-                let sortButton = UIBarButtonItem(image: action.image, style: .plain, target: nil, action: nil)
+                let sortButton = UIBarButtonItem(image: action.image, menu: createSortMenu(for: data.sortType))
+                sortButton.tag = ToolbarItem.sort.tag
                 sortButton.accessibilityLabel = L10n.Accessibility.Items.sortItems
-                sortButton.rx.tap.subscribe(onNext: { [weak self] _ in
-                    self?.delegate?.process(action: action.type, button: sortButton)
-                })
-                .disposed(by: disposeBag)
-
                 items.append(contentsOf: [flexibleSpacer, sortButton, fixedSpacer])
             } else {
                 items.append(contentsOf: [flexibleSpacer, fixedSpacer])
@@ -224,6 +235,7 @@ final class ItemsToolbarController {
         } else {
             updateNormalToolbarItems(
                 for: sizeClassSpecificFilters(from: data.filters),
+                sortType: data.sortType,
                 downloadBatchData: data.downloadBatchData,
                 remoteDownloadBatchData: data.remoteDownloadBatchData,
                 identifierLookupBatchData: data.identifierLookupBatchData,
@@ -250,6 +262,37 @@ final class ItemsToolbarController {
         })
     }
 
+    private func createFilterMenu(downloadsFilterEnabled: Bool) -> UIMenu {
+        let downloadsAction = UIAction(title: L10n.Items.Filters.downloads, image: UIImage(systemName: "arrow.down.circle"), state: downloadsFilterEnabled ? .on : .off) { [weak self] _ in
+            self?.delegate?.downloadsFilterChanged(enabled: !downloadsFilterEnabled)
+        }
+        return UIMenu(title: L10n.Items.Filters.title, children: [downloadsAction])
+    }
+
+    private func createSortMenu(for sortType: ItemsSortType) -> UIMenu {
+        let ascendingAction = UIAction(title: L10n.Items.ascending, state: sortType.ascending ? .on : .off) { [weak self] _ in
+            var newSortType = sortType
+            newSortType.ascending = true
+            self?.delegate?.sortTypeChanged(newSortType)
+        }
+        let descendingAction = UIAction(title: L10n.Items.descending, state: sortType.ascending ? .off : .on) { [weak self] _ in
+            var newSortType = sortType
+            newSortType.ascending = false
+            self?.delegate?.sortTypeChanged(newSortType)
+        }
+        let orderMenu = UIMenu(title: L10n.Items.sortOrder, options: .displayInline, children: [ascendingAction, descendingAction])
+
+        let fieldActions = ItemsSortType.Field.allCases.map { field in
+            UIAction(title: field.title, state: sortType.field == field ? .on : .off) { [weak self] _ in
+                let newSortType = ItemsSortType(field: field, ascending: field.defaultOrderAscending)
+                self?.delegate?.sortTypeChanged(newSortType)
+            }
+        }
+        let fieldsMenu = UIMenu(title: L10n.Items.sortBy, options: .displayInline, children: fieldActions)
+
+        return UIMenu(children: [orderMenu, fieldsMenu])
+    }
+
     // MARK: - Helpers
 
     private func updateEditingToolbarItems(for selectedItems: Set<AnyHashable>) {
@@ -269,14 +312,30 @@ final class ItemsToolbarController {
 
     private func updateNormalToolbarItems(
         for filters: [ItemsFilter],
+        sortType: ItemsSortType,
         downloadBatchData: ItemsState.DownloadBatchData?,
         remoteDownloadBatchData: ItemsState.DownloadBatchData?,
         identifierLookupBatchData: ItemsState.IdentifierLookupBatchData,
         itemCount: Int
     ) {
+        if let item = viewController.toolbarItems?.first(where: { $0.tag == ToolbarItem.sort.tag }) {
+            item.menu = createSortMenu(for: sortType)
+        }
+
         if let item = viewController.toolbarItems?.first(where: { $0.tag == ToolbarItem.filter.tag }) {
             let filterImageName = filters.isEmpty ? "line.horizontal.3.decrease.circle" : "line.horizontal.3.decrease.circle.fill"
             item.image = UIImage(systemName: filterImageName)
+            if isCompact {
+                item.menu = nil
+                item.primaryAction = UIAction { [weak self, weak item] _ in
+                    guard let item else { return }
+                    self?.delegate?.showFilters(button: item)
+                }
+            } else {
+                item.primaryAction = nil
+                let downloadsFilterEnabled = filters.contains(where: { $0.isDownloadedFilesFilter })
+                item.menu = createFilterMenu(downloadsFilterEnabled: downloadsFilterEnabled)
+            }
         }
 
         if let item = viewController.toolbarItems?.first(where: { $0.tag == ToolbarItem.title.tag }),

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

@@ -102,6 +102,9 @@ class BaseItemsViewController: UIViewController {
 
         // willTransition(to:with:) seems to not be not called for all transitions, so instead traitCollectionDidChange(_:) is used w/ a short animation block.
         guard UIDevice.current.userInterfaceIdiom == .pad, traitCollection.horizontalSizeClass != previousTraitCollection?.horizontalSizeClass else { return }
+        if traitCollection.horizontalSizeClass == .regular {
+            coordinatorDelegate?.dismissFilters()
+        }
         setupTitle()
         UIView.animate(withDuration: 0.1) {
             self.toolbarController?.reloadToolbarItems(for: self.toolbarData)
@@ -287,6 +290,7 @@ class BaseItemsViewController: UIViewController {
             isEditing: false,
             selectedItems: [],
             filters: [],
+            sortType: .default,
             allowsManualSort: true,
             downloadBatchData: nil,
             remoteDownloadBatchData: nil,