Improve key commands (#862)

* Reimplement PDFSearchViewController dismiss shortcut as key command * Remove PDFDocumentViewController forward action shortcut * Move PDF document shortcuts to PDF reader key commands * Suppress PSPDFKitUI.PDFViewController keyCommands
zotero · Feb 29, 2024 · f71110b · f71110b
1 parent cf5a6cc
commit f71110b
Show file tree

Hide file tree

Showing 8 changed files with 55 additions and 32 deletions.
diff --git a/Zotero.xcodeproj/project.pbxproj b/Zotero.xcodeproj/project.pbxproj
@@ -29,6 +29,7 @@
 		614D65872A8030C9007CF449 /* OrderedCollections in Frameworks */ = {isa = PBXBuildFile; productRef = 614D65862A8030C9007CF449 /* OrderedCollections */; };
 		61639F852AE03B8500026003 /* InstantPresenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61639F842AE03B8500026003 /* InstantPresenter.swift */; };
 		618404262A4456A9005AAF22 /* IdentifierLookupController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 618404252A4456A9005AAF22 /* IdentifierLookupController.swift */; };
+		61A0C8472B8F669C0048FF92 /* PSPDFKitUI+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61A0C8462B8F669B0048FF92 /* PSPDFKitUI+Extensions.swift */; };
 		61ABA7512A6137D1002A4219 /* ShareableImage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61ABA7502A6137D1002A4219 /* ShareableImage.swift */; };
 		61BD13952A5831EF008A0704 /* TextKit1TextView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61BD13942A5831EF008A0704 /* TextKit1TextView.swift */; };
 		61C817F22A49B5D30085B1E6 /* CollectionResponseSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3B1EDEE250242E700D8BC1E /* CollectionResponseSpec.swift */; };
@@ -1236,6 +1237,7 @@
 		614D65842A7BCC22007CF449 /* ci_post_clone.sh */ = {isa = PBXFileReference; lastKnownFileType = text.script.sh; path = ci_post_clone.sh; sourceTree = "<group>"; };
 		61639F842AE03B8500026003 /* InstantPresenter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InstantPresenter.swift; sourceTree = "<group>"; };
 		618404252A4456A9005AAF22 /* IdentifierLookupController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IdentifierLookupController.swift; sourceTree = "<group>"; };
+		61A0C8462B8F669B0048FF92 /* PSPDFKitUI+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "PSPDFKitUI+Extensions.swift"; sourceTree = "<group>"; };
 		61ABA7502A6137D1002A4219 /* ShareableImage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShareableImage.swift; sourceTree = "<group>"; };
 		61BD13942A5831EF008A0704 /* TextKit1TextView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TextKit1TextView.swift; sourceTree = "<group>"; };
 		61FA14CD2B05081D00E7D423 /* TextConverter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TextConverter.swift; sourceTree = "<group>"; };
@@ -2521,6 +2523,7 @@
 				B305650B23FC051E003304F2 /* Observable+Completable.swift */,
 				B305650D23FC051E003304F2 /* PreferenceKeys.swift */,
 				B3A94B492462F5D300BC7910 /* PSPDFKit+Extensions.swift */,
+				61A0C8462B8F669B0048FF92 /* PSPDFKitUI+Extensions.swift */,
 				B3DDC0CA2667824D00B2DFD1 /* RegularExpression+Extensions.swift */,
 				B340692124A60D6A009ECE48 /* Rounding+Extensions.swift */,
 				B305650823FC051E003304F2 /* String+Extensions.swift */,
@@ -5308,6 +5311,7 @@
 				B35C529C26383BDD007BD036 /* ReadAllDownloadedItemsDbRequest.swift in Sources */,
 				B373C98F2B1F5431007FD56C /* PDFThumbnailsLayout.swift in Sources */,
 				B305660723FC051E003304F2 /* LoginRequest.swift in Sources */,
+				61A0C8472B8F669C0048FF92 /* PSPDFKitUI+Extensions.swift in Sources */,
 				B30566B323FC051F003304F2 /* CollectionResponse.swift in Sources */,
 				B34341AB260A48A200093E63 /* ReadAllWritableGroupsDbRequest.swift in Sources */,
 				B3593F4B241A61C700760E20 /* LibrariesError.swift in Sources */,

diff --git a/Zotero/Assets/en.lproj/Localizable.strings b/Zotero/Assets/en.lproj/Localizable.strings
@@ -221,6 +221,7 @@
 "pdf.annotations_sidebar.filter.tags_placeholder" = "Select Tags…";
 "pdf.search.title" = "Search in Document";
 "pdf.search.failed" = "Search failed";
+"pdf.search.dismiss" = "Dismiss Search";
 "pdf.annotation_popover.title" = "Edit Annotation";
 "pdf.annotation_popover.delete" = "Delete Annotation";
 "pdf.annotation_popover.update_subsequent_pages" = "Update subsequent pages";

diff --git a/Zotero/Extensions/Localizable.swift b/Zotero/Extensions/Localizable.swift
@@ -968,6 +968,8 @@ internal enum L10n {
       internal static let locked = L10n.tr("Localizable", "pdf.locked.locked", fallback: "Locked")
     }
     internal enum Search {
+      /// Dismiss Search
+      internal static let dismiss = L10n.tr("Localizable", "pdf.search.dismiss", fallback: "Dismiss Search")
       /// Search failed
       internal static let failed = L10n.tr("Localizable", "pdf.search.failed", fallback: "Search failed")
       /// Plural format key: "%#@matches@"

diff --git a/Zotero/Extensions/PSPDFKitUI+Extensions.swift b/Zotero/Extensions/PSPDFKitUI+Extensions.swift
@@ -0,0 +1,15 @@
+//
+//  PSPDFKitUI+Extensions.swift
+//  Zotero
+//
+//  Created by Miltiadis Vasilakis on 28/2/24.
+//  Copyright © 2024 Corporation for Digital Scholarship. All rights reserved.
+//
+
+import PSPDFKitUI
+
+extension PSPDFKitUI.PDFViewController {
+    open override var keyCommands: [UIKeyCommand]? {
+        []
+    }
+}
diff --git a/Zotero/Scenes/Detail/PDF/Views/IntraDocumentNavigationButtonsHandler.swift b/Zotero/Scenes/Detail/PDF/Views/IntraDocumentNavigationButtonsHandler.swift
@@ -10,6 +10,9 @@ import UIKit
 
 final class IntraDocumentNavigationButtonsHandler {
     private weak var backButton: UIButton!
+    var showsBackButton: Bool {
+        backButton?.isHidden == false
+    }
 
     init(parent: UIViewController, back: @escaping () -> Void) {
         var backConfiguration = UIButton.Configuration.plain()

diff --git a/Zotero/Scenes/Detail/PDF/Views/PDFDocumentViewController.swift b/Zotero/Scenes/Detail/PDF/Views/PDFDocumentViewController.swift
@@ -99,36 +99,12 @@ final class PDFDocumentViewController: UIViewController {
         self.updatePencilSettingsIfNeeded()
     }
 
-    override func pressesBegan(_ presses: Set<UIPress>, with event: UIPressesEvent?) {
-        guard let controller = self.pdfController, let key = presses.first?.key, key.modifierFlags.contains(.command) else {
-            super.pressesBegan(presses, with: event)
-            return
-        }
-
-        if key.characters == "f" {
-            self.parentDelegate?.showSearch(pdfController: controller, text: nil)
-            return
-        }
-        if key.characters == "[" || key.keyCode == .keyboardLeftArrow {
-            performBackAction()
-            return
-        }
-        if key.characters == "]" || key.keyCode == .keyboardRightArrow {
-            performForwardAction()
-            return
-        }
-    }
-
     // MARK: - Actions
 
     func performBackAction() {
         pdfController?.backForwardList.requestBack(animated: true)
     }
 
-    func performForwardAction() {
-        pdfController?.backForwardList.requestForward(animated: true)
-    }
-
     func focus(page: UInt) {
         self.scrollIfNeeded(to: page, animated: true, completion: {})
     }

diff --git a/Zotero/Scenes/Detail/PDF/Views/PDFReaderViewController.swift b/Zotero/Scenes/Detail/PDF/Views/PDFReaderViewController.swift
@@ -149,6 +149,19 @@ class PDFReaderViewController: UIViewController {
         return barButton
     }()
 
+    override var keyCommands: [UIKeyCommand]? {
+        var keyCommands: [UIKeyCommand] = [
+            .init(title: L10n.Pdf.Search.title, action: #selector(search), input: "f", modifierFlags: [.command])
+        ]
+        if intraDocumentNavigationHandler?.showsBackButton == true {
+            keyCommands += [
+                .init(title: L10n.back, action: #selector(performBackAction), input: "[", modifierFlags: [.command]),
+                .init(title: L10n.back, action: #selector(performBackAction), input: UIKeyCommand.inputLeftArrow, modifierFlags: [.command])
+            ]
+        }
+        return keyCommands
+    }
+
     init(viewModel: ViewModel<PDFReaderActionHandler>, compactSize: Bool) {
         self.viewModel = viewModel
         isCompactWidth = compactSize
@@ -601,6 +614,15 @@ class PDFReaderViewController: UIViewController {
         navigationController?.presentingViewController?.dismiss(animated: true, completion: nil)
     }
 
+    @objc private func search() {
+        guard let pdfController = documentController.pdfController else { return }
+        showSearch(pdfController: pdfController, text: nil)
+    }
+
+    @objc private func performBackAction() {
+        documentController.performBackAction()
+    }
+
     // MARK: - Setups
 
     private func setupAccessibility(forSidebarButton button: UIBarButtonItem) {

diff --git a/Zotero/Scenes/Detail/PDF/Views/PDFSearchViewController.swift b/Zotero/Scenes/Detail/PDF/Views/PDFSearchViewController.swift
@@ -43,6 +43,10 @@ final class PDFSearchViewController: UIViewController {
         }
     }
 
+    override var keyCommands: [UIKeyCommand]? {
+        [.init(title: L10n.Pdf.Search.dismiss, action: #selector(dismissSearch), input: UIKeyCommand.inputEscape)]
+    }
+
     init(controller: PDFViewController, text: String?) {
         self.text = text
         pdfController = controller
@@ -119,14 +123,6 @@ final class PDFSearchViewController: UIViewController {
         DDLogInfo("PDFSearchViewController deinitialized")
     }
 
-    override func pressesBegan(_ presses: Set<UIPress>, with event: UIPressesEvent?) {
-        guard let key = presses.first?.key, key.keyCode == .keyboardEscape else {
-            super.pressesBegan(presses, with: event)
-            return
-        }
-        dismiss(animated: true, completion: nil)
-    }
-
     // MARK: - Actions
 
     private func search(for string: String) {
@@ -150,6 +146,10 @@ final class PDFSearchViewController: UIViewController {
         delegate?.didFinishSearch(with: results, for: text)
         self.tableView.reloadData()
     }
+
+    @objc private func dismissSearch() {
+        dismiss(animated: true)
+    }
 }
 
 extension PDFSearchViewController: UITableViewDataSource, UITableViewDelegate {