UI improvements, fixed font size picker

Zotero/Controllers/AttributedTagStringGenerator.swift

@@ -50,8 +50,16 @@ struct AttributedTagStringGenerator {
     }
 
     static func attributedString(from tags: [Tag], limit: Int? = nil) -> NSMutableAttributedString {
+        let sorted = tags.sorted { lTag, rTag in
+            if !rTag.color.isEmpty && lTag.color.isEmpty {
+                return false
+            } else if rTag.color.isEmpty && !lTag.color.isEmpty {
+                return true
+            }
+            return lTag.name.localizedCaseInsensitiveCompare(rTag.name) == .orderedAscending
+        }
         let wholeString = NSMutableAttributedString()
-        for (index, tag) in tags.enumerated() {
+        for (index, tag) in sorted.enumerated() {
             if let limit = limit, index == limit {
                 break
             }

Zotero/Scenes/Detail/Annotation Popover/Views/AnnotationViewController.swift

@@ -290,7 +290,7 @@ final class AnnotationViewController: UIViewController {
 
             case .freeText:
                 // Setup font size picker
-                let lineView = FontSizeView(contentInsets: UIEdgeInsets(top: 8, left: 16, bottom: 8, right: 16))
+                let lineView = FontSizeView(contentInsets: UIEdgeInsets(top: 8, left: 16, bottom: 8, right: 16), stepperEnabled: true)
                 lineView.value = annotation.fontSize ?? 0
                 lineView.tapObservable
                     .subscribe(with: self, onNext: { `self`, _ in

Zotero/Scenes/Detail/Annotation Popover/Views/FontSizeCell.swift

@@ -30,7 +30,7 @@ class FontSizeCell: RxTableViewCell {
     }
 
     private func setup() {
-        let fontSizeView = FontSizeView(contentInsets: UIEdgeInsets(top: 8, left: 16, bottom: 8, right: 16))
+        let fontSizeView = FontSizeView(contentInsets: UIEdgeInsets(top: 8, left: 16, bottom: 8, right: 16), stepperEnabled: true)
         fontSizeView.translatesAutoresizingMaskIntoConstraints = false
         fontSizeView.button.isUserInteractionEnabled = false
         self.fontSizeView = fontSizeView

Zotero/Scenes/Detail/Annotation Popover/Views/FontSizePickerViewController.swift

@@ -8,6 +8,8 @@
 
 import UIKit
 
+import RxSwift
+
 class FontSizePickerViewController: UIViewController {
     private static let sizes: [UInt] = [
         10,
@@ -25,12 +27,14 @@ class FontSizePickerViewController: UIViewController {
     ]
 
     private let pickAction: (UInt) -> Void
+    private let disposeBag: DisposeBag
 
     private weak var tableView: UITableView!
     private var dataSource: UITableViewDiffableDataSource<Int, UInt>!
 
     init(pickAction: @escaping (UInt) -> Void) {
         self.pickAction = pickAction
+        self.disposeBag = DisposeBag()
         super.init(nibName: nil, bundle: nil)
     }
 
@@ -41,52 +45,72 @@ class FontSizePickerViewController: UIViewController {
     override func viewDidLoad() {
         super.viewDidLoad()
 
-        self.setupViews()
-        self.setupSizes()
-    }
+        setupNavigationBarIfNeeded()
+        setupViews()
+        setupSizes()
 
-    override func loadView() {
-        self.view = UIView()
-    }
+        func setupSizes() {
+            var snapshot = NSDiffableDataSourceSnapshot<Int, UInt>()
+            snapshot.appendSections([0])
+            snapshot.appendItems(FontSizePickerViewController.sizes)
+            dataSource.apply(snapshot, animatingDifferences: false)
+        }
 
-    private func setupSizes() {
-        var snapshot = NSDiffableDataSourceSnapshot<Int, UInt>()
-        snapshot.appendSections([0])
-        snapshot.appendItems(FontSizePickerViewController.sizes)
-        self.dataSource.apply(snapshot, animatingDifferences: false)
-    }
+        func setupViews() {
+            let tableView = UITableView()
+            tableView.register(UITableViewCell.self, forCellReuseIdentifier: "Cell")
+            dataSource = UITableViewDiffableDataSource(tableView: tableView, cellProvider: { tableView, indexPath, size in
+                let cell = tableView.dequeueReusableCell(withIdentifier: "Cell", for: indexPath)
+                var configuration = cell.defaultContentConfiguration()
+                configuration.text = "\(size)pt"
+                cell.contentConfiguration = configuration
+                return cell
+            })
+            tableView.dataSource = self.dataSource
+            tableView.delegate = self
+            tableView.translatesAutoresizingMaskIntoConstraints = false
+            view.addSubview(tableView)
 
-    private func setupViews() {
-        let tableView = UITableView()
-        tableView.register(UITableViewCell.self, forCellReuseIdentifier: "Cell")
-        self.dataSource = UITableViewDiffableDataSource(tableView: tableView, cellProvider: { tableView, indexPath, size in
-            let cell = tableView.dequeueReusableCell(withIdentifier: "Cell", for: indexPath)
-            var configuration = cell.defaultContentConfiguration()
-            configuration.text = "\(size)pt"
-            cell.contentConfiguration = configuration
-            return cell
-        })
-        tableView.dataSource = self.dataSource
-        tableView.delegate = self
-        tableView.translatesAutoresizingMaskIntoConstraints = false
-        self.view.addSubview(tableView)
+            NSLayoutConstraint.activate([
+                view.leadingAnchor.constraint(equalTo: tableView.leadingAnchor),
+                view.trailingAnchor.constraint(equalTo: tableView.trailingAnchor),
+                view.topAnchor.constraint(equalTo: tableView.topAnchor),
+                view.bottomAnchor.constraint(equalTo: tableView.bottomAnchor)
+            ])
+        }
+
+        func setupNavigationBarIfNeeded() {
+            // Check whether this controller is used in UINavigationController container which only contains this controller. Otherwise if this controller was pushed into navigation stack we don't
+            // need the cancel button.
+            guard let navigationController, navigationController.viewControllers.count == 1 else { return }
+            let cancel = UIBarButtonItem(systemItem: .cancel)
+            cancel
+                .rx
+                .tap
+                .subscribe(with: self, onNext: { `self`, _ in
+                    self.navigationController?.presentingViewController?.dismiss(animated: true)
+                })
+                .disposed(by: disposeBag)
+            self.navigationItem.leftBarButtonItem = cancel
+        }
+    }
 
-        NSLayoutConstraint.activate([
-            self.view.leadingAnchor.constraint(equalTo: tableView.leadingAnchor),
-            self.view.trailingAnchor.constraint(equalTo: tableView.trailingAnchor),
-            self.view.topAnchor.constraint(equalTo: tableView.topAnchor),
-            self.view.bottomAnchor.constraint(equalTo: tableView.bottomAnchor)
-        ])
+    override func loadView() {
+        self.view = UIView()
     }
 }
 
 extension FontSizePickerViewController: UITableViewDelegate {
     func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
         tableView.deselectRow(at: indexPath, animated: true)
-        guard let size = self.dataSource.itemIdentifier(for: indexPath) else { return }
+        guard let size = dataSource.itemIdentifier(for: indexPath) else { return }
         self.pickAction(size)
-        if let controller = self.navigationController {
-            controller.popViewController(animated: true)
+        if let controller = navigationController {
+            if controller.viewControllers.count == 1 {
+                controller.presentingViewController?.dismiss(animated: true)
+            } else {
+                controller.popViewController(animated: true)
+            }
         } else {
             self.presentingViewController?.dismiss(animated: true)
         }

Zotero/Scenes/Detail/Annotation Popover/Views/FontSizeView.swift

@@ -17,6 +17,11 @@ final class FontSizeView: UIView {
     let tapObservable: PublishSubject<()>
     let valueObservable: PublishSubject<UInt>
 
+    var stepperEnabled: Bool {
+        didSet {
+            self.stepper.isHidden = !self.stepperEnabled
+        }
+    }
     private(set) weak var button: UIButton!
     private weak var stepper: UIStepper!
 
@@ -31,8 +36,9 @@ final class FontSizeView: UIView {
         }
     }
 
-    init(contentInsets: UIEdgeInsets) {
+    init(contentInsets: UIEdgeInsets, stepperEnabled: Bool) {
         self.contentInsets = contentInsets
+        self.stepperEnabled = stepperEnabled
         self.disposeBag = DisposeBag()
         self.tapObservable = PublishSubject()
         self.valueObservable = PublishSubject()
@@ -42,6 +48,7 @@ final class FontSizeView: UIView {
 
     required init?(coder: NSCoder) {
         self.contentInsets = UIEdgeInsets(top: 8, left: 16, bottom: 8, right: 16)
+        self.stepperEnabled = true
         self.disposeBag = DisposeBag()
         self.tapObservable = PublishSubject()
         self.valueObservable = PublishSubject()
@@ -70,6 +77,7 @@ final class FontSizeView: UIView {
 
     private func setup() {
         let stepper = UIStepper()
+        stepper.isHidden = !self.stepperEnabled
         stepper.stepValue = 1
         stepper.minimumValue = 1
         stepper.maximumValue = 200

Zotero/Scenes/Detail/PDF/PDFCoordinator.swift

@@ -393,10 +393,21 @@ extension PDFCoordinator: PdfReaderCoordinatorDelegate {
 
     func showFontSizePicker(sender: UIView, picked: @escaping (UInt) -> Void) {
         let controller = FontSizePickerViewController(pickAction: picked)
-        controller.preferredContentSize = CGSize(width: 200, height: 400)
-        controller.modalPresentationStyle = UIDevice.current.userInterfaceIdiom == .pad ? .popover : .formSheet
-        controller.popoverPresentationController?.sourceView = sender
-        self.navigationController?.present(controller, animated: true)
+        let presentedController: UIViewController
+        switch UIDevice.current.userInterfaceIdiom {
+        case .pad:
+            controller.modalPresentationStyle = .popover
+            controller.popoverPresentationController?.sourceView = sender
+            controller.preferredContentSize = CGSize(width: 200, height: 400)
+            presentedController = controller
+
+        default:
+            let navigationController = UINavigationController(rootViewController: controller)
+            navigationController.modalPresentationStyle = .formSheet
+            presentedController = navigationController
+        }
+
+        self.navigationController?.present(presentedController, animated: true)
     }
 }
 

Zotero/Scenes/Detail/PDF/Views/CustomFreeTextAnnotationView.swift

@@ -39,6 +39,7 @@ final class CustomFreeTextAnnotationView: FreeTextAnnotationView {
 
 final class FreeTextInputAccessory: UIView {
     private weak var delegate: FreeTextInputDelegate?
+    private weak var sizePicker: FontSizeView?
     private let disposeBag: DisposeBag
 
     init(key: PDFReaderState.AnnotationKey, delegate: FreeTextInputDelegate) {
@@ -55,8 +56,9 @@ final class FreeTextInputAccessory: UIView {
         let separator3 = UIView()
         separator3.backgroundColor = .opaqueSeparator
 
-        let sizePicker = FontSizeView(contentInsets: UIEdgeInsets.zero)
+        let sizePicker = FontSizeView(contentInsets: UIEdgeInsets.zero, stepperEnabled: self.traitCollection.horizontalSizeClass != .compact)
         sizePicker.value = delegate.getFontSize(for: key) ?? 0
+        self.sizePicker = sizePicker
         sizePicker.valueObservable
             .observe(on: MainScheduler.instance)
             .subscribe(with: self, onNext: { `self`, value in
@@ -75,25 +77,16 @@ final class FreeTextInputAccessory: UIView {
             .disposed(by: self.disposeBag)
 
         let colorButton = UIButton()
-        let deleteButton = UIButton()
-
-        if #available(iOS 15.0, *) {
-            var colorConfiguration = UIButton.Configuration.plain()
-            colorConfiguration.contentInsets = NSDirectionalEdgeInsets(top: 0, leading: 20, bottom: 0, trailing: 20)
-            colorConfiguration.image = UIImage(systemName: "circle.fill", withConfiguration: UIImage.SymbolConfiguration(scale: .large))
-            colorButton.configuration = colorConfiguration
-
-            var deleteConfiguration = UIButton.Configuration.plain()
-            deleteConfiguration.image = UIImage(systemName: "trash", withConfiguration: UIImage.SymbolConfiguration(scale: .large))
-            deleteConfiguration.contentInsets = NSDirectionalEdgeInsets(top: 0, leading: 20, bottom: 0, trailing: 20)
-            deleteButton.configuration = deleteConfiguration
-        } else {
-            colorButton.setImage(UIImage(systemName: "circle.fill", withConfiguration: UIImage.SymbolConfiguration(scale: .large)), for: .normal)
-            colorButton.contentEdgeInsets = UIEdgeInsets(top: 0, left: 20, bottom: 0, right: 20)
+        var colorConfiguration = UIButton.Configuration.plain()
+        colorConfiguration.contentInsets = NSDirectionalEdgeInsets(top: 0, leading: 20, bottom: 0, trailing: 20)
+        colorConfiguration.image = UIImage(systemName: "circle.fill", withConfiguration: UIImage.SymbolConfiguration(scale: .large))
+        colorButton.configuration = colorConfiguration
 
-            deleteButton.setImage(UIImage(systemName: "trash", withConfiguration: UIImage.SymbolConfiguration(scale: .large)), for: .normal)
-            deleteButton.contentEdgeInsets = UIEdgeInsets(top: 0, left: 20, bottom: 0, right: 20)
-        }
+        let deleteButton = UIButton()
+        var deleteConfiguration = UIButton.Configuration.plain()
+        deleteConfiguration.image = UIImage(systemName: "trash", withConfiguration: UIImage.SymbolConfiguration(scale: .large))
+        deleteConfiguration.contentInsets = NSDirectionalEdgeInsets(top: 0, leading: 20, bottom: 0, trailing: 20)
+        deleteButton.configuration = deleteConfiguration
 
         colorButton.tintColor = self.delegate?.getColor(for: key) ?? Asset.Colors.zoteroBlueWithDarkMode.color
         colorButton.rx.tap
@@ -168,19 +161,12 @@ final class FreeTextInputAccessory: UIView {
         }
     }
 
-    @available(iOS 15, *)
-    private func attributedString(from tags: [Tag]) -> AttributedString {
-        if tags.isEmpty {
-            var string = AttributedString(L10n.Pdf.AnnotationsSidebar.addTags)
-            string.foregroundColor = Asset.Colors.zoteroBlueWithDarkMode.color
-            return string
-        } else {
-            let nsAttributedString = AttributedTagStringGenerator.attributedString(from: tags, limit: 3)
-            return (try? AttributedString(nsAttributedString, including: \.uiKit)) ?? AttributedString()
-        }
-    }
-
     required init?(coder: NSCoder) {
         fatalError("init(coder:) has not been implemented")
     }
+
+    override func traitCollectionDidChange(_ previousTraitCollection: UITraitCollection?) {
+        super.traitCollectionDidChange(previousTraitCollection)
+        self.sizePicker?.stepperEnabled = self.traitCollection.horizontalSizeClass != .compact
+    }
 }