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

implement update modified items #11

Closed
wants to merge 8 commits into from
Closed

implement update modified items #11

wants to merge 8 commits into from

Conversation

wangsha
Copy link

@wangsha wangsha commented Dec 24, 2013

No description provided.

@wtmoose
Copy link
Member

wtmoose commented Dec 24, 2013

Have you done any testing with mixed updates, i.e. inserted, deleted, moved and modified rows together? I have had trouble with this, which is why it was a todo item. If you look at performBatchUpdatesOnTableView, modified rows are done separately in the completion block because I was unable to do mixed updates in a single batch update without crashing:

[CATransaction setCompletionBlock: ^{

    //modified items have to be reloaded after all other batch updates
    //because, otherwise, the table view will throw an exception about
    //duplicate animations being applied to cells. This doesn't always look
    //nice, but it is better than a crash.

    if (self.modifiedItems.count) {
        NSMutableArray *indexPaths = [[NSMutableArray alloc] init];
        for (id item in self.modifiedItems) {
            NSIndexPath *indexPath = [self.updatedDataModel indexPathForItem:item];
            [indexPaths addObject:indexPath];
            [tableView reloadRowsAtIndexPaths:indexPaths withRowAnimation:animation];
        }
    }

}];

@wangsha
Copy link
Author

wangsha commented Dec 25, 2013

Thanks for pointing that out. I haven't test it throughly, but I do have use cases with mixed insert/delete/update. If I do the same thing for collection view, updating modified rows in completionBlock, will it work?

@wangsha
Copy link
Author

wangsha commented Dec 25, 2013

Can you give me a use case that will cause the crash. I tried with mixed updates, seems working fine. tested on iPad simulator, ios7

if (self.modifiedItems.count) {
NSMutableArray *indexPaths = [[NSMutableArray alloc] init];
for (id item in self.modifiedItems) {
NSIndexPath *indexPath = [self.oldDataModel indexPathForItem:item];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be?

NSIndexPath *indexPath = [self.updatedDataModel indexPathForItem:item];`

@wtmoose
Copy link
Member

wtmoose commented Dec 25, 2013

Maybe it works on iOS7. I haven't looked at it since iOS6. I don't remember a specific use case, but I'll do some testing after the holidays.

@wangsha
Copy link
Author

wangsha commented Dec 26, 2013

cool, thanks! happy holiday.

@wtmoose
Copy link
Member

wtmoose commented Apr 2, 2014

I've added updating modified rows in completion block.

@wtmoose
Copy link
Member

wtmoose commented Apr 6, 2014

Regarding occasional crashes, are you referring to my latest update or this pull request? If you mean the pull request, I would suggest you try wrapping both batch updates in a single CATransaction as performBatchUpdatesOnTableView:withRowAnimation:completion: does and set the completion block on the transaction.

@wangsha
Copy link
Author

wangsha commented Apr 8, 2014

I am referring to the pull request. Will try your suggestion. Thanks.

@wtmoose
Copy link
Member

wtmoose commented May 28, 2014

I think this can be closed now. Please reopen if you continue to have problems.

@wtmoose wtmoose closed this May 28, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants