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

childReachedTheTop calculation ignores contentInset #30

Open
bingosabi opened this issue May 12, 2020 · 1 comment
Open

childReachedTheTop calculation ignores contentInset #30

bingosabi opened this issue May 12, 2020 · 1 comment

Comments

@bingosabi
Copy link

I have a DrawerView with a descendent UITableView inside a containerView. That tableview extends under a sibling header view so that the header can be collapsed to reveal more of the tableview on scroll. (Similar to how Apple collapses the Navigation Bar to show more content when you scroll).

To achieve this I set a contentInset for the top of the tableview so content can be fully exposed while the header view is fully expanded.

tableview.contentInset = UIEdgeInset(top:175, left:0, bottom:view.safeAreaInsets.bottom, right:0)

However, DrawerView's handlePan() is not taking the content inset into account when determining if there is more content in the scrollview to be shown

The fix is to change the line 810 in DrawerView:

let childReachedTheTop = activeScrollViews.contains { $0.contentOffset.y <= 0 }

to:

let childReachedTheTop = activeScrollViews.contains { $0.contentOffset.y <= -$0.contentInset.top }

and this also necessitates a change to line 837:

let minContentOffset = activeScrollViews.map { $0.contentOffset.y }.min() ?? 0

to

let minContentOffset = activeScrollViews.map { $0.contentOffset.y + $0.contentInset.top}.min() ?? 0

Happy to make a pull request with the fix, but need to take a look at the unit test setup you have first, if it even something you'd want to add a unit test for.

Great component BTW!

@mkko
Copy link
Owner

mkko commented May 22, 2020

Hi! Thank you for the feedback! I must apologize that I haven't had the time to check this any further, but if you have the time, I'd be happy to review the PR. Unit tests are more or less nonexistent, only some utility function(s) are tested as far as I remember.

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

No branches or pull requests

2 participants