Improve annotation toolbar logic (#856)

* Improve AnnotationToolbarHandlerDelegate statusBarHeight computation * Fix document top offset for PDF annotation popovers
zotero · Feb 26, 2024 · ddbfb88 · ddbfb88
1 parent c488bc5
commit ddbfb88
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 15 deletions.
diff --git a/Zotero/Scenes/Detail/PDF/Views/AnnotationToolbarHandler.swift b/Zotero/Scenes/Detail/PDF/Views/AnnotationToolbarHandler.swift
@@ -13,7 +13,7 @@ import RxSwift
 protocol AnnotationToolbarHandlerDelegate: AnyObject {
     var toolbarState: AnnotationToolbarHandler.State { get set }
     var statusBarVisible: Bool { get set }
-    var statusBarHeight: CGFloat { get set }
+    var statusBarHeight: CGFloat { get }
     var isNavigationBarHidden: Bool { get }
     var navigationBarHeight: CGFloat { get }
     var isSidebarHidden: Bool { get }

diff --git a/Zotero/Scenes/Detail/PDF/Views/PDFDocumentViewController.swift b/Zotero/Scenes/Detail/PDF/Views/PDFDocumentViewController.swift
@@ -350,7 +350,7 @@ final class PDFDocumentViewController: UIViewController {
 
         let key = annotation.readerKey
         var frame = self.view.convert(annotation.boundingBox(boundingBoxConverter: self), from: pageView.pdfCoordinateSpace)
-        frame.origin.y += (self.parentDelegate?.statusBarHeight ?? 0) + (self.parentDelegate?.navigationBarHeight ?? 0)
+        frame.origin.y += parentDelegate?.documentTopOffset ?? 0
         let observable = self.coordinatorDelegate?.showAnnotationPopover(
             viewModel: self.viewModel,
             sourceRect: frame,

diff --git a/Zotero/Scenes/Detail/PDF/Views/PDFReaderViewController.swift b/Zotero/Scenes/Detail/PDF/Views/PDFReaderViewController.swift
@@ -15,8 +15,7 @@ import RxSwift
 
 protocol PDFReaderContainerDelegate: AnyObject {
     var isSidebarVisible: Bool { get }
-    var statusBarHeight: CGFloat { get }
-    var navigationBarHeight: CGFloat { get }
+    var documentTopOffset: CGFloat { get }
 
     func showSearch(pdfController: PDFViewController, text: String?)
 }
@@ -51,10 +50,6 @@ class PDFReaderViewController: UIViewController {
     private var previousTraitCollection: UITraitCollection?
     var isSidebarVisible: Bool { return sidebarControllerLeft?.constant == 0 }
     var key: String { return viewModel.state.key }
-    var statusBarHeight: CGFloat = .zero
-    var navigationBarHeight: CGFloat {
-        return navigationController?.navigationBar.frame.height ?? 0.0
-    }
 
     weak var coordinatorDelegate: (PdfReaderCoordinatorDelegate & PdfAnnotationsCoordinatorDelegate)?
 
@@ -168,7 +163,6 @@ class PDFReaderViewController: UIViewController {
     override func viewDidLoad() {
         super.viewDidLoad()
 
-        statusBarHeight = (view.scene as? UIWindowScene)?.statusBarManager?.statusBarFrame.height ?? .zero
         set(userActivity: .pdfActivity(for: viewModel.state.key, libraryId: viewModel.state.library.identifier, collectionId: Defaults.shared.selectedCollectionId))
         view.backgroundColor = .systemGray6
         setupViews()
@@ -371,7 +365,6 @@ class PDFReaderViewController: UIViewController {
 
         coordinator.animate { [weak self] _ in
             guard let self else { return }
-            statusBarHeight = view.safeAreaInsets.top - (navigationController?.isNavigationBarHidden == true ? 0 : navigationBarHeight)
             annotationToolbarHandler.viewWillTransitionToNewSize()
         }
     }
@@ -616,13 +609,30 @@ class PDFReaderViewController: UIViewController {
     }
 }
 
-extension PDFReaderViewController: PDFReaderContainerDelegate {}
+extension PDFReaderViewController: PDFReaderContainerDelegate {
+    var documentTopOffset: CGFloat {
+        documentTop.constant
+    }
+}
 
 extension PDFReaderViewController: AnnotationToolbarHandlerDelegate {
+    var statusBarHeight: CGFloat {
+        guard let view = viewIfLoaded else { return 0 }
+        if let statusBarManager = (view.scene as? UIWindowScene)?.statusBarManager, !statusBarManager.isStatusBarHidden {
+            return statusBarManager.statusBarFrame.height
+        } else {
+            return max(view.safeAreaInsets.top - (navigationController?.isNavigationBarHidden == true ? 0 : navigationBarHeight), 0)
+        }
+    }
+
     var isNavigationBarHidden: Bool {
         navigationController?.navigationBar.isHidden ?? false
     }
-
+
+    var navigationBarHeight: CGFloat {
+        return navigationController?.navigationBar.frame.height ?? 0.0
+    }
+
     var isSidebarHidden: Bool {
         !isSidebarVisible
     }
@@ -806,10 +816,9 @@ extension PDFReaderViewController: PDFDocumentDelegate {
             view.layoutIfNeeded()
             if shouldChangeNavigationBarVisibility {
                 navigationController?.navigationBar.alpha = isHidden ? 0 : 1
+                navigationController?.setNavigationBarHidden(isHidden, animated: false)
             }
-        }, completion: { [weak self] finished in
-            guard let self, finished, shouldChangeNavigationBarVisibility else { return }
-            navigationController?.setNavigationBarHidden(isHidden, animated: false)
+            annotationToolbarHandler.interfaceVisibilityDidChange()
         })
 
         if isHidden && isSidebarVisible {