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

Fix stacked section controller OOB when cell ends display #358

Closed
wants to merge 2 commits into from

Conversation

rnystrom
Copy link
Contributor

Changes in this pull request

Fixes bug reported internally. When items are removed dynamically the stack internal store will attempt to access data that has already been removed. Instead use assoc objects.

We did change IGListAdapter to use a map instead of assoc objects. That could be a good cleanup.

cc @cdoncarroll

Pull request checklist

  • All tests pass. Demo project builds and runs.
  • I added tests, an experiment, or detailed why my change isn't tested.
  • I have reviewed the contributing guide

@jessesquires jessesquires added this to the 2.1.0 milestone Dec 22, 2016
@@ -21,6 +21,8 @@ This release closes the [2.1.0 milestone](https://github.com/Instagram/IGListKit

- Prevent adapter data source from deallocating after queueing an update. [Ryan Nystrom](https://github.com/rnystrom) (tbd)

- Fix OOB bug when child section controllers in a stack remove cells. [Ryan Nystrom](https://github.com/rnystrom) [(#358)](https://github.com/Instagram/IGListKit/pull/358)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spell out out-of-bounds 😄

@facebook-github-bot
Copy link
Contributor

@rnystrom updated the pull request - view changes

@jessesquires
Copy link
Contributor

@rnystrom - oh i forgot, we can edit files inline on the GH web UI. pretty sweet. fixed my own nits. 😄

we should probably do this with contributors from now on instead of commenting.

@facebook-github-bot
Copy link
Contributor

@jessesquires has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@rnystrom
Copy link
Contributor Author

@jessesquires that is AWESOME. Hell yes.

@coveralls
Copy link

coveralls commented Dec 23, 2016

Coverage Status

Coverage increased (+0.01%) to 97.967% when pulling 911d032 on stack-oob into 2957ee5 on master.

@coveralls
Copy link

coveralls commented Dec 23, 2016

Coverage Status

Coverage increased (+0.01%) to 97.967% when pulling 911d032 on stack-oob into 2957ee5 on master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants