DifferenceKit icon indicating copy to clipboard operation
DifferenceKit copied to clipboard

Add completion handlers to the extensions

Open ellneal opened this issue 4 years ago • 7 comments

Checklist

Description

Adds completion handlers to the UIKit/AppKit extensions.

This is a duplicate of #107 but I've taken a different approach to how the completion handlers are implemented (e.g. using a CATransaction for the non-closure based APIs).

Motivation and Context

I'm using DifferenceKit to create an API compatible version of the NSDiffableDataSource family of classes that run on iOS 12 and earlier, which requires completion handlers.

Impact on Existing Code

None. This is API compatible as the completion handlers are optional and default to nil.

ellneal avatar Jul 14 '20 12:07 ellneal

👏 can we merge this?

fruitcoder avatar Oct 05 '20 17:10 fruitcoder

@fruitcoder I actually need to fix it, the completion handlers don't work.

ellneal avatar Oct 06 '20 09:10 ellneal

Hm :/ the code makes sense to me, do you have an idea why it's not working?

fruitcoder avatar Oct 06 '20 10:10 fruitcoder

Calling group.notify before the first group.enter causes the notify block to be called immediately. I need to put that in a defer block.

ellneal avatar Oct 06 '20 13:10 ellneal

Had a chance to test the UICollectionView extension here and it works fine, and it finally solves the problem of not having a completion on every changeset applied. I ran into some issues due to this before.

This enables a more controlled way of notifying that all changesets were applied.

Q: I would only ask if adding a default parameter like queue = .main would be a good option. Allowing consumers to inject the queue in which the completion should be called.

@ra1028 if you have the chance of looking into this PR 🙏

crsantos avatar May 16 '21 09:05 crsantos

Q: I would only ask if adding a default parameter like queue = .main would be a good option. Allowing consumers to inject the queue in which the completion should be called.

The reason I didn't do this is that these functions should never be called from anything other than the main thread (since they're calling UIKit functions), so made sense to call the completion block on the same thread.

ellneal avatar May 17 '21 10:05 ellneal

Totally agree @ellneal 👌

crsantos avatar May 17 '21 11:05 crsantos