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 = .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.
Totally agree @ellneal 👌