Skip to content

Commit

Permalink
Improve PDF reader selection menu (#1006)
Browse files Browse the repository at this point in the history 
* Improve code

* Fix coordinates for lookup/share popup from selected PDF text

* Add PDF text selection menu action for creating underline annotation

* Improve code

* Use PDF reader's style for text selection lookup/share
  • Loading branch information
@mvasilak
mvasilak authored Sep 10, 2024
1 parent 6b2f0db commit d179556
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 58 deletions.
19 changes: 17 additions & 2 deletions Zotero/Controllers/Architecture/Coordinator.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,13 @@ protocol Coordinator: AnyObject {

func start(animated: Bool)
func childDidFinish(_ child: Coordinator)
func share(item: Any, sourceView: SourceView, presenter: UIViewController?, completionWithItemsHandler: UIActivityViewController.CompletionWithItemsHandler?)
func share(
item: Any,
sourceView: SourceView,
presenter: UIViewController?,
userInterfaceStyle: UIUserInterfaceStyle?,
completionWithItemsHandler: UIActivityViewController.CompletionWithItemsHandler?
)
}

extension Coordinator {
Expand All @@ -36,8 +42,17 @@ extension Coordinator {
}
}

func share(item: Any, sourceView: SourceView, presenter: UIViewController? = nil, completionWithItemsHandler: UIActivityViewController.CompletionWithItemsHandler? = nil) {
func share(
item: Any,
sourceView: SourceView,
presenter: UIViewController? = nil,
userInterfaceStyle: UIUserInterfaceStyle? = nil,
completionWithItemsHandler: UIActivityViewController.CompletionWithItemsHandler? = nil
) {
let controller = UIActivityViewController(activityItems: [item], applicationActivities: nil)
if let userInterfaceStyle {
controller.overrideUserInterfaceStyle = userInterfaceStyle
}
controller.modalPresentationStyle = .pageSheet
controller.completionWithItemsHandler = completionWithItemsHandler

Expand Down
1 change: 1 addition & 0 deletions Zotero/Scenes/Detail/PDF/Models/PDFReaderAction.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ enum PDFReaderAction {
case createNote(pageIndex: PageIndex, origin: CGPoint)
case createImage(pageIndex: PageIndex, origin: CGPoint)
case createHighlight(pageIndex: PageIndex, rects: [CGRect])
case createUnderline(pageIndex: PageIndex, rects: [CGRect])
case parseAndCacheText(key: String, text: String, font: UIFont)
case parseAndCacheComment(key: String, comment: String)
case setComment(key: String, comment: NSAttributedString)
Expand Down
10 changes: 6 additions & 4 deletions Zotero/Scenes/Detail/PDF/PDFCoordinator.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ protocol PdfReaderCoordinatorDelegate: AnyObject {
func show(error: PDFReaderState.Error)
func show(error: PDFDocumentExporter.Error)
func share(url: URL, barButton: UIBarButtonItem)
func share(text: String, rect: CGRect, view: UIView)
func share(text: String, rect: CGRect, view: UIView, userInterfaceStyle: UIUserInterfaceStyle)
func lookup(text: String, rect: CGRect, view: UIView, userInterfaceStyle: UIUserInterfaceStyle)
func showDeletedAlertForPdf(completion: @escaping (Bool) -> Void)
func showSettings(with settings: PDFSettings, sender: UIBarButtonItem) -> ViewModel<ReaderSettingsActionHandler>
Expand Down Expand Up @@ -293,13 +293,15 @@ extension PDFCoordinator: PdfReaderCoordinatorDelegate {
self.share(item: url, sourceView: .item(barButton))
}

func share(text: String, rect: CGRect, view: UIView) {
self.share(item: text, sourceView: .view(view, rect))
func share(text: String, rect: CGRect, view: UIView, userInterfaceStyle: UIUserInterfaceStyle) {
self.share(item: text, sourceView: .view(view, rect), userInterfaceStyle: userInterfaceStyle)
}

func lookup(text: String, rect: CGRect, view: UIView, userInterfaceStyle: UIUserInterfaceStyle) {
DDLogInfo("PDFCoordinator: show lookup")
let controller = UIReferenceLibraryViewController(term: text)
// When presented as a popover, UIReferenceLibraryViewController ignores overrideUserInterfaceStyle, so we wrap it in a navigation controller to force it.
let controller = UINavigationController(rootViewController: UIReferenceLibraryViewController(term: text))
controller.setNavigationBarHidden(true, animated: false)
controller.overrideUserInterfaceStyle = userInterfaceStyle
controller.modalPresentationStyle = .popover
controller.popoverPresentationController?.sourceView = view
Expand Down
27 changes: 15 additions & 12 deletions Zotero/Scenes/Detail/PDF/ViewModels/PDFReaderActionHandler.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,10 @@ final class PDFReaderActionHandler: ViewModelActionHandler, BackgroundDbProcessi
addNote(onPage: pageIndex, origin: origin, in: viewModel)

case .createHighlight(let pageIndex, let rects):
addHighlight(onPage: pageIndex, rects: rects, in: viewModel)
addHighlightOrUnderline(isHighlight: true, onPage: pageIndex, rects: rects, in: viewModel)

case .createUnderline(let pageIndex, let rects):
addHighlightOrUnderline(isHighlight: false, onPage: pageIndex, rects: rects, in: viewModel)

case .setVisiblePage(let page, let userActionFromDocument, let fromThumbnailList):
set(page: page, userActionFromDocument: userActionFromDocument, fromThumbnailList: fromThumbnailList, in: viewModel)
Expand Down Expand Up @@ -1182,22 +1185,22 @@ final class PDFReaderActionHandler: ViewModelActionHandler, BackgroundDbProcessi
}
}

private func addHighlight(onPage pageIndex: PageIndex, rects: [CGRect], in viewModel: ViewModel<PDFReaderActionHandler>) {
guard let activeColor = viewModel.state.toolColors[tool(from: .highlight)] else { return }
private func addHighlightOrUnderline(isHighlight: Bool, onPage pageIndex: PageIndex, rects: [CGRect], in viewModel: ViewModel<PDFReaderActionHandler>) {
guard let activeColor = viewModel.state.toolColors[tool(from: isHighlight ? .highlight : .underline)] else { return }
let (color, alpha, blendMode) = AnnotationColorGenerator.color(from: activeColor, isHighlight: true, userInterfaceStyle: viewModel.state.interfaceStyle)

let highlight = HighlightAnnotation()
highlight.rects = rects
highlight.boundingBox = AnnotationBoundingBoxCalculator.boundingBox(from: rects)
highlight.alpha = alpha
highlight.color = color
let annotation = isHighlight ? HighlightAnnotation() : UnderlineAnnotation()
annotation.rects = rects
annotation.boundingBox = AnnotationBoundingBoxCalculator.boundingBox(from: rects)
annotation.alpha = alpha
annotation.color = color
if let blendMode {
highlight.blendMode = blendMode
annotation.blendMode = blendMode
}
highlight.pageIndex = pageIndex
annotation.pageIndex = pageIndex

viewModel.state.document.undoController.recordCommand(named: nil, adding: [highlight]) {
viewModel.state.document.add(annotations: [highlight], options: nil)
viewModel.state.document.undoController.recordCommand(named: nil, adding: [annotation]) {
viewModel.state.document.add(annotations: [annotation], options: nil)
}
}

Expand Down
102 changes: 62 additions & 40 deletions Zotero/Scenes/Detail/PDF/Views/PDFDocumentViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -731,9 +731,9 @@ extension PDFDocumentViewController: PDFViewControllerDelegate {
}

func pdfViewController(_ sender: PDFViewController, menuForText glyphs: GlyphSequence, onPageView pageView: PDFPageView, appearance: EditMenuAppearance, suggestedMenu: UIMenu) -> UIMenu {
return self.filterActions(
return filterActions(
forMenu: suggestedMenu,
predicate: { menuId, action -> UIAction? in
predicate: { menuId, action -> UIMenuElement? in
switch menuId {
case .standardEdit:
switch action.identifier {
Expand All @@ -752,40 +752,44 @@ extension PDFDocumentViewController: PDFViewControllerDelegate {
case .share:
guard action.identifier == .PSPDFKit.share else { return nil }
return action.replacing(handler: { [weak self] _ in
guard let view = self?.pdfController?.view else { return }
self?.coordinatorDelegate?.share(text: glyphs.text, rect: glyphs.boundingBox, view: view)
guard let self else { return }
coordinatorDelegate?.share(
text: glyphs.text,
rect: pageView.convert(glyphs.boundingBox, from: pageView.pdfCoordinateSpace),
view: pageView,
userInterfaceStyle: viewModel.state.settings.appearanceMode.userInterfaceStyle
)
})

case .pspdfkitActions:
switch action.identifier {
case .PSPDFKit.define:
return action.replacing(title: L10n.lookUp, handler: { [weak self] _ in
guard let self, let view = self.pdfController?.view else { return }
self.coordinatorDelegate?.lookup(
guard let self else { return }
coordinatorDelegate?.lookup(
text: glyphs.text,
rect: glyphs.boundingBox,
view: view,
userInterfaceStyle: self.viewModel.state.settings.appearanceMode.userInterfaceStyle
rect: pageView.convert(glyphs.boundingBox, from: pageView.pdfCoordinateSpace),
view: pageView,
userInterfaceStyle: viewModel.state.settings.appearanceMode.userInterfaceStyle
)
})

case .PSPDFKit.searchDocument:
return action.replacing(handler: { [weak self] _ in
guard let self, let pdfController = self.pdfController else { return }
self.parentDelegate?.showSearch(pdfController: pdfController, text: glyphs.text)
guard let self, let pdfController else { return }
parentDelegate?.showSearch(pdfController: pdfController, text: glyphs.text)
})

default:
return action
}

case .PSPDFKit.annotate:
let rects = pageView.selectionView.selectionRects.map({ pageView.convert($0.cgRectValue, to: pageView.pdfCoordinateSpace) })
return action.replacing(title: L10n.Pdf.highlight, handler: { [weak self] _ in
guard let self else { return }
self.viewModel.process(action: .createHighlight(pageIndex: pageView.pageIndex, rects: rects))
pageView.selectionView.selectedGlyphs = nil
})
let actions = [
action.replacing(title: L10n.Pdf.highlight, handler: createHighlightActionHandler(for: pageView, in: viewModel)),
UIAction(title: L10n.Pdf.underline, identifier: .underline, handler: createUnderlineActionHandler(for: pageView, in: viewModel))
]
return UIMenu(options: [.displayInline], children: actions)

default:
return action
Expand All @@ -794,41 +798,55 @@ extension PDFDocumentViewController: PDFViewControllerDelegate {
populatingEmptyMenu: { menu -> [UIAction]? in
switch menu.identifier {
case .PSPDFKit.annotate:
let rects = pageView.selectionView.selectionRects.map({ pageView.convert($0.cgRectValue, to: pageView.pdfCoordinateSpace) })
return [
UIAction(title: L10n.Pdf.highlight, handler: { [weak self] _ in
guard let self else { return }
self.viewModel.process(action: .createHighlight(pageIndex: pageView.pageIndex, rects: rects))
pageView.selectionView.selectedGlyphs = nil
})
UIAction(title: L10n.Pdf.highlight, handler: createHighlightActionHandler(for: pageView, in: viewModel)),
UIAction(title: L10n.Pdf.underline, identifier: .underline, handler: createUnderlineActionHandler(for: pageView, in: viewModel))
]

default:
return nil
}
}
)
}

private func filterActions(forMenu menu: UIMenu, predicate: (UIMenu.Identifier, UIAction) -> UIAction?, populatingEmptyMenu: (UIMenu) -> [UIAction]?) -> UIMenu {
return menu.replacingChildren(menu.children.compactMap { element in
if let action = element as? UIAction {
if let action = predicate(menu.identifier, action) {
return action
} else {
return nil
}
} else if let menu = element as? UIMenu {
if menu.children.isEmpty {
return populatingEmptyMenu(menu).flatMap({ menu.replacingChildren($0) }) ?? menu
func filterActions(forMenu menu: UIMenu, predicate: (UIMenu.Identifier, UIAction) -> UIMenuElement?, populatingEmptyMenu: (UIMenu) -> [UIAction]?) -> UIMenu {
return menu.replacingChildren(menu.children.compactMap { element -> UIMenuElement? in
if let action = element as? UIAction {
if let element = predicate(menu.identifier, action) {
return element
} else {
return nil
}
} else if let menu = element as? UIMenu {
if menu.children.isEmpty {
return populatingEmptyMenu(menu).flatMap({ menu.replacingChildren($0) }) ?? menu
} else {
// Filter children of submenus recursively.
return filterActions(forMenu: menu, predicate: predicate, populatingEmptyMenu: populatingEmptyMenu)
}
} else {
// Filter children of submenus recursively.
return self.filterActions(forMenu: menu, predicate: predicate, populatingEmptyMenu: populatingEmptyMenu)
return element
}
} else {
return element
})
}

func createHighlightActionHandler(for pageView: PDFPageView, in viewModel: ViewModel<PDFReaderActionHandler>) -> UIActionHandler {
let rects = pageView.selectionView.selectionRects.map({ pageView.convert($0.cgRectValue, to: pageView.pdfCoordinateSpace) })
return { [weak viewModel] _ in
guard let viewModel else { return }
viewModel.process(action: .createHighlight(pageIndex: pageView.pageIndex, rects: rects))
pageView.selectionView.selectedGlyphs = nil
}
})
}

func createUnderlineActionHandler(for pageView: PDFPageView, in viewModel: ViewModel<PDFReaderActionHandler>) -> UIActionHandler {
let rects = pageView.selectionView.selectionRects.map({ pageView.convert($0.cgRectValue, to: pageView.pdfCoordinateSpace) })
return { [weak viewModel] _ in
guard let viewModel else { return }
viewModel.process(action: .createUnderline(pageIndex: pageView.pageIndex, rects: rects))
pageView.selectionView.selectedGlyphs = nil
}
}
}

func pdfViewController(_ pdfController: PDFViewController, didSelectText text: String, with glyphs: [Glyph], at rect: CGRect, on pageView: PDFPageView) {
Expand Down Expand Up @@ -1102,3 +1120,7 @@ extension UIAction {
)
}
}

extension UIAction.Identifier {
fileprivate static let underline = UIAction.Identifier(rawValue: "org.zotero.menu")
}

0 comments on commit d179556

Please sign in to comment.