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

Fix for contentOffset #602

Closed

Conversation

artem-shmatkov
Copy link

@artem-shmatkov artem-shmatkov commented Aug 1, 2023

Found a problem when I use FloatingPanel with UINavigationController which holds some controller with scrollView.
When FloatingPanel goes from state to state scrollView's contentOffset becomes (0, 0) instead of (0, -44) (which is normal if there is a UINavigationBar).
Tried to fix it here.

Also here are two examples before and after fix.
before: https://github.com/scenee/FloatingPanel/assets/340680/eb6cb644-4bbe-4174-ac3e-25885600bffd
after: https://github.com/scenee/FloatingPanel/assets/340680/da4a8e40-6923-45c4-b92e-eb151454f5f2

@scenee
Copy link
Owner

scenee commented Aug 2, 2023

Thanks for your PR and report. Your example helped me recognize the problem. Let me check one point. Is this problem also happening in v2.6.1?

Thanks to this PR, I noticed that the change at 7511ce5 commit in version 2.6.2 was not appropriate for some cases. Therefore I’m thinking I will revert the change.

@artem-shmatkov
Copy link
Author

Is this problem also happening in v2.6.1?

Looks like 2.6.1 doesn't have this problem.

@scenee
Copy link
Owner

scenee commented Aug 4, 2023

Thank you for your quick confirmation. Based on the result, I was wondering if you could go back to the implementation of v2.6.1 in this PR. I should have noticed this when I reviewed it, but there is another issue with the current logic which only works with the bottom positioned panel(i.e. not with left/right positioned one). This is because it only uses y of contentOffset.

I would like to fix the problem v2.6.2 was trying to fix in a different way.

@scenee
Copy link
Owner

scenee commented Aug 11, 2023

I'm closing this pull request to address the issue you reported. Thank you for your contribution. I'll add an acknowledgment to the next release note.

@scenee scenee closed this Aug 11, 2023
scenee added a commit that referenced this pull request Aug 11, 2023
…inset

These issues arose in 'Show Navigation Controller' sample of Samples app.

1. The scrollView's contentOffset always becomes (0, 0) instead of (0, -44),
   which is normal if there is a UINavigationBar.

2. The scrollView's contentOffset sometimes becomes (0, 0) after moving
   a panel quickly like picking.

Case 1 is caused by 7511ce5 commit.
Case 2 is caused by a workaround added at 81fd85e commit.

I tested this library from iOS 11 to iOS 16, and then I confirmed this
workaround doesn't need anymore.

Related to #602, #603.
scenee added a commit that referenced this pull request Aug 11, 2023
…inset

These issues arose in 'Show Navigation Controller' sample of Samples app.

1. The scrollView's contentOffset always becomes (0, 0) instead of (0, -44),
   which is normal if there is a UINavigationBar.

2. The scrollView's contentOffset sometimes becomes (0, 0) after moving
   a panel quickly like picking.

Case 1 is caused by 7511ce5 commit.
Case 2 is caused by a workaround added at 81fd85e commit.

I tested this library from iOS 11 to iOS 16, and then I confirmed this
workaround doesn't need anymore.

Related to #602, #603.
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

Successfully merging this pull request may close these issues.

2 participants