Open winstondu opened 4 years ago
@ra1028
Seems really welcome fix in my opinion! 👍
Just for clarification, do I understand correctly:
I have item with automatic hash and equality:
struct MyText: Hashable, Equatable {
var id: Int
var text: String
}
Change [ {id: 1, text: "a"} ]
-> [ {id: 1, text: "b"} ]
will result in one deletion and one insert.
but
struct MyText: Hashable, Equatable {
var id: Int
var text: String
func hash(into hasher: inout Hasher) {
hasher.combine(id)
}
}
[ {id: 1, text: "a"} ]
-> [ {id: 1, text: "b"} ]
will result in one update.
@ollitapa , that is correct.
@ra1028 , awaiting your approval to merge :)
@ra1028 ping. Would be nice to get this merged 😄
Thanks @winstondu and reviewers.
Sorry for my late reply.
I think it's difficult to judge this specification change.
I agree with its behavior, but it works differently than the original version UICollectionViewDiffableDataSource
.
Because, DiffableDataSources
is only a backport, differences between the original version and the specification can adversely affect future replacements.
Checklist
Description
The current code was not correctly leveraging
DifferenceKit
to address moves and updates. In particular, it was not distinguishing changes in content versus changes in the identifier.This code change ensures that the difference identifier is no longer of the whole
Item
, but instead just the hashValue of theItem
(as determined by the implemention ofItem
.)For a visualization of the difference, please see the below gifs (the example iOS code in this repo was slightly modified to illustrate this).
For a clear conceptual example of the correction that this pull request sets out to accomplish, consider if
Item
was the following:And we had to compare the datasource diff of the following lists (being used as datasource updates for a collectionview): Source:
[(birthName: "Andy", age: 15), (birthName: "Kevin", age: 16)]
Target:
[(birthName: "Kevin", age: 16)]
Without the change in this pull request, we get 2 deletions and 1 insertion (animated as such)
With this change in this PR , we get 1 deletion and 1 update (correctly animated).
Related Issue
N/A
Motivation and Context
The motivation was for a better, and more correct, diff animation.
Impact on Existing Code
None beyond what is described.
Screenshots
Notice the "move" animations that occur in after that do not happen in before
Before:![Before](https://user-images.githubusercontent.com/8174310/74995346-7b64e180-5405-11ea-837f-6c60fd8b220e.gif)
After:![after](https://user-images.githubusercontent.com/8174310/74995354-815ac280-5405-11ea-802b-f608ed7f0e31.gif)