Skip to content

Commit

Permalink
Fix size class transition issues and related crashes (#762)
Browse files Browse the repository at this point in the history
  • Loading branch information
mvasilak authored Sep 12, 2023
1 parent 8eecbc7 commit a8d4811
Show file tree
Hide file tree
Showing 12 changed files with 470 additions and 403 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,8 @@ class AnnotationsFilterViewController: UIViewController {
self.setupClearButton(visible: (!state.colors.isEmpty || !state.tags.isEmpty))
}

if UIDevice.current.userInterfaceIdiom == .pad {
self.updateFilter()
self.updatePreferredContentSize()
}
self.updateFilter()
self.updatePreferredContentSize()
}

private func updatePreferredContentSize() {
Expand Down Expand Up @@ -104,9 +102,7 @@ class AnnotationsFilterViewController: UIViewController {
}

private func close() {
if UIDevice.current.userInterfaceIdiom == .phone {
self.updateFilter()
}
self.updateFilter()
self.navigationController?.presentingViewController?.dismiss(animated: true)
}

Expand Down Expand Up @@ -149,14 +145,12 @@ class AnnotationsFilterViewController: UIViewController {
}

private func setupNavigationBar() {
if UIDevice.current.userInterfaceIdiom == .phone {
let close = UIBarButtonItem(title: L10n.close, style: .plain, target: nil, action: nil)
close.rx.tap.subscribe(onNext: { [weak self] _ in
self?.close()
})
.disposed(by: self.disposeBag)
self.navigationItem.leftBarButtonItem = close
}
let close = UIBarButtonItem(title: L10n.close, style: .plain, target: nil, action: nil)
close.rx.tap.subscribe(onNext: { [weak self] _ in
self?.close()
})
.disposed(by: self.disposeBag)
self.navigationItem.leftBarButtonItem = close

self.setupClearButton(visible: (!self.viewModel.state.colors.isEmpty || !self.viewModel.state.tags.isEmpty))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,7 @@ final class ItemDetailCollectionViewHandler: NSObject {
private func setupCollectionView() {
self.collectionView.collectionViewLayout = self.createCollectionViewLayout()
self.collectionView.delegate = self
// keyboardDismissMode is device based, regardless of horizontal size class.
self.collectionView.keyboardDismissMode = UIDevice.current.userInterfaceIdiom == .phone ? .interactive : .none
self.collectionView.register(UINib(nibName: "ItemDetailSectionView", bundle: nil), forSupplementaryViewOfKind: UICollectionView.elementKindSectionHeader, withReuseIdentifier: "Header")
self.collectionView.isEditing = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ final class ItemDetailTitleContentView: UIView {

let font = self.textView.font!
self.topConstraint.constant = font.capHeight - font.ascender
switch UIDevice.current.userInterfaceIdiom {
case .pad:
self.bottomConstraint.constant = -font.descender
default: break
}
setupBottomConstraint()
}

override func traitCollectionDidChange(_ previousTraitCollection: UITraitCollection?) {
setupBottomConstraint()
}

override func layoutSubviews() {
Expand All @@ -49,4 +49,13 @@ final class ItemDetailTitleContentView: UIView {
self.textView.isEditable = isEditing
self.delegate.set(text: title, to: self.textView)
}

private func setupBottomConstraint() {
if traitCollection.horizontalSizeClass == .regular && UIDevice.current.userInterfaceIdiom == .pad {
let font = textView.font!
bottomConstraint.constant = -font.descender
} else {
bottomConstraint.constant = 0
}
}
}
34 changes: 17 additions & 17 deletions Zotero/Scenes/Detail/Items/ViewModels/ItemsToolbarController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import UIKit
import RealmSwift
import RxSwift

protocol ItemsToolbarControllerDelegate: AnyObject {
protocol ItemsToolbarControllerDelegate: UITraitEnvironment {
func process(action: ItemAction.Kind, button: UIBarButtonItem)
}

Expand Down Expand Up @@ -63,7 +63,7 @@ final class ItemsToolbarController {
self.viewController.toolbarItems = self.createEditingToolbarItems(from: self.editingActions)
self.updateEditingToolbarItems(for: state.selectedItems, results: state.results)
} else {
let filters = self.deviceSpecificFilters(from: state.filters)
let filters = self.sizeClassSpecificFilters(from: state.filters)
self.viewController.toolbarItems = self.createNormalToolbarItems(for: filters)
self.updateNormalToolbarItems(for: filters, downloadBatchData: state.downloadBatchData, results: state.results)
}
Expand All @@ -73,26 +73,26 @@ final class ItemsToolbarController {
if state.isEditing {
self.updateEditingToolbarItems(for: state.selectedItems, results: state.results)
} else {
self.updateNormalToolbarItems(for: self.deviceSpecificFilters(from: state.filters), downloadBatchData: state.downloadBatchData, results: state.results)
self.updateNormalToolbarItems(for: self.sizeClassSpecificFilters(from: state.filters), downloadBatchData: state.downloadBatchData, results: state.results)
}
}

private func deviceSpecificFilters(from filters: [ItemsFilter]) -> [ItemsFilter] {
// There is different functionality on iPad and iPhone. iPhone shows tag filters in filter popup in items screen while iPad shows tag filters in master controller.
// So filter icon and description should always show up on iPhone, while it should not show up on iPad for tag filters.
// Therefore we ignore `.tag` filter on iPad and keep it on iPhone.
switch UIDevice.current.userInterfaceIdiom {
case .pad:
return filters.filter({
switch $0 {
case .tags: return false
case .downloadedFiles: return true
}
})

default:
private func sizeClassSpecificFilters(from filters: [ItemsFilter]) -> [ItemsFilter] {
// There is different functionality based on horizontal size class. iPhone and compact iPad show tag filters in filter popup in items screen while iPad shows tag filters in master controller.
// So filter icon and description should always show up on iPhone and compact iPad, while it should not show up on regular iPad for tag filters.
// Therefore we ignore `.tag` filter on iPhone and compact iPad, and keep it on regular iPad.
if delegate?.traitCollection.horizontalSizeClass == .compact || UIDevice.current.userInterfaceIdiom == .phone {
return filters
}
return filters.filter({
switch $0 {
case .tags:
return false

case .downloadedFiles:
return true
}
})
}

// MARK: - Helpers
Expand Down
44 changes: 35 additions & 9 deletions Zotero/Scenes/Detail/Items/Views/ItemsFilterViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ class ItemsFilterViewController: UIViewController {
self.update(state: state)
})
.disposed(by: self.disposeBag)

parent?.presentationController?.delegate = self
}

override func viewWillAppear(_ animated: Bool) {
super.viewWillAppear(animated)

guard UIDevice.current.userInterfaceIdiom == .pad else { return }

var preferredSize = self.container.systemLayoutSizeFitting(CGSize(width: ItemsFilterViewController.width, height: .greatestFiniteMagnitude))
preferredSize.width = ItemsFilterViewController.width
self.preferredContentSize = preferredSize
Expand Down Expand Up @@ -98,13 +98,6 @@ class ItemsFilterViewController: UIViewController {
self.downloadsTitleLabel.text = L10n.Items.Filters.downloads
self.downloadsSwitch.isOn = self.downloadsFilterEnabled

guard UIDevice.current.userInterfaceIdiom == .phone else {
self.tagFilterControllerContainer.isHidden = true
self.separator.isHidden = true
return
}

self.containerTop.constant = 4
self.tagFilterController.willMove(toParent: self)
self.tagFilterControllerContainer.addSubview(self.tagFilterController.view)
self.addChild(self.tagFilterController)
Expand All @@ -117,5 +110,38 @@ class ItemsFilterViewController: UIViewController {
self.tagFilterControllerContainer.bottomAnchor.constraint(equalTo: self.tagFilterController.view.bottomAnchor),
self.separator.heightAnchor.constraint(equalToConstant: 1 / UIScreen.main.scale)
])

showHideTagFilter()
}

func showHideTagFilter() {
let isCollapsed = (presentingViewController as? MainViewController)?.isCollapsed
if UIDevice.current.userInterfaceIdiom == .phone || isCollapsed == true {
tagFilterControllerContainer.isHidden = false
separator.isHidden = false
containerTop.constant = 4
} else {
tagFilterControllerContainer.isHidden = true
separator.isHidden = true
containerTop.constant = 15
}
}
}

extension ItemsFilterViewController: UIAdaptivePresentationControllerDelegate {
func presentationController(
_ presentationController: UIPresentationController,
willPresentWithAdaptiveStyle style: UIModalPresentationStyle,
transitionCoordinator: UIViewControllerTransitionCoordinator?
) {
if let transitionCoordinator {
transitionCoordinator.animate { _ in
self.showHideTagFilter()
}
} else {
UIView.animate(withDuration: 0.1) {
self.showHideTagFilter()
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ final class ItemsTableViewHandler: NSObject {
self.tableView.rowHeight = UITableView.automaticDimension
self.tableView.estimatedRowHeight = 60
self.tableView.allowsMultipleSelectionDuringEditing = true
// keyboardDismissMode is device based, regardless of horizontal size class.
self.tableView.keyboardDismissMode = UIDevice.current.userInterfaceIdiom == .phone ? .interactive : .none
self.tableView.shouldGroupAccessibilityChildren = true

Expand Down
Loading

0 comments on commit a8d4811

Please sign in to comment.