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.

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

Fix crash upon SwiftUICollectionViewCell deinit and add some more breadcrumbs and optimization in HomeViewModel snapshot calculation and rendering #1053

Merged
merged 5 commits into from
Jul 29, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ private extension CollectionViewController {
return cell
case .error:
let cell: EmptyStateCollectionViewCell = collectionView.dequeueCell(for: indexPath)
cell.configure(parent: self, model.errorEmptyState)
cell.configure(viewModel: model.errorEmptyState)
return cell
}
}
Expand Down
11 changes: 6 additions & 5 deletions PocketKit/Sources/PocketKit/Home/HomeViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ struct HomeViewControllerSwiftUI: UIViewControllerRepresentable {

func makeUIViewController(context: UIViewControllerRepresentableContext<Self>) -> UINavigationController {
let homeViewController = HomeViewController(model: model)
homeViewController.updateHeroCardCount()
homeViewController.updateLayout()
let navigationController = UINavigationController(rootViewController: homeViewController)
navigationController.navigationBar.prefersLargeTitles = true
navigationController.navigationBar.barTintColor = UIColor(.ui.white1)
Expand Down Expand Up @@ -154,6 +154,7 @@ class HomeViewController: UIViewController {
return
}
dataSource.apply(snapshot)
Log.breadcrumb(category: "home", level: .debug, message: "➡️ Applying snapshot - #sections: \(snapshot.numberOfSections), #items: \(snapshot.numberOfItems)")
}.store(in: &subscriptions)
}

Expand Down Expand Up @@ -216,14 +217,14 @@ class HomeViewController: UIViewController {
}

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

func updateHeroCardCount() {
fileprivate func updateLayout() {
if traitCollection.shouldUseWideLayout() {
self.model.numberOfHeroItems = 2
model.useWideLayout()
} else {
self.model.numberOfHeroItems = 1
model.useCompactLayout()
}
}
}
Expand Down
30 changes: 24 additions & 6 deletions PocketKit/Sources/PocketKit/Home/HomeViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,7 @@ class HomeViewModel: NSObject {

@Published var tappedSeeAll: SeeAll?

var numberOfHeroItems: Int = 1 {
didSet {
self.snapshot = buildSnapshot()
}
}
private var numberOfHeroItems: Int = 1

private let source: Source
let tracker: Tracker
Expand Down Expand Up @@ -311,6 +307,27 @@ extension HomeViewModel {
}
return snapshot
}

/// Updates the collection view layout for compact or wide layout, if this changed.
/// Wide layout has two columns and two hero items per recommendation section.
/// - Parameter heroItems: the number of hero items to use.
private func updateLayout(_ heroItems: Int) {
guard heroItems != numberOfHeroItems else {
return
}
numberOfHeroItems = heroItems
snapshot = buildSnapshot()
}

/// Updates the layout to wide, if the previous layout was compact.
func useWideLayout() {
updateLayout(2)
}

/// Updates the layout to compact, if the previpus layout was wide.
func useCompactLayout() {
updateLayout(1)
}
}

// MARK: - Cell Selection
Expand Down Expand Up @@ -618,10 +635,11 @@ extension HomeViewModel {

// MARK: - Loading Section
extension HomeViewModel {
static func loadingSnapshot() -> Snapshot {
private static func loadingSnapshot() -> Snapshot {
var snapshot = Snapshot()
snapshot.appendSections([.loading])
snapshot.appendItems([.loading], toSection: .loading)
Log.breadcrumb(category: "home", level: .debug, message: "➡️ Sending loading snapshot.")
return snapshot
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,37 +6,33 @@ import SwiftUI
import Textile

open class SwiftUICollectionViewCell<T>: UICollectionViewCell where T: View {
private(set) var hosting: UIHostingController<T>?

func embed(in parent: UIViewController, withView content: T) {
if let hosting = self.hosting {
hosting.rootView = content
hosting.view.layoutIfNeeded()
} else {
let hosting = UIHostingController(rootView: content)
parent.addChild(hosting)
hosting.didMove(toParent: parent)
self.contentView.addSubview(hosting.view)
self.hosting = hosting
}
}

deinit {
Task { @MainActor in
hosting?.willMove(toParent: nil)
hosting?.view.removeFromSuperview()
hosting?.removeFromParent()
hosting = nil
}
/// Conveets a `SwiftUI View` in a `UIKit UIView`
/// - Parameters:
/// - content: the `SwiftUI View`
/// - Returns: the `UIKit View` that embeds the original `SwiftUI View`
func uiView(from content: T) -> UIView {
let controller = UIHostingController(rootView: content)
controller.view.translatesAutoresizingMaskIntoConstraints = false
controller.view.backgroundColor = .clear
return controller.view
}
}

class EmptyStateCollectionViewCell: SwiftUICollectionViewCell<EmptyStateView<EmptyView>> {
func configure(parent: UIViewController, _ viewModel: EmptyStateViewModel) {
embed(in: parent, withView: EmptyStateView(viewModel: viewModel))
hosting?.view.frame = self.contentView.bounds
hosting?.view.backgroundColor = .clear
hosting?.view.accessibilityIdentifier = viewModel.accessibilityIdentifier
func configure(viewModel: EmptyStateViewModel) {
let view = uiView(from: EmptyStateView(viewModel: viewModel))
contentView.addSubview(view)
contentView.pinSubviewToAllEdges(view)
view.accessibilityIdentifier = viewModel.accessibilityIdentifier
}

override func prepareForReuse() {
// default implementation does nothing, adding it here just in case it changes in the future
super.prepareForReuse()
// clear up any existing content from the view before adding one
contentView.subviews.forEach {
$0.removeFromSuperview()
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ class ItemsListViewController<ViewModel: ItemsListViewModel>: UIViewController,
guard let viewModel = model.emptyState else {
return
}
cell.configure(parent: self, viewModel)
cell.configure(viewModel: viewModel)
}

private func handle(savesEvent event: ItemsListEvent<ViewModel.ItemIdentifier>) {
Expand Down