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 content offset adjustment for bounds changing #130

Merged

Conversation

jinxiao-zang
Copy link
Contributor

Details

For the bottomToTop layout, the y offset adjustment max(currentCollectionView.bounds.height - newBounds.height + contentInset.bottom, 0) is not right. we should not involve the contentInset.bottom.
The iOS system will help adjust the yOffset when new bounds exceed the boundary. It means we should check
if newBounds.maxY < currentCollectionView.contentSize.height + contentInset.bottom as well.

Motivation and Context

We are building an messaging screen for Airbnb and I find this issue causes the message cells jumping when typing indicator hide and show.

How Has This Been Tested

I built a small sample to test this case.
Before this change: The cells move up every time the bounds changed.
After the change: The cells keep bottom spacing the same.

Before After
ezgif-1-b9833aa1ea ezgif-1-99777f3cb7

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

@jinxiao-zang
Copy link
Contributor Author

@brynbodayle @bryankeller
PTAL!

@bryankeller
Copy link
Contributor

Thanks @jinxiao-zang !

@bryankeller bryankeller merged commit 91ad7e5 into airbnb:master May 23, 2024
1 check passed
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