malcommac / FlowKit

A declarative type-safe framework for building fast and flexible list with Tables & Collection
MIT License
211 stars 30 forks source link

TableAdapter and CollectionAdapter are now open for sublclassing #6

Closed wow-such-amazing closed 6 years ago

wow-such-amazing commented 6 years ago

My suggestion is to add possibility to subclass TableAdapter or CollectionAdapter in order to create our own subclasses for specific cells and models. This way we will be able to separate this logic from the controller to different class and reuse this class everywhere. Of course, we can create a factory for often used adapters and return them from it. But it seems to be clearer and easier to subclass and then use it. To make it kind of like this:

final class LoadingCellAdapter: TableAdapter<LoadingCellModel, LoadingCell> {
    ...
}

What do you think about this idea?

wow-such-amazing commented 6 years ago

Also why not to use performBatchUpdates on tableView here instead of begin/end updates?

self.tableView?.beginUpdates()
self.sections.enumerated().forEach { (idx,newSection) in
    if let oldSectionItems = oldItemsInSections[newSection.UUID] {
        let diffData = diff(old: (oldSectionItems as! [AnyHashable]), new: (newSection.models as! [AnyHashable]))
        let itemChanges = SectionItemsChanges.create(fromChanges: diffData, section: idx)
        itemChanges.applyChangesToSectionItems(ofTable: self.tableView, withAnimations: animationsToPerform)
    }
}

self.tableView?.endUpdates()
DispatchQueue.main.asyncAfter(deadline: .now() + 0.25, execute: { onEnd?() })
wow-such-amazing commented 6 years ago

And thanks a lot for your awesome job 👍

geraldeersteling commented 6 years ago

I second this idea.

Right now I kind of divide this logic by either creating an extension of my viewcontroller(s) in separate file (with the director logic there) or a separate class for managing a director. The latter being more troublesome however.

malcommac commented 6 years ago

@crabman448 It's a nice idea but what you need specifically? I've tried to implement it right now and it works fine with the example. What's wrong with this?

public class ArticleAdapter: TableAdapter<Article,TableArticleCell> {

    init() {
        super.init()
        self.on.dequeue = { ctx in
            ctx.cell?.titleLabel?.text = ctx.model.title
            ctx.cell?.subtitleLabel?.text = ctx.model.text
        }
        self.on.tap = { ctx in
            print("Tapped on article \(ctx.model.id)")
            return .deselectAnimated
        }
    }

}
malcommac commented 6 years ago

@crabman448 regarding the performBatchUpdates() we surely implement it but I need to add a #if available(iOS 11.0,*) because it's available only since iOS 11 or later.

wow-such-amazing commented 6 years ago

@malcommac Hm, I have this error and warning when I try to create an adapter. And this error looks reasonable) screen shot 2018-08-22 at 12 08 54

wow-such-amazing commented 6 years ago

In the example project the sources of the FlowKit are in the same module and that's why you can subclass it, I suppose

malcommac commented 6 years ago

Yeah I think you're right; it should be enough to mark the class as open leaving functions and properties not open. Or you think some of these should be open to subclass?

wow-such-amazing commented 6 years ago

I think it should be enough 👍

malcommac commented 6 years ago

975be471 TableAdapter and CollectionAdapter are now open for subclassing