-
Notifications
You must be signed in to change notification settings - Fork 4
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
Adds a scroll offset manager class which caches and restores scroll offsets #63
Changes from 2 commits
ade30eb
568a814
43fe942
1cb3ed6
085f2f4
19b7474
65d0d34
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
// | ||
// ScrollDelegatable.swift | ||
// ThunderTable | ||
// | ||
// Created by Simon Mitchell on 02/09/2020. | ||
// Copyright © 2020 threesidedcube. All rights reserved. | ||
// | ||
|
||
import UIKit | ||
|
||
/// A simpler version of `UIScrollViewDelegate` so we don't intefere | ||
/// with users `UIScrollViewDelegate` implementations | ||
public protocol ScrollOffsetDelegate: class { | ||
|
||
/// Called when the content offset of the scroll view changes due to `scrollViewDidScroll` | ||
/// - Parameters: | ||
/// - scrollable: The scrollable that the change was for | ||
func scrollViewDidChangeContentOffset(_ scrollable: ScrollOffsetManagable) | ||
} | ||
|
||
/// A protocol implemented to allow control and listening to scroll view offset changes | ||
/// by `ScrollOfffsetManager` | ||
public protocol ScrollOffsetManagable: class { | ||
|
||
/// The scroll view that is controllable on the object | ||
var scrollView: UIScrollView? { get } | ||
|
||
/// The delegate to have scroll view delegate methods passed to, this should be a `weak` | ||
/// property to avoid retain cycles. | ||
/// - Warning: It is your job to call the method on this delegate from your scrollViewDidScroll method. | ||
var scrollDelegate: ScrollOffsetDelegate? { get set } | ||
|
||
/// An identifier used by `ScrollOffsetManager` | ||
var identifier: AnyHashable? { get set } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
// | ||
// ScrollOffsetManager.swift | ||
// ThunderTable | ||
// | ||
// Created by Simon Mitchell on 02/09/2020. | ||
// Copyright © 2020 threesidedcube. All rights reserved. | ||
// | ||
|
||
import CoreGraphics | ||
import Foundation | ||
|
||
/// Scroll offset manager manages caching scroll offsets for UI in any scenario where | ||
/// scroll views may be re-used and so we need to store their current offset in memory | ||
/// to avoid re-use issues. | ||
class ScrollOffsetManager { | ||
|
||
private var offsetMap: [AnyHashable : CGPoint] = [:] | ||
BenShutt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/// Registers the scrollable to the manager for listening for scroll offset changes | ||
/// - Parameters: | ||
/// - scrollable: The scrollable to register | ||
func register( | ||
scrollable: ScrollOffsetManagable | ||
) { | ||
scrollable.scrollDelegate = self | ||
} | ||
|
||
/// Caches the offset for the given scrollable | ||
/// - Parameters: | ||
/// - scrollable: The scrollable to update the content offset for | ||
func updateCachedOffset(scrollable: ScrollOffsetManagable) { | ||
guard let identifier = scrollable.identifier else { return } | ||
offsetMap[identifier] = scrollable.scrollView?.contentOffset | ||
} | ||
|
||
/// Sets the content offset on the given scrollable from the internal cache | ||
/// - Parameters: | ||
/// - scrollable: The scrollable to adjust the content offset on | ||
/// - animated: Whether the transition should animate | ||
/// - fallback: A fallback to set the content offset to if there is no cached value | ||
func setScrollOffset( | ||
_ scrollable: ScrollOffsetManagable, | ||
animated: Bool = false, | ||
fallback: CGPoint? = nil | ||
) { | ||
guard let identifier = scrollable.identifier else { return } | ||
guard let newOffset = offsetMap[identifier] ?? fallback else { return } | ||
|
||
guard let scrollView = scrollable.scrollView else { return } | ||
|
||
// Disable then re-enable scroll indicators otherwise calling setContentOffset flashes the scroll indicators | ||
let preShowVerticalScrollIndicator = scrollView.showsVerticalScrollIndicator | ||
let preShowHorizontalScrollIndicator = scrollView.showsHorizontalScrollIndicator | ||
|
||
scrollView.showsVerticalScrollIndicator = false | ||
scrollView.showsHorizontalScrollIndicator = false | ||
|
||
scrollable.scrollView?.setContentOffset(newOffset, animated: animated) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you! 🤦 |
||
|
||
scrollView.showsVerticalScrollIndicator = preShowVerticalScrollIndicator | ||
scrollView.showsHorizontalScrollIndicator = preShowHorizontalScrollIndicator | ||
} | ||
|
||
/// Resets all content offsets by removing them from the offset map | ||
func resetAllOffsets() { | ||
offsetMap = [:] | ||
} | ||
} | ||
|
||
extension ScrollOffsetManager: ScrollOffsetDelegate { | ||
|
||
func scrollViewDidChangeContentOffset(_ scrollable: ScrollOffsetManagable) { | ||
updateCachedOffset(scrollable: scrollable) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,14 +113,19 @@ open class TableViewController: UITableViewController, UIContentSizeCategoryAdju | |
|
||
open var data: [Section] { | ||
set { | ||
// If the table view has had rows removed/added | ||
BenShutt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if newValue.indexPaths != data.indexPaths { | ||
resetEmbeddedScrollOffsets() | ||
} | ||
_data = newValue | ||
tableView.reloadData() | ||
} | ||
get { | ||
return _data | ||
} | ||
} | ||
|
||
|
||
/// The currently selected index path of the table view | ||
public var selectedIndexPath: IndexPath? | ||
|
||
public var selectedRows: [Row]? { | ||
|
@@ -326,6 +331,8 @@ open class TableViewController: UITableViewController, UIContentSizeCategoryAdju | |
|
||
textLabel?.paragraphStyle = ThemeManager.shared.theme.cellTitleParagraphStyle | ||
detailLabel?.paragraphStyle = ThemeManager.shared.theme.cellDetailParagraphStyle | ||
|
||
updateScrollPosition(cell: cell, at: indexPath) | ||
|
||
row.configure(cell: cell, at: indexPath, in: self) | ||
} | ||
|
@@ -570,7 +577,53 @@ open class TableViewController: UITableViewController, UIContentSizeCategoryAdju | |
set(indexPath: indexPath, selected: false) | ||
} | ||
} | ||
|
||
|
||
//MARK: - | ||
//MARK: Scroll Offset Management | ||
//MARK: | ||
|
||
private lazy var scrollOffsetManager: ScrollOffsetManager = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Up to you here, there's nothing wrong with doing it with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah let's go with that. It's memory footprint should be tiny anyways! |
||
ScrollOffsetManager() | ||
}() | ||
|
||
/// Whether the table view should keep track of scroll view positions within it's cells. | ||
/// This allows us to prevent re-use issues with re-using cells that the user has scrolled. | ||
/// To allow your cell's scroll position to be remembered you need to implement the `ScrollOffsetManagable` | ||
/// protocol on your `UITableViewCell` subclass. | ||
/// - Note: This will not handle re-ordering of cells at present, and if the length of your | ||
/// data changes (different numbers of rows in any section, or different number of sections) | ||
/// then the cached values will be reset. This may be improved in future if we decide to enforce | ||
/// row's being `Equatable`. | ||
public var rememberEmbeddedScrollPositions: Bool = true | ||
|
||
/// Resets all the "remembered" scroll offsets back to `.zero` for all embedded scrollable cells | ||
/// that conform to `ScrollOffsetManagable`. | ||
/// | ||
/// - Note: This will not actually perform any scrolling on the visible cells (Based on the value provided in `scrollVisibleCells`, | ||
/// but they will reset to `.zero` the next time that `cellForRow:` is called regardless of the value provided. | ||
/// - Parameter scrollVisibleCells: Whether to scroll the visible cells as well as resetting the cache. Defaults to `false` | ||
/// - Parameter animated: If `scrollVisibleCells == true`, whether we should animate the transition. Defaults to `false` | ||
public func resetEmbeddedScrollOffsets(scrollingVisibleCells: Bool = false, animated: Bool = false) { | ||
scrollOffsetManager.resetAllOffsets() | ||
guard scrollingVisibleCells else { return } | ||
tableView.visibleCells.forEach { (cell) in | ||
guard let scrollable = cell as? ScrollOffsetManagable else { return } | ||
scrollable.scrollView?.setContentOffset(.zero, animated: animated) | ||
} | ||
} | ||
|
||
func updateScrollPosition(cell: UITableViewCell, at indexPath: IndexPath) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No harm in an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done... and documented! |
||
|
||
guard rememberEmbeddedScrollPositions, let scrollable = cell as? ScrollOffsetManagable else { return } | ||
|
||
// Set the identifier on the scrollable so it can be tracked. Use `indexPath` for this | ||
scrollable.identifier = indexPath | ||
// Register the scrollable so it's offset is tracked | ||
scrollOffsetManager.register(scrollable: scrollable) | ||
// Set the scroll offset based on `scrollOffsetManager`. This fixes scroll re-use issues. | ||
scrollOffsetManager.setScrollOffset(scrollable, animated: false, fallback: .zero) | ||
} | ||
|
||
//MARK - variable header/footer size | ||
|
||
private var headerTranslatesAutoResizingMask: Bool = false | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
// | ||
// CollectionTableViewCell.swift | ||
// ThunderTableDemo | ||
// | ||
// Created by Simon Mitchell on 02/09/2020. | ||
// Copyright © 2020 3SidedCube. All rights reserved. | ||
// | ||
|
||
import UIKit | ||
import ThunderTable | ||
|
||
class CollectionTableViewCell: UITableViewCell, ScrollOffsetManagable, UICollectionViewDelegate { | ||
|
||
var scrollView: UIScrollView? { | ||
return collectionView | ||
} | ||
|
||
weak var scrollDelegate: ScrollOffsetDelegate? | ||
|
||
var identifier: AnyHashable? | ||
|
||
@IBOutlet weak var collectionView: UICollectionView! | ||
|
||
override func awakeFromNib() { | ||
super.awakeFromNib() | ||
collectionView.register(CollectionViewCell.self, forCellWithReuseIdentifier: "Cell") | ||
collectionView.delegate = self | ||
} | ||
|
||
func scrollViewDidScroll(_ scrollView: UIScrollView) { | ||
scrollDelegate?.scrollViewDidChangeContentOffset(self) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can defo see why if the implementor providing an identifier is real nice. Hence this. But it's a little annoying to have to provide it. Maybe I'm overlooking here, how's this going to work if some provide it and some don't?
Anyway we probably want some default implementations here? :) Like this and delegate as
nil
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So
TableViewController
actually sets this to the givenIndexPath
so all implementers have to do is provide the property. Not sure if that makes sense to you? It just means all information is stored on the cell itself, and we don't need to fetch different bits of info from different places. Maybe I need to make that clearer in the docs?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well Apple choose not to put an
IndexPath
property on theUITableViewCell
. That's where the manager would hold onto that mapping. I'll come back to