lxcid / LXReorderableCollectionViewFlowLayout

Extends `UICollectionViewFlowLayout` to support reordering of cells. Similar to long press and pan on books in iBook.
http://lxcid.com/
MIT License
1.86k stars 328 forks source link

clean up gesture recognizers #80

Closed myeyesareblind closed 9 years ago

myeyesareblind commented 9 years ago

I got a weird crash when I changed collectionViewFlowLayout dynamically. Current version leaves gestures alive, so when the collectionView is tapped, gestureRecognizer will send message to zombie.

bartoszhernas commented 9 years ago

I have the same issue, but your fix is not helping. I am setting this layout in Storyboard and then changing it dynamically and with your fix its still crashing.

lxcid commented 9 years ago

Do you have a demo project that demonstrate this? If you have I can review it.

bartoszhernas commented 9 years ago

If you add to your example something like

- (void)viewDidAppear:(BOOL)animated {
    [super viewDidAppear:animated];
    self.collectionView.collectionViewLayout = [LXReorderableCollectionViewFlowLayout new];
    self.collectionView.collectionViewLayout = [LXReorderableCollectionViewFlowLayout new];
}

It will crash then

lxcid commented 9 years ago

Noted. Thanks. Will give it a try and get back to you guys.

myeyesareblind commented 9 years ago

@bartg , thank you, my first commit didn't fix the issue. The problem is that in dealloc, collection view is already nil, so the gestures still exists.

myeyesareblind commented 9 years ago

I won't do that, feel free to contribute.

lxcid commented 9 years ago

@bartg That was what I was thinking as well.

I was thinking of the following changes but some of it I'm currently not confident enough about them. I'm tagging LOW, MED, HIGH as an indication of my confidence level of each point.

myeyesareblind commented 9 years ago

@lxcid I like 2-3, this way you are making it obvious that there is some important code to be run on dealloc.

lxcid commented 9 years ago

Thanks @myeyesareblind, didn't realise its your changes. I have pull from your repro and work on top of your changes. :)

https://github.com/lxcid/LXReorderableCollectionViewFlowLayout/commit/337711bab35e5b6a5035d5f545df8b79ed4b3a20

Could you help me review if this look good? My initial test seems to work. Let me know if you find any quirkiness. If all is well I'll merge to master and release a tag.

/cc @bartg

myeyesareblind commented 9 years ago

It's good

lxcid commented 9 years ago

Thanks for the help! Its merged!