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

Improve PDF reader navigation buttons appearance logic #1081

Merged
merged 5 commits into from
Mar 4, 2025
Merged

Improve PDF reader navigation buttons appearance logic #1081

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
519ef61
Add back action when PDF reader scrolls to top by tapping status bar
mvasilak Feb 28, 2025
672f4ec
Improve PDF reader back button placement when sidebar is visible
mvasilak Feb 28, 2025
73a616e
Add back action on PDF reader page change by annotation/outline/search
mvasilak Mar 1, 2025
0dcc00e
Show forward action on PDF reader when visible
mvasilak Mar 4, 2025
054f5ae
Add back action on PDF reader page change by thumbnail list/scrubber
mvasilak Mar 4, 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
36 changes: 29 additions & 7 deletions Zotero/Scenes/Detail/PDF/Views/IntraDocumentNavigationButtonsHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,15 @@ import UIKit

final class IntraDocumentNavigationButtonsHandler {
private weak var backButton: UIButton!
private weak var forwardButton: UIButton!
var showsBackButton: Bool {
backButton?.isHidden == false
}
var showsForwardButton: Bool {
forwardButton?.isHidden == false
}

init(parent: UIViewController, back: @escaping () -> Void) {
init(parent: UIViewController, back: @escaping () -> Void, forward: @escaping () -> Void) {
var backConfiguration = UIButton.Configuration.plain()
backConfiguration.title = L10n.back
backConfiguration.image = UIImage(systemName: "chevron.left", withConfiguration: UIImage.SymbolConfiguration(scale: .small))
Expand All @@ -31,17 +35,35 @@ final class IntraDocumentNavigationButtonsHandler {
parent.view.addSubview(backButton)
self.backButton = backButton

var forwardConfiguration = UIButton.Configuration.plain()
forwardConfiguration.title = L10n.forward
forwardConfiguration.image = UIImage(systemName: "chevron.right", withConfiguration: UIImage.SymbolConfiguration(scale: .small))
forwardConfiguration.baseForegroundColor = Asset.Colors.zoteroBlueWithDarkMode.color
forwardConfiguration.background.backgroundColor = Asset.Colors.navbarBackground.color
forwardConfiguration.imagePadding = 8
forwardConfiguration.imagePlacement = .trailing
let forwardButton = UIButton(configuration: forwardConfiguration)
forwardButton.translatesAutoresizingMaskIntoConstraints = false
forwardButton.isHidden = true
forwardButton.addAction(
UIAction(handler: { _ in forward() }),
for: .touchUpInside
)
parent.view.addSubview(forwardButton)
self.forwardButton = forwardButton

NSLayoutConstraint.activate([
backButton.leadingAnchor.constraint(equalTo: parent.view.leadingAnchor, constant: 30),
parent.view.bottomAnchor.constraint(equalTo: backButton.bottomAnchor, constant: 80)
parent.view.bottomAnchor.constraint(equalTo: backButton.bottomAnchor, constant: 80),
forwardButton.trailingAnchor.constraint(equalTo: parent.view.trailingAnchor, constant: -30),
parent.view.bottomAnchor.constraint(equalTo: forwardButton.bottomAnchor, constant: 80)
])
}

func bringButtonToFront() {
backButton.superview?.bringSubviewToFront(backButton)
}

func set(backButtonVisible: Bool) {
func set(backButtonVisible: Bool, forwardButtonVisible: Bool) {
backButton.isHidden = !backButtonVisible
forwardButton.isHidden = !forwardButtonVisible
backButton.superview?.bringSubviewToFront(backButton)
forwardButton.superview?.bringSubviewToFront(forwardButton)
}
}
45 changes: 40 additions & 5 deletions Zotero/Scenes/Detail/PDF/Views/PDFDocumentViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ protocol PDFDocumentDelegate: AnyObject {
func didChange(undoState undoEnabled: Bool, redoState redoEnabled: Bool)
func interfaceVisibilityDidChange(to isHidden: Bool)
func showToolOptions()
func backNavigationButtonChanged(visible: Bool)
func navigationButtonsChanged(backVisible: Bool, forwardVisible: Bool)
func didSelectText(_ text: String)
}

Expand Down Expand Up @@ -111,6 +111,10 @@ final class PDFDocumentViewController: UIViewController {
pdfController?.backForwardList.requestBack(animated: true)
}

func performForwardAction() {
pdfController?.backForwardList.requestForward(animated: true)
}

func focus(page: UInt) {
self.scrollIfNeeded(to: page, animated: true, completion: {})
}
Expand Down Expand Up @@ -256,7 +260,9 @@ final class PDFDocumentViewController: UIViewController {
}

if state.changes.contains(.visiblePageFromThumbnailList) {
let currentPageIndex = pdfController.pageIndex
pdfController.setPageIndex(PageIndex(state.visiblePage), animated: false)
pdfController.backForwardList.register(PSPDFKit.GoToAction(pageIndex: currentPageIndex))
}

if let tool = state.changedColorForTool, let color = state.toolColors[tool] {
Expand Down Expand Up @@ -432,20 +438,23 @@ final class PDFDocumentViewController: UIViewController {
/// - parameter animated: `true` if scrolling is animated, `false` otherwise.
/// - parameter completion: Completion block called after scroll. Block is also called when scroll was not needed.
private func scrollIfNeeded(to pageIndex: PageIndex, animated: Bool, completion: @escaping () -> Void) {
guard self.pdfController?.pageIndex != pageIndex else {
guard let pdfController, pdfController.pageIndex != pageIndex else {
completion()
return
}
let currentPageIndex = pdfController.pageIndex

if !animated {
self.pdfController?.setPageIndex(pageIndex, animated: false)
pdfController.setPageIndex(pageIndex, animated: false)
pdfController.backForwardList.register(PSPDFKit.GoToAction(pageIndex: currentPageIndex))
completion()
return
}

UIView.animate(withDuration: 0.25, animations: {
self.pdfController?.setPageIndex(pageIndex, animated: false)
pdfController.setPageIndex(pageIndex, animated: false)
}, completion: { finished in
pdfController.backForwardList.register(PSPDFKit.GoToAction(pageIndex: currentPageIndex))
guard finished else { return }
completion()
})
Expand Down Expand Up @@ -663,6 +672,7 @@ final class PDFDocumentViewController: UIViewController {
self.setup(scrubberBar: controller.userInterfaceView.scrubberBar)
self.setup(interactions: controller.interactions)
controller.shouldResetAppearanceModeWhenViewDisappears = false
controller.documentViewController?.delegate = self

return controller
}
Expand All @@ -680,6 +690,7 @@ final class PDFDocumentViewController: UIViewController {

scrubberBar.standardAppearance = appearance
scrubberBar.compactAppearance = appearance
scrubberBar.delegate = self
}

private func setup(interactions: DocumentViewInteractions) {
Expand Down Expand Up @@ -912,7 +923,7 @@ extension PDFDocumentViewController: BackForwardActionListDelegate {

func backForwardListDidUpdate(_ list: BackForwardActionList) {
pdfController?.backForwardListDidUpdate(list)
parentDelegate?.backNavigationButtonChanged(visible: list.backAction != nil)
parentDelegate?.navigationButtonsChanged(backVisible: list.backAction != nil, forwardVisible: list.forwardAction != nil)
}
}

Expand Down Expand Up @@ -1171,3 +1182,27 @@ extension UIAction.Identifier {
fileprivate static let pspdfkitAnnotationToolHighlight = UIAction.Identifier(rawValue: "com.pspdfkit.action.annotation-tool-Highlight")
fileprivate static let pspdfkitAnnotationToolUnderline = UIAction.Identifier(rawValue: "com.pspdfkit.action.annotation-tool-Underline")
}

extension PDFDocumentViewController: PDFDocumentViewControllerDelegate {
func documentViewController(_ documentViewController: PSPDFKitUI.PDFDocumentViewController, configureScrollView scrollView: UIScrollView) {
scrollView.delegate = self
}
}

extension PDFDocumentViewController: UIScrollViewDelegate {
func scrollViewShouldScrollToTop(_ scrollView: UIScrollView) -> Bool {
if let pdfController, pdfController.configuration.scrollDirection == .vertical, pdfController.pageIndex != 0 {
pdfController.backForwardList.register(PSPDFKit.GoToAction(pageIndex: pdfController.pageIndex))
}
return true
}
}

extension PDFDocumentViewController: PSPDFKitUI.ScrubberBarDelegate {
func scrubberBar(_ scrubberBar: ScrubberBar, didSelectPageAt pageIndex: PageIndex) {
guard let pdfController, pdfController.pageIndex != pageIndex else { return }
let currentPageIndex = pdfController.pageIndex
pdfController.userInterfaceView.scrubberBar(scrubberBar, didSelectPageAt: pageIndex)
pdfController.backForwardList.register(PSPDFKit.GoToAction(pageIndex: currentPageIndex))
}
}
27 changes: 19 additions & 8 deletions Zotero/Scenes/Detail/PDF/Views/PDFReaderViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class PDFReaderViewController: UIViewController {
private weak var annotationToolbarController: AnnotationToolbarViewController!
private var documentTop: NSLayoutConstraint!
private var annotationToolbarHandler: AnnotationToolbarHandler!
private var intraDocumentNavigationHandler: IntraDocumentNavigationButtonsHandler!
private var intraDocumentNavigationHandler: IntraDocumentNavigationButtonsHandler?
private var selectedText: String?
private(set) var isCompactWidth: Bool
@CodableUserDefault(key: "PDFReaderToolbarState", defaultValue: AnnotationToolbarHandler.State(position: .leading, visible: true), encoder: Defaults.jsonEncoder, decoder: Defaults.jsonDecoder)
Expand Down Expand Up @@ -169,6 +169,12 @@ class PDFReaderViewController: UIViewController {
.init(title: L10n.back, action: #selector(performBackAction), input: UIKeyCommand.inputLeftArrow, modifierFlags: [.command])
]
}
if intraDocumentNavigationHandler?.showsForwardButton == true {
keyCommands += [
.init(title: L10n.back, action: #selector(performForwardAction), input: "]", modifierFlags: [.command]),
.init(title: L10n.back, action: #selector(performForwardAction), input: UIKeyCommand.inputRightArrow, modifierFlags: [.command])
]
}
return keyCommands
}

Expand All @@ -178,7 +184,7 @@ class PDFReaderViewController: UIViewController {
case #selector(UIResponderStandardEditActions.copy(_:)):
return selectedText != nil

case #selector(search), #selector(performBackAction):
case #selector(search), #selector(performBackAction), #selector(performForwardAction):
return true

case #selector(undo(_:)):
Expand Down Expand Up @@ -214,14 +220,16 @@ class PDFReaderViewController: UIViewController {
)
viewModel.process(action: .changeIdleTimerDisabled(true))
view.backgroundColor = .systemGray6
// Create intraDocumentNavigationHandler before setting up views, as it may be called by a child view controller, before view has finished loading.
setupViews()
intraDocumentNavigationHandler = IntraDocumentNavigationButtonsHandler(
parent: self,
parent: documentController,
back: { [weak self] in
self?.documentController?.performBackAction()
},
forward: { [weak self] in
self?.documentController?.performForwardAction()
}
)
setupViews()
setupObserving()

if !viewModel.state.document.isLocked {
Expand All @@ -230,7 +238,6 @@ class PDFReaderViewController: UIViewController {

setupNavigationBar()
updateInterface(to: viewModel.state.settings)
intraDocumentNavigationHandler.bringButtonToFront()

func setupViews() {
let topSafeAreaSpacer = UIView()
Expand Down Expand Up @@ -674,6 +681,10 @@ class PDFReaderViewController: UIViewController {
documentController.performBackAction()
}

@objc private func performForwardAction() {
documentController.performForwardAction()
}

@objc private func undo(_ sender: Any?) {
performUndo()
}
Expand Down Expand Up @@ -928,8 +939,8 @@ extension PDFReaderViewController: PDFDocumentDelegate {
}
}

func backNavigationButtonChanged(visible: Bool) {
intraDocumentNavigationHandler?.set(backButtonVisible: visible)
func navigationButtonsChanged(backVisible: Bool, forwardVisible: Bool) {
intraDocumentNavigationHandler?.set(backButtonVisible: backVisible, forwardButtonVisible: forwardVisible)
}

func didSelectText(_ text: String) {
Expand Down