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

Suggestion in consideration of UIKit behaviour #11

Closed
lxcid opened this issue Jan 23, 2016 · 5 comments
Closed

Suggestion in consideration of UIKit behaviour #11

lxcid opened this issue Jan 23, 2016 · 5 comments

Comments

@lxcid
Copy link

lxcid commented Jan 23, 2016

I have some thoughts and suggestions, but I'm not sure about if we need them resolved. Why I want to raise the issue is because maybe someone out there might be interested. They are mostly concerning substitution/reload and how UIKit and its documentation seems to treat it inconsistently.

Expose edit's origin (source) index, at least for substitution.

My translation from changeset's edit operations to UITableView's row operations is as follow:

Edit Row
Insert Insert
Delete Delete
Move Move
Substitution Reload

When executing row operations between beginUpdates and endUpdates, like delete, reload index is expected to be the ones prior to any updates. As stated in its API documentation:

The indexes that UITableView passes to the method are specified in the state of the table view prior to any updates.

Changeset provides the index after update instead. Making it unavailable to be mixed with other operations within beginUpdates and endUpdates.

Makes reducing edits to move optional.

As of 1.0.x, in order to use it with UITableView's row operation, I have to do something like this.

        var deletedIndexPaths = [NSIndexPath]()
        var insertedIndexPaths = [NSIndexPath]()
        var movedIndexPaths = [(from: NSIndexPath, to: NSIndexPath)]()
        var reloadIndexPaths = [NSIndexPath]()

        for edit in changeset.edits {
            switch edit.operation {
            case .Insertion:
                let insertedIndexPath = NSIndexPath(forRow: edit.destination, inSection: 0)
                insertedIndexPaths.append(insertedIndexPath)
            case .Deletion:
                let deletedIndexPath = NSIndexPath(forRow: edit.destination, inSection: 0)
                deletedIndexPaths.append(deletedIndexPath)
            case .Substitution:
                let reloadIndexPath = NSIndexPath(forRow: edit.destination, inSection: 0)
                reloadIndexPaths.append(reloadIndexPath)
            case .Move(origin: let origin):
                let fromIndexPath = NSIndexPath(forRow: origin, inSection: 0)
                let toIndexPath = NSIndexPath(forRow: edit.destination, inSection: 0)
                movedIndexPaths.append((from: fromIndexPath, to: toIndexPath))
            }
        }

        self.tableView.beginUpdates()

        if deletedIndexPaths.count > 0 {
            self.tableView.deleteRowsAtIndexPaths(deletedIndexPaths, withRowAnimation: .Automatic)
        }

        if insertedIndexPaths.count > 0 {
            self.tableView.insertRowsAtIndexPaths(insertedIndexPaths, withRowAnimation: .Automatic)
        }

        for (from, to) in movedIndexPaths {
            self.tableView.moveRowAtIndexPath(from, toIndexPath: to)
        }

        self.tableView.endUpdates()

        if reloadIndexPaths.count > 0 {
            self.tableView.reloadRowsAtIndexPaths(reloadIndexPaths, withRowAnimation: .Automatic)
        }

Reload is outside the beginUpdates and endUpdates for reason stated previous. But if I exposes the origin index for substitution edit operation, theoretically, I probably can wrap all operations in beginUpdates and endUpdates.

But this is not the case. It seems a little counter intuitive but the exception thrown by UITableView is as follows:

2016-01-23 22:58:35.504 ChangesetTest[3247:111068] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'attempt to perform an insert and a move to the same index path (<NSIndexPath: 0xc000000003800016> {length = 2, path = 0 - 28})'

It stated that we are trying to attempt an insert and a move row operation but those code have not change. Funny thing is that if I commented out the reload row operation code (which is possible here as their value will just remain outdated), it will no longer complain.

I believe this is more of a wording bug, UIKit probably meant insert and reload of the same index. But this I can't be sure.

The funny things is that the API documentation for move only mention that it can work with insert and delete, but avoid mentioning reload.

As for UICollectionView, its API documentation for batch update only mention that it can work with insert, delete and move but did not mention about reload. Which will make the current solution for UITableView seems correct.

If I remove the reduce edit step. I can make it work nicely with reload in table view.

But the thing about reload is that it is the least obvious row operation visually. And there are several other options that are reasonable as well. For example, we can get visible rows and update the cell manually (no animation), or just not batching it like above (not an issue as well).

@lxcid
Copy link
Author

lxcid commented Jan 23, 2016

To be honest, I think we should leave it as status quo. Feel free to close this issue if you think the same.

@hfossli
Copy link

hfossli commented Jan 24, 2016

Have you had a look at what they are doing here https://github.com/chrisamanse/Changes/blob/master/README.md ?

(I haven't. I just noticed that project is concretely geared towards collection views thus maybe solving your issue).

@greggjaskiewicz
Copy link

thanks dude, this also applies to collection views with exceptions like:

> [error] error: Serious application error.  An exception was caught from the delegate of NSFetchedResultsController during a call to -controllerDidChangeContent:.  attempt to perform an insert and a move to the same index path (<NSIndexPath: 0xc000000000000016> {length = 2, path = 0 - 0}) with userInfo (null)
> CoreData: error: Serious application error.  An exception was caught from the delegate of NSFetchedResultsController during a call to -controllerDidChangeContent:.  attempt to perform an insert and a move to the same index path (<NSIndexPath: 0xc000000000000016> {length = 2, path = 0 - 0}) with userInfo (null)

@ghost
Copy link

ghost commented Mar 3, 2017

Thanks for this 👍

@sgermain06
Copy link

Omg, you totally saved my ass! Thanks so much!

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

No branches or pull requests

5 participants