-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add move detection and support to ASLayoutTransition #1006
Changes from 10 commits
b0c715d
f189d1b
09d43ae
baba616
beb204e
d4ce1bc
18bf81a
22d24b5
9898f0e
a8cf065
14150e1
b400cac
0c949c4
067aa85
d2ca43e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,60 @@ | |
|
||
#import <Foundation/Foundation.h> | ||
|
||
/** | ||
* These changes can be used to transform `self` to `array` by applying them in (any) order, *without shifting* the | ||
* other elements. This can be done (in an NSMutableArray) by calling `setObject:atIndexedSubscript:` (or just use | ||
* [subscripting] directly) for insertions from `array` into `self` (not the seemingly more apt `insertObject:atIndex`!), | ||
* and using the same method for deletions from `self` (*set* a `[NSNull null]` as opposed to `removeObject:atIndex:`). | ||
* After all inserts/deletes have been applied, there will be no nulls left (except possibly at the end of the array if | ||
* `[array count] < [self count]`) | ||
|
||
* Some examples: | ||
* in: ab c | ||
* out: abdc | ||
* diff: ..+. | ||
* | ||
* in: abcd | ||
* out: dcba | ||
* dif: ---.+++ | ||
* | ||
* in: abcd | ||
* out: ab d | ||
* diff: ..-. | ||
* | ||
* in: a bcd | ||
* out: adbc | ||
* diff: .+..- | ||
* | ||
* If `moves` pointer is passed in, instances where one element moves to another location are detected and reported, | ||
* possibly replacing pairs of delete/insert. The process for transforming an array remains the same, however now it is | ||
* important to apply the moves in order and not overwrite an element that needs to be moved somewhere else. | ||
* | ||
* the same examples, with moves: | ||
* in: ab c | ||
* out: abdc | ||
* diff: ..+. | ||
* | ||
* in: abcd | ||
* out: dcba | ||
* diff: 321. | ||
* | ||
* in: abcd | ||
* out: ab d | ||
* diff: ..-. | ||
* | ||
* in: abcd | ||
* out: adbc | ||
* diff: .312 | ||
* | ||
* Other notes: | ||
* | ||
* No index will be both moved from and deleted. | ||
* Each index 0...[self count] will be either moved from or deleted. If it is moved to the same location, we omit it. | ||
* Each index 0...[array count] will be the destination of ONE move or ONE insert. | ||
* Knowing these things means any two of the three (delete, move, insert) implies the third. | ||
*/ | ||
|
||
@interface NSArray (Diffing) | ||
|
||
/** | ||
|
@@ -35,4 +89,12 @@ | |
*/ | ||
- (void)asdk_diffWithArray:(NSArray *)array insertions:(NSIndexSet **)insertions deletions:(NSIndexSet **)deletions compareBlock:(BOOL (^)(id lhs, id rhs))comparison; | ||
|
||
/** | ||
* @abstract Compares two arrays, providing the insertion, deletion, and move indexes needed to transform into the target array. | ||
* @discussion This compares the equality of each object with `isEqual:`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should add to the method above that it will not transform into the same order, only to the same contents? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ahh but it will, you only need the moves if you want to animate them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will add some comments on this, as it was not clear to me until close to the end that this was actually true (that the -/+ information is sufficient and accurate to do the reordering without moves). It requires applying the diff in a certain way (basically in order into a sparse array) that was neither documented nor being done. |
||
* This diffing algorithm uses a bottom-up memoized longest common subsequence solution to identify differences. | ||
* It runs in O(mn) complexity. | ||
* The moves are returned in ascending order of their destination index. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Usually, the moves are returned in a form that applies either before any changes, after insertions, or after deletions - in other words, the order of insert / delete being applied (if at all) does matter for the move indices being valid. Could you expand a bit on the comment to describe the exact order of application for insertions, deletions, and moves that results in the correct transformation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will do - these are returned as simple (previousIndex -> newIndex) moves, which is easy to work with and consistent with what vanilla |
||
*/ | ||
- (void)asdk_diffWithArray:(NSArray *)array insertions:(NSIndexSet **)insertions deletions:(NSIndexSet **)deletions moves:(NSArray<NSIndexPath *> **)moves; | ||
@end |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Outside the scope of this change - but I've heard that CALayer .zPosition is animatable. If we wanted to, it might be possible to animate the move operations (not sure exactly what it looks like though).
I suppose doing the reordering at the beginning is better than at the end (usually this is less noticeable than a pop at the end), so I think this implementation is ideal for current needs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's great, maybe it is a cross-fade animation or something. But yes being able to animate that is a good win from this.
The second point is very related, as setting up animations that will run simultaneously also allows us to do the setup in whatever order makes life easiest for ourselves, not necessarily tied to the order in which anything plays out on the screen. But at present (where we are not animating z-order) I agree it looks nicer to have the z-order swap happen before the animations rather than at the end.