Fix for tableView headers

[palcalde]

@bryankeller This solves the issues with the tableView headers in #8
and maybe in (didn't test it) #14

Section headers in UITableView are based on contentInset. If the bar shrinks, the tableView needs to have contentInset udpated in order to have the headers scroll smoothly.

I don't think there is a need to change the contentInset for all UIScrollView's (just for the ones that are UITableViews) so I created one initializer in the bar to pass the tableview. This way people can still use the regular initializer you created for UIScrollViews.

Finally, I had to change scrollViewDidScroll of SquareCashStyleBehaviorDefiner. It was doing a check based on the contentInset and since that changes now, it didn't work well. Now is based on the max bar height which works great.

Hope this helps!

[bryankeller]

This sounds fantastic, I'll take a look this week. Thank you!

[palcalde]

@DanielKrofchick yep, I'll update it!

[palcalde]

@DanielKrofchick updated.

[palcalde]

@bryankeller any updates on this?

[bryankeller]

After finals next week. Really sorry for keeping this and a few other PRs up so long. Just have to prioritize school before I can take a serious look at these.

[palcalde]

Absolutely! good luck your finals!

[bryankeller]

Hey @palcalde, this is great. Thanks for taking the time to figure this out. I tried it out and it works really well. Before I merge this in, I want to discuss this point that you made:

I don't think there is a need to change the contentInset for all UIScrollView's (just for the ones that are UITableViews) so I created one initializer in the bar to pass the tableview.

I see that you give BLKFlexibleHeightBar a custom initializer to init with a table view. I don't think that BLKFlexibleHeightBar should have a reference to a table view. One of my goals when architecting this library was to have a strict separation between the bar and the content being displayed. The communication between these two things should only be done through BLKFlexibleHeightBarBehaviorDefiner.

Hypothetically, what if Apple releases a class in the future that does calculations based on content inset? Adding another initializer every time Apple does this isn't feasible.

With that said, what if we implement your content inset adjustment code for all UIScrollViews, not just UITableViews. I don't think this would cause undesired behavior. Perhaps in your testing you found that it does? Let me know what you think of this change. If you agree, let's:

1. Remove the custom initializers
2. Remove the table view property
3. Correct content insets in BLKFlexibleHeightBarBehaviorDefiner, not in BLKFlexibleHeightBar

Lastly, just a couple of super minor things. If you can change your { and } to be on new lines (so that it's consistent with the rest of the project), I'd appreciate that.

Thanks again for fixing this. I know a lot of people came across this issue and will appreciate a fix. Let me know if you have any thoughts on what I said.

[palcalde]

good morning @bryankeller! It makes sense to me the separation of concerns that you want to have. I'll update the pull request with your points soon and let you know.

[palcalde]

@bryankeller After taking a look, these are the issues I see with the implementation you propose:
As dicussed, in order to have the tableView smoothly change the sectionheader frame, we need to update the contentInset every time tableView.frame.origin.y changes.
With current implementation, scrollViewDidScroll is the one that changes 'progress' and calls setNeedsLayout. These triggers layoutSubviews in BLKFlexibleHeightBar at some point (but not synchronously) which causes the updates on the frame. This means that the BehaviorDefiner never knows at which exact point the frame of flexibleHeightBar changes, since layoutSubviews gets triggered async, so it can't update the contentInset as we need.

The only solution that comes to my mind, without tightly coupling FlexibleHeightBar with the ScrollView, is to have FlexibleHeightBar notify the behaviorDefiner whenever layoutSubviews did get executed. To do this we would have to:

1. Add a public method in the BehaviorDefiner like 'flexibleHeightBarDidLayoutSubviews', which I don't really like since it would mess a little bit the pubic interface of that class. Users might get confused about why that is there and when should they call it.
2. At the end of layoutSubviews in FlexibleHeightBar, do [self.behaviorDefiner flexibleHeightBarDidLayoutSubviews]
3. Inside flexibleHeightBarDidLayoutSubviews, we wouldn't have a reference to the scrollView, since there isn't any @Property in BehaviorDefiner. We would have to add that property, change the initializer of BehaviorDefiner to be sometihg like initWithScrollView and then, finally, update the contentInset of the ScrollView.

Let me know if this makes sense or if you find a better solution.

[bryankeller]

@palcalde Unfortunately, I just noticed an issue with this method (the one implemented in your PR). It causes strange and abrupt scroll view deceleration whenever the content inset is being tweaked on the fly. I'm looking into this now.

Regarding your suggestions - it sounds like it would work, but I'd still like to investigate other possible solutions before committing to that, especially since there seems to be some issues with adjusting content inset.

[palcalde]

@bryankeller sure, if you found an issue with the implementation of the PR take your time to find other solutions. Np!

[mthole-old]

Any progress on this, folks? I spent a bit of time playing with BKLFlexibleHeightBar tonight and ran into the issue described here... although for a UICollectionView. (An additional reason why the more general solution and separation of concerns that @bryankeller mentioned above is important).

[asterhouse]

It sounds like you all have been really busy, but I had a question about this fix. Currently this assumes we will be using initWithFrame. If we are getting the layout from a xib file, would the best way to modify this be to create a setTableView method for the bar object?

[andreyoshev]

Any progress?