search

mbigatti / BMXSwipableCell

A custom UITableViewCell that supports swipe to reveal (similar to iOS7 Mail App)
MIT License
139 stars 27 forks source link

Does not work with AutoLayout #6

Closed cameroncooke closed 10 years ago

cameroncooke commented 11 years ago

Hi,

This looked really promising until I noticed it does not play well with Auto Layout. Because it moves the subviews out of the UITableViewCell's contentView and adds them as subviews to scrollViewContentView which causes the NSLayoutConstraints to be lost.

iOS will remove any constraints related to the view when it's removed from it's superview.

It would be nice if this library could detect all the existing constraints and add them back relative to the scrollViewContentView or alternatively try and incorporate the contentView within a UIScrollView and not the other way around as it is current doing it.

mbigatti commented 11 years ago

Hi. I believe that moving the contentView inside the UIScrollView is not possible as contentView is a readonly property and if I detach contentView from its superview I'll not be able to replace it with the UIScrollView. I think the other your suggestion is explorable: to copy also the layout constraints. I will look into it. Thanks for the report

cameroncooke commented 11 years ago

I've actually managed to implement the later suggestion of reinstalling the constraints against the new contentView. What's the best way for me to contribute the code back?

mbigatti commented 11 years ago

glad to hear that. The best way to contribute back on a github project in general is to fork the original project, modify the code, commit and create a pull request. Then the repo owner will be able to merge into his project, most of the time automatically. If you feel uncomfortable with that you can create a gist with the modified code snippets and I will import that into my version

cameroncooke commented 11 years ago

That's cool, I'll do that.

mbigatti commented 11 years ago

Thanks for the contribution. I'll merge it ASAP

cameroncooke commented 11 years ago

I've not tested it a huge amount but it seems to work with the existing demo project and works with my auto-layout cells that I'm using it for on another project. I did move the code that moved the subviews from the orignal contentView out of the layoutSubviews as I was worried about performance has this method gets hit lots of times and it seems to make sense for it to do it at the end of the init code. That said it expects storyboards to have already populated the contentView by that point so it might not work if not using IB so it might need some tweaking.

Cameron Cooke Senior Mobile Developer

Tel: 01273 625959 Web: www.brightec.co.uk Twitter: www.twitter.com/brightec

On 31 October 2013 16:04, mbigatti notifications@github.com wrote:

Thanks for the contribution. I'll merge it ASAP

— Reply to this email directly or view it on GitHubhttps://github.com/mbigatti/BMXSwipableCell/issues/6#issuecomment-27499026 .

mbigatti commented 11 years ago

I'll do some more testing. BTW, the call was in layoutSubviews for a mean, that I do not recall currently. It has to do to different timing of various iOS mechanisms. I'll double check that. Thanks

cameroncooke commented 11 years ago

Yeah I was trying to find somewhere else to put that logic as layoutSubviews can get called all the time especially when layout changes. I guess you don't really want to put code that add/removes subviews in layoutSubviews as this method is used for laying out existing subviews. Though I get why it's there as it's hard work trying to detect when the view hierarchy changes. I did try using KVO to detect CRUD operation on the contentView but it seems UIView does not support KVC.

In my fork I've added the moving logic to the end of the bmx_init method which works great for storyboard and nib files as they are guaranteed to be loaded, but I guess it won't guarantee that custom UITableView cells where the views have been added programmatically will have finished adding their views by the time that init call happens.

Cameron Cooke Senior Mobile Developer

Tel: 01273 625959 Web: www.brightec.co.uk Twitter: www.twitter.com/brightec

On 31 October 2013 16:15, mbigatti notifications@github.com wrote:

I'll do some more testing. BTW, the call was in layoutSubviews for a mean, that I do not recall currently. It has to do to different timing of various iOS mechanisms. I'll double check that. Thanks

— Reply to this email directly or view it on GitHubhttps://github.com/mbigatti/BMXSwipableCell/issues/6#issuecomment-27500180 .

mbigatti commented 11 years ago

Hi. Just a quick note to let you know that I reintroduced the call to bmx_moveContentViewSubViews into layoutSubviews, because in the case of a cell that has no label text defined in the NIB/Storyboard, the related UILabel is created only when text field is populated, and it is added to contentView rather than (obviusly) in the scrollViewContentView. I still have a call bmx_moveContentViewSubViews in the init phase as this method is designed to work incrementally so you can do some work on init and (eventually) some additional work afterwards. Unfortunately, contentView is a readonly property so I can not provide my own subclass to intercept subviews adding. This is not tested with Auto Layout, I'll do that when I have the time. Thanks