Skip to content
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

[CDS] Fix appearance tracking #154

Merged
merged 1 commit into from
Apr 28, 2015
Merged

[CDS] Fix appearance tracking #154

merged 1 commit into from
Apr 28, 2015

Conversation

benlodotcom
Copy link
Contributor

Fix #129, indexPath is not a reliable thing to use for disappearance announcements on deletion (either the index would be out of bounds if the last item is deleted or it will point to an invalid item).

Calling - (void)announceWillAppearForItemInCell:(UICollectionViewCell *)cell atIndexPath:(NSIndexPath *)indexPath and - (void)announceDidDisappearForItemInCell:(UICollectionViewCell *)cell atIndexPath:(NSIndexPath *)indexPath guarantees the announcement will be dispatched to the component tree associated to the cell.

It is however non opinionated in the sense that it won't do deduplication of events, or check for imbalanced calls.
For instance while reloading a cell on screen, the collection view first send appearance events for the newly created cell and then send disappearance events for the old cell, this situation should be handled properly in the controller layer. #153 has been opened to track these use cases and provide examples/code to deal with them.

@adamjernst
Copy link
Contributor

Nice. Since we're now not even using the index path parameter from the announce... methods, mind removing it?

@benlodotcom
Copy link
Contributor Author

Kept it for API symmetry but totally fine removing it.

@adamjernst
Copy link
Contributor

I do like the symmetry but... #yagni

On Apr 28, 2015, at 11:51 AM, Benjamin Loulier notifications@github.com wrote:

Kept it for API symmetry but totally fine removing it.


Reply to this email directly or view it on GitHub.

adamjernst added a commit that referenced this pull request Apr 28, 2015
[CDS] Fix appearance tracking
@adamjernst adamjernst merged commit 90d2e1d into facebook:master Apr 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Array out of bounds in announceDidDisappearForItemAtIndexPath
3 participants