Citrrus / MBContactPicker

iOS7 Styled Contact Picker Library that uses a UICollectionView
MIT License
501 stars 60 forks source link

Use a simpler method of invalidating our layout. This handles resizing ... #71

Closed mhupman closed 10 years ago

mhupman commented 10 years ago

...cells due to any size change (for example, on device rotation). Potential fix for #66. @lttldrgn Would you mind checking this out?

MattCBowman commented 10 years ago

:goat:

lttldrgn commented 10 years ago

Hey, just now getting around to looking at this. It definitely doesn't crash anymore, but I did notice a small change in behavior. Maybe not a big deal, but the view does not seem to resize on rotation when contacts are already populated. If the view is populated/modified while in a particular orientation, it will resize correctly to fit the current size of the the view in the current orientation.

I first tried merging the changes into master and then just tried the develop branch. Is there anything else I should try?

To test I started in portrait, added three contacts to make the view resize. Rotated to landscape and the view maintained the two line size instead of collapsing to one, even though there is plenty of room left on the line. Alternatively, start in landscape and add enough contacts to almost fill the view. Rotate to portrait and it does not expand the view to two lines to wrap the content as the old implementation did. I duplicated this behavior in the sample app as well as my own. Let me know if you need any more details.

mhupman commented 10 years ago

Using the develop branch should be sufficient. I'll do another round of testing here - I was focused on trying to make sure the cells got resized appropriately on rotation, I may have missed the behavior you are noticing.

mhupman commented 10 years ago

@lttldrgn Ok, I think I've got a fix that addresses both of our issues. Could you take a look at 6966f58 and let me know what you think?

lttldrgn commented 10 years ago

Tested and looks good to me.

mhupman commented 10 years ago

Great, thanks for checking. I'll get it merged in.