jessesquires / ReactiveCollectionsKit

Data-driven, declarative, reactive, diffable collections (and lists!) for iOS. A modern, fast, and flexible library for UICollectionView done right.
https://jessesquires.github.io/ReactiveCollectionsKit/
MIT License
84 stars 3 forks source link

[Delegate] implement `willDisplay` + `didEndDisplay` APIs #96

Closed jessesquires closed 2 months ago

jessesquires commented 4 months ago

Docs: https://developer.apple.com/documentation/uikit/uicollectionviewdelegate#1656776

nuomi1 commented 2 months ago

I've thought of two ways to implement these APIs, which one should we use?

// Version A

// new base coordinator
// without this, if a `UIViewController` conforms to both `CellEventCoordinator` and `SupplementaryViewEventCoordinator` it will build error
protocol EventCoordinator: AnyObject {
    var underlyingViewController: UIViewController? { get }
}

// change from `AnyObject` to `EventCoordinator`
protocol CellEventCoordinator: EventCoordinator {
    func willDisplayCell(viewModel: any CellViewModel) // new
    func didEndDisplayingCell(viewModel: any CellViewModel) // new
}

protocol CellViewModel {
    func willDisplay(with coordinator: CellEventCoordinator?) // new
    func didEndDisplaying(with coordinator: CellEventCoordinator?) // new
}

// new
protocol SupplementaryViewEventCoordinator: EventCoordinator {
    func willDisplaySupplementaryView(viewModel: any SupplementaryViewModel)
    func didEndDisplayingSupplementaryView(viewModel: any SupplementaryViewModel)
}

protocol SupplementaryViewModel {
    func willDisplaySupplementaryView(with coordinator: SupplementaryViewEventCoordinator?) // new
    func didEndDisplayingSupplementaryView(with coordinator: SupplementaryViewEventCoordinator?) // new
}
// Version B

// change from `CellEventCoordinator` to `EventCoordinator`
protocol EventCoordinator: AnyObject {
    var underlyingViewController: UIViewController? { get } // current
    func willDisplayCell(viewModel: any CellViewModel) // new
    func didEndDisplayingCell(viewModel: any CellViewModel) // new
    func willDisplaySupplementaryView(viewModel: any SupplementaryViewModel) // new
    func didEndDisplayingSupplementaryView(viewModel: any SupplementaryViewModel) // new
}

protocol CellViewModel {
    func willDisplay(with coordinator: EventCoordinator?) // new
    func didEndDisplaying(with coordinator: EventCoordinator?) // new
}

protocol SupplementaryViewModel {
    func willDisplaySupplementaryView(with coordinator: EventCoordinator?) // new
    func didEndDisplayingSupplementaryView(with coordinator: EventCoordinator?) // new
}
jessesquires commented 2 months ago

Hey @nuomi1 thanks for discussing first! ☺️

So, my thinking about these APIs was even simpler:

protocol CellViewModel {
    func willDisplay() // new
    func didEndDisplaying() // new
}

protocol SupplementaryViewModel {
    func willDisplay() // new
    func didEndDisplaying() // new
}

Then, for both we can add default implementations that do nothing.

At least for now, I don't think we need to involve the EventCoordinator at all. (I'm open to possibly changing this in the future and implementing one of your suggestions.)


Some context about CellEventCoordinator:

First, this is intended for managing user-interaction events. (Thus, display events and highlight events are not part of this.)

More importantly, CellEventCoordinator primarily exists to provide an "escape hatch" for users to handle navigation and presentation events in response to cell selection. The library provides multiple options to do this.

You could architect your app in a way where the CellViewModel handles everything. Maybe didSelect() dispatches an action (using the FLUX architecture pattern), in which case the user will ignore everything about CellEventCoordinator.

Or, some users will likely want to simply forward didSelect() to their view controller and then call self.navigationController.pushViewController(...) or self.present(...). In this scenario, the view controller could implement CellEventCoordinator and do all the navigation. Alternatively, the CellViewModel could implement the navigation in didSelect() by accessing the .underlyingViewController property.

Examples:


The motivations for providing this flexibility are:

  1. I don't want the library to have a strong "opinion" about this. (cell selection / user interaction)
  2. If CellEventCoordinator did not exist and the user is not using something like FLUX, then they would have to pass their ViewController into their CellViewModel as a property in order to implement didSelect() and that's extremely cumbersome and awkward.