Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

### Fixed

- Fixed an issue where supplementary views (headers or footers) that contained a first responder would result in the view being duplicated when scrolled off-screen.

### Added

### Removed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ protocol AnyPresentationHeaderFooterState : AnyObject

var oldIndexPath : IndexPath? { get }

var containsFirstResponder : Bool { get set }

func updateOldIndexPath(in section : Int)

func dequeueAndPrepareReusableHeaderFooterView(
Expand Down Expand Up @@ -132,6 +134,8 @@ extension PresentationState

var oldIndexPath : IndexPath? = nil

var containsFirstResponder : Bool = false

func updateOldIndexPath(in section : Int) {
oldIndexPath = kind.indexPath(in: section)
}
Expand Down
6 changes: 6 additions & 0 deletions ListableUI/Sources/Internal/UIView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,10 @@ extension UIView {

return nil
}

func isInside(superview : UIView) -> Bool {

sequence(first: self, next: \.superview)
.contains(superview)
}
}
16 changes: 11 additions & 5 deletions ListableUI/Sources/Layout/ListLayout/ListLayoutContent.swift
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ public final class ListLayoutContent

// Container Header

if rect.intersects(self.containerHeader.visibleFrame) && include(self.containerHeader) {
if (rect.intersects(self.containerHeader.visibleFrame) || self.containerHeader.containsFirstResponder) && include(self.containerHeader) {
attributes.append(
.supplementary(
self.containerHeader,
Expand All @@ -153,7 +153,7 @@ public final class ListLayoutContent

// List Header

if rect.intersects(self.header.visibleFrame) && include(self.header) {
if (rect.intersects(self.header.visibleFrame) || self.header.containsFirstResponder) && include(self.header) {
attributes.append(
.supplementary(
self.header,
Expand All @@ -172,7 +172,7 @@ public final class ListLayoutContent

// Section Header

if rect.intersects(section.header.visibleFrame) && include(section.header) {
if (rect.intersects(section.header.visibleFrame) || section.header.containsFirstResponder) && include(section.header) {
attributes.append(
.supplementary(
section.header,
Expand All @@ -196,7 +196,7 @@ public final class ListLayoutContent

// Section Footer

if rect.intersects(section.footer.visibleFrame) && include(section.footer) {
if (rect.intersects(section.footer.visibleFrame) || section.footer.containsFirstResponder) && include(section.footer) {
attributes.append(
.supplementary(
section.footer,
Expand All @@ -208,7 +208,7 @@ public final class ListLayoutContent

// List Footer

if rect.intersects(self.footer.visibleFrame) && include(self.footer) {
if (rect.intersects(self.footer.visibleFrame) || self.footer.containsFirstResponder) && include(self.footer) {
attributes.append(
.supplementary(
self.footer,
Expand All @@ -220,8 +220,10 @@ public final class ListLayoutContent
// Overscroll Footer

if alwaysIncludeOverscroll || (rect.intersects(self.overscrollFooter.visibleFrame) && include(self.overscrollFooter)) {

// Don't check the rect for the overscroll view as we do with other views; it's always outside of the contentSize.
// Instead, just return it all the time to ensure the collection view will display it when needed.

attributes.append(
.supplementary(
self.overscrollFooter,
Expand Down Expand Up @@ -436,6 +438,10 @@ extension ListLayoutContent
self.state?.anyModel.layouts ?? .init()
}

public var containsFirstResponder : Bool {
self.state?.containsFirstResponder ?? false
}

public var defaultFrame : CGRect {
CGRect(
origin: CGPoint(x: self.x, y: self.y),
Expand Down
74 changes: 59 additions & 15 deletions ListableUI/Sources/ListView/ListView.DataSource.swift
Original file line number Diff line number Diff line change
Expand Up @@ -57,22 +57,66 @@ internal extension ListView
at indexPath: IndexPath
) -> UICollectionReusableView
{
let container = SupplementaryContainerView.dequeue(
in: collectionView,
for: kind,
at: indexPath,
reuseCache: self.headerFooterReuseCache,
environment: self.view.environment
)

let headerFooter : AnyPresentationHeaderFooterState? = {
let statePair : PresentationState.HeaderFooterViewStatePair = {
switch SupplementaryKind(rawValue: kind)! {
case .listContainerHeader: return self.presentationState.containerHeader.state
case .listHeader: return self.presentationState.header.state
case .listFooter: return self.presentationState.footer.state
case .sectionHeader: return self.presentationState.sections[indexPath.section].header.state
case .sectionFooter: return self.presentationState.sections[indexPath.section].footer.state
case .overscrollFooter: return self.presentationState.overscrollFooter.state
case .listContainerHeader: return self.presentationState.containerHeader
case .listHeader: return self.presentationState.header
case .listFooter: return self.presentationState.footer
case .sectionHeader: return self.presentationState.sections[indexPath.section].header
case .sectionFooter: return self.presentationState.sections[indexPath.section].footer
case .overscrollFooter: return self.presentationState.overscrollFooter
}
}()

let headerFooter = statePair.state

let container : SupplementaryContainerView = {

/// The below works around a (seeming?) bug or odd behavior in `UICollectionView`,
/// where it tries to be smart about recycling supplementary views that contain a
/// first responder such as a text field. Specifically, it holds onto a supplementary view
/// that contains a first responder, not immediately recycling it when it is scrolled out
/// of view. That ensures that the keyboard isn't immediately dismissed, which would
/// be jarring.
///
/// ...Unfortunately, this doesn't seem to actually work in practice very well. When the
/// supplementary view is scrolled back _into_ view, and we're asked to dequeue
/// a view, the collection view hands us back a _different_ view, leading to double
/// views that get stacked on top of each other in the layout, leading to a bunch
/// of weirdness.
///
/// So, to work around this, we do a few things:
///
/// 1) We begin tracking which supplementary views currently contain a first responder.
/// For practicality of implementation, we only track text fields right now. This could
/// change, but is harder, given there's no generic "first responder changed" notification.
/// This code lives in `ListView`.
///
/// 2) We update `ListLayoutContent.content(in: ...)` to _always_ return
/// supplementary info when a supplementary view contains a first responder,
/// even when out of frame. This ensures the supplementary view
/// instance is kept alive by the collection view.
///
/// 3) Within this method, we check to see if there's a live, existing `visibleContainer`
/// (aka the supplementary view) view, and if there is, we return _that_, instead of
/// just dequeuing a new, wrong view.
///
/// After all that, the correct thing happens.
///
/// PR with more info and screenshots, etc:
/// https://github.com/square/Listable/pull/507
///

if let view = statePair.visibleContainer {
return view
} else {
return SupplementaryContainerView.dequeue(
in: collectionView,
for: kind,
at: indexPath,
reuseCache: self.headerFooterReuseCache,
environment: self.view.environment
)
}
}()

Expand Down
46 changes: 46 additions & 0 deletions ListableUI/Sources/ListView/ListView.swift
Copy link
Member

Choose a reason for hiding this comment

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

For practicality of implementation, we only track text fields right now. This could change, but is harder, given there's no generic "first responder changed" notification.

If we really need to, we can try out the UIWindowFirstResponderDidChangeNotification private api. I think scoping to text fields is totally fine though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

of course it's from Peter Steinberger

Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,26 @@ public final class ListView : UIView
self.applyAppearance()
self.applyBehavior()
self.updateScrollViewInsets()

/// We track first responder status in supplementary views
/// to fix a view recycling issue.
///
/// See the comment in `collectionView(_:viewForSupplementaryElementOfKind:at:)
/// within `ListView.DataSource.swift` for more.

NotificationCenter.default.addObserver(
self,
selector: #selector(textDidBeginEditingNotification(_:)),
name: UITextField.textDidBeginEditingNotification,
object: nil
)

NotificationCenter.default.addObserver(
self,
selector: #selector(textDidEndEditingNotification(_:)),
name: UITextField.textDidEndEditingNotification,
object: nil
)
}

deinit
Expand Down Expand Up @@ -975,6 +995,32 @@ public final class ListView : UIView
self.updateScrollViewInsets()
}

//
// MARK: Internal – First Responder Tracking
//

@objc private func textDidBeginEditingNotification(_ notification : Notification) {

guard let field = notification.object as? UIView else {
return
}

if let containingSupplementaryView = field.firstSuperview(ofType: SupplementaryContainerView.self) {
containingSupplementaryView.headerFooter?.containsFirstResponder = true
}
}

@objc private func textDidEndEditingNotification(_ notification : Notification) {

guard let field = notification.object as? UIView else {
return
}

if let containingSupplementaryView = field.firstSuperview(ofType: SupplementaryContainerView.self) {
containingSupplementaryView.headerFooter?.containsFirstResponder = false
}
}

//
// MARK: Internal – Swipe To Delete
//
Expand Down