Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

TUITableView -reloadLayout does not update cell data #57

Closed
jwilling opened this Issue · 21 comments

5 participants

@jwilling

Please correct me if I'm wrong, but from the scarce documentation in the header of TUITableView it appears that the cells, along with the layout + data, are supposed to be reloaded. This appears to not be the case.

When calling -reloadLayout, it seems that -cellForRowAtIndexPath is never called, which means that the cell never has a chance to react to new data. Is this intentional?

@joshaber
Owner

It sounds like you want to call -reloadData instead. -reloadLayout just lays out the cells it already has, after re-calculating their positions.

@jwilling

-reloadData is not animatable.

@joshaber
Owner

Right, but as soon as you talk about reloading cells any idea of animation goes out the window since it can reuse different cells to display the same index path. This is where we start to feel the same pain as UITableView.

@avaidyam

So how does UITableView work around it? TUITableView is an insanely large portion of TwUI, at least to me.

@jspahrsummers

UITableView has insertion, deletion, and reordering methods that support animation. TUITableView has no such parallel methods.

@jwilling

Would this be something worth investing time in, or is it outside the scope of TUITableView?

@joshaber
Owner

I think it's certainly within the scope of TUITableView.

@jwilling

The problem is, who wants to do it? I'm not sure I want to tackle that thing. :stuck_out_tongue_winking_eye:

@jspahrsummers

I agree that such methods are entirely within the scope of TUITableView, but I also want to add a word of caution – namely, animating individual cells will still be hard. It is a hard problem to solve, and UITableView only gives you the primitives. If you don't keep your model in sync with your animations (which is hard to do), you can easily run into crashes and glitchy behavior.

Fancying MVVM as I do, I think the real solution would involve some bindings/RAC-based approach to cell animations, so that the view always reflects the current state of the model. This is even more work, though, and probably not worth it for TUITableView.

@CodaFi

Why not support animated reloading the way you did it with JAListView, JoshAber? Sure it's old, but it works.

@joshaber
Owner

@CodaFi The architecture of JAListView is really different from that of TUITableView. With JAListView you always had one view per row. (It's actually a terrible idea for performance scalability but it made animations straightforward.) TUITableView reuses its cells. The API, I guess, could end up looking similar to that of JAListView, but there isn't really an obvious way to share their implementations.

@jwilling

Okay, so here's another idea. This could be modeled after UIC********nView, in a sense, with a block-based update method like the following:

- (void)performBatchUpdates:(void (^)(void))updates completion:(void (^)(BOOL finished))completion

with optional methods such as:

- (void)insertItemsAtIndexPaths:(NSArray *)indexPaths
- (void)moveItemAtIndexPath:(NSIndexPath *)indexPath toIndexPath:(NSIndexPath *)newIndexPath
- (void)deleteItemsAtIndexPaths:(NSArray *)indexPaths

Thoughts?

@jspahrsummers

@jwilling That interface doesn't seem substantially different from that of UITableView, with the exception of having a completion block. It'll still be just as complex to implement and use.

@jwilling

@jspahrsummers Well of course the optional methods should be similar or at least the same, but the block-based batch update method is not in UITableView. I'm sure it would be a challenge, but I've had the opportunity to use that API and it is so much easier to use when compared to the UITableView begin/end updates dance.

@CodaFi

I think the begin/end updates dance is really there to freeze/defrost the state of both the model and the view to perform updates to only either the visible cells or the model (the docs do say we aren't allowed to call -reloadData, or it will invalidate something), which means UITableView might be capturing a reference to visible cells, and calculating the cells it needs to make or destroy. I don't know. I'll try to fork and write out some primitive batch updates tonight.

@avaidyam

@CodaFi Please do, and I'll help if I can :D
@jspahrsummers Do you think TwUI could use a UI State Preservation system?

@jspahrsummers

@galaxas0 Please open a new issue to discuss new proposed features, and include some rationale as to why you think it should be included.

@avaidyam

Will do then.

@jwilling

Okay, here's a thought. Inserting and removing rows is going to be pretty difficult. Updating a single row, on the other hand, doesn't seem like it should be an extremely difficult task.

I'm not quite sure I understand what's going on in TUITableView very well, but from what I gather it appears that TUITableView caches row heights into rowInfo. Since this contains the height at the last update, this could be easily used to calculate the difference between the updated row height for a specific row and update the total section height accordingly.

The slightly tricky part is how to update the entire table view's height and relative positioning without causing a complete update of all the cells. Any thoughts on how this would work would be appreciated.

@avaidyam

I'm almost done with a UICollectionView-like class which allows for grid-based layouts in TwUI (and I will create a nice pull request for it!). I do something similar, but it may not be what you want:

When I trigger a reload, or I want to update a single cell that affects other cells, first I backup the row info cache, and grab a new copy of row heights. Then, for each cell on the screen, I map it to the row heights before and after. When there's a height change, I animate (optionally) that cell's growth, and at the same time, push all the other cells downwards. When the cells are pushed downwards, I determine which ones are off-screen after the change, and remove those from the active cell list.

This way, the cells aren't notified of updates, and only the off-screen cells after update are told to enqueue. All of this logic is handled by the table view/collection view.

@jwilling

Closing this out, as these changes (if implemented) could possibly be breaking and/or would most likely introduce bugs.

@jwilling jwilling closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.