Conversation
There was a problem hiding this comment.
Just wondering if we'd also want support for NSAttributedStrings here?
There was a problem hiding this comment.
I think for most cases I can think of, a plain String will be enough. If it becomes necessary we can add it later
|
@koke Code looks great! but it's honestly hard to follow the entire hierarchy. (Edit: solvable with some docs!) Plus, i've got some doubts, such as:
|
You'd need to implement another
I added the prefix because they inherit from |
Maybe there's a shorter way, but I'm not sure if that'd make it better or more readable |
Several changes to make the API simpler: - Only one row protocol: dropped CustomImmuTableRow, CustomCellImmuTableRow, and CustomNibImmuTableRow in favor of just ImmuTableRow. - Moved all the cell class and reusable identifier data and logic inside the registrable enum. - Renamed CellRegistrable to ImmuTableCell for clarity. - Provided a default (nil) implementation for customHeight
|
@jleandroperez ready for round two, I'm quite happy with this iteration as everything looks much simpler. I'm going to keep adding some more documentation, but feel free to start with the code review |
|
I'm done with documentation as well, and reorganised the file to make it more approachable. Please let me know if you find the docs easy to understand. After this, I'd like to try to move this to a separate library with a bunch of default cells, so it might help with documentation to have a few examples. Also a README separate from the header docs could really help explain what this is and how you're supposed to use it. I added a bit of that at the top of the file, but there's a lot of room for improvement. |
There was a problem hiding this comment.
+1 Looking nice! (Both, unowed, and a single Handler object!)
|
@koke i really like what you did with ImmuTable!. I meant to answer this one, but Github doesn't show the Q anymore: Latest implementation looks better than that (less complicated!). There's always room to extend / improve (support for UIViewControllers, ie), but i'm sure ImmuTable can grow as needed / when needed / if needed.
|
I think if you are building a view that's just a table that's really kind of a form, your view controller should only have to specify those cells in a declarative way. The only code there should be how to get a model, turn it into a view model, and handle any changes.
ImmuTable was already introduced in #4510 but I'm extracting it to a separate PR for easier review.
If you want to test it and see it in action, I've reimplemented Settings with ImmuTable in #4549
Needs Review: @jleandroperez