DifferenceKit
DifferenceKit copied to clipboard
Add completion handlers to the extensions
Checklist
- [x] All tests are passed.
- [x] ~~Added tests.~~ N/A.
- [x] Documented the code using Xcode markup.
- [x] Searched existing pull requests for ensure not duplicated.
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.
👏 can we merge this?
@fruitcoder I actually need to fix it, the completion handlers don't work.
Hm :/ the code makes sense to me, do you have an idea why it's not working?
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.
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 🙏
Q: I would only ask if adding a default parameter like
queue = .mainwould 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.
Totally agree @ellneal 👌