Skip to content

CONV-1435: Add scroll indicator insets to customScrollViewInsets #500

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.

Sign up for GitHub

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

Merged
gilgtm merged 3 commits into from
Aug 4, 2023
Merged

CONV-1435: Add scroll indicator insets to customScrollViewInsets #500

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a86fa4
CONV-1435: Add scroll indicator insets to customScrollViewInsets
gilgtm Aug 3, 2023
8746cb7
Refactor updateScrollViewInsets to check for difference before settin…
gilgtm Aug 3, 2023
80acc3b
refactor to use new type ScrollViewInsets instead of a tuple
gilgtm Aug 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions Demo/Sources/Demos/Demo Screens/ChatDemoViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,16 @@ final class ChatDemoViewController : UIViewController {
listView.frame = view.bounds

listView.customScrollViewInsets = { [weak self] in
guard let self = self else { return .zero }
guard let self = self else { return (.zero, .zero, .zero) }
let inset = max(self.footerHeight, self.keyboardHeight - self.view.safeAreaInsets.bottom)
print("new bottom inset:", inset)
return UIEdgeInsets(
top: 0,
left: 0,
bottom: inset,
right: 0
)
let insets = UIEdgeInsets(
top: 0,
left: 0,
bottom: inset,
right: 0
)
return (insets, .zero, insets)
}
listView.onKeyboardFrameWillChange = { [weak self] keyboardCurrentFrameProvider, keyboardAnimation in
guard let self = self else { return }
Expand Down
6 changes: 5 additions & 1 deletion Demo/Sources/Demos/Demo Screens/KeyboardTestingViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ final class KeyboardTestingViewController : UIViewController
}

self.listView.customScrollViewInsets = { [weak self] in
self?.insets ?? .zero
if let insets = self?.insets {
return (insets, insets, insets)
} else {
return (.zero, .zero, .zero)
}
}

self.listView.onKeyboardFrameWillChange = { [weak self] keyboardCurrentFrameProvider, animation in
Expand Down
11 changes: 9 additions & 2 deletions ListableUI/Sources/ListView/ListView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,11 @@ public final class ListView : UIView

/// This callback determines the scroll view's insets only when
/// `behavior.keyboardAdjustmentMode` is `.custom`
public var customScrollViewInsets: () -> UIEdgeInsets = { .zero }
public var customScrollViewInsets: () -> (
Copy link
Collaborator

@kyleve kyleve Aug 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you, but it may be nice to make this a real struct vs just a tuple, so we can document it, etc. But just a suggestion, not a big deal.

Copy link
Collaborator Author

@gilgtm gilgtm Aug 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added! new type ScrollViewInsets

content: UIEdgeInsets,
horizontalScroll: UIEdgeInsets,
verticalScroll: UIEdgeInsets
) = { (.zero, .zero, .zero) }

/// Call this to trigger an insets update.
/// When the `keyboardAdjustmentMode` is `.custom`, you should set
Expand All @@ -338,7 +342,10 @@ public final class ListView : UIView
public func updateScrollViewInsets()
{
if case .custom = self.behavior.keyboardAdjustmentMode {
self.collectionView.contentInset = self.customScrollViewInsets()
let insets = self.customScrollViewInsets()
self.collectionView.contentInset = insets.content
Copy link
Collaborator

@kyleve kyleve Aug 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: We should wrap each of these assignments in a != check (as is done below) before assigning: It's mostly a "belt and suspenders" thing, but sometimes, even setting these values to the same exact value as before can break in-progress scroll events for whatever reason.

Copy link
Collaborator Author

@gilgtm gilgtm Aug 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored!

Copy link
Collaborator

@kyleve kyleve Aug 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏 Thank you!

self.collectionView.horizontalScrollIndicatorInsets = insets.horizontalScroll
self.collectionView.verticalScrollIndicatorInsets = insets.verticalScroll
return
}

Expand Down