diff --git a/CHANGELOG.md b/CHANGELOG.md index 79302529b..c6772690d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### Fixed - Fixed a bug that resulted in header/footer views not properly updating, by fixing the underlying tracking of collection view supplementary views. +- 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 diff --git a/ListableUI/Sources/Internal/PresentationState/PresentationState.HeaderFooterState.swift b/ListableUI/Sources/Internal/PresentationState/PresentationState.HeaderFooterState.swift index d1b2d05ee..49128916e 100644 --- a/ListableUI/Sources/Internal/PresentationState/PresentationState.HeaderFooterState.swift +++ b/ListableUI/Sources/Internal/PresentationState/PresentationState.HeaderFooterState.swift @@ -17,6 +17,8 @@ protocol AnyPresentationHeaderFooterState : AnyObject var oldIndexPath : IndexPath? { get } + var containsFirstResponder : Bool { get set } + func updateOldIndexPath(in section : Int) func dequeueAndPrepareReusableHeaderFooterView( @@ -157,6 +159,8 @@ extension PresentationState var oldIndexPath : IndexPath? = nil + var containsFirstResponder : Bool = false + func updateOldIndexPath(in section : Int) { oldIndexPath = kind.indexPath(in: section) } diff --git a/ListableUI/Sources/Layout/ListLayout/ListLayoutContent.swift b/ListableUI/Sources/Layout/ListLayout/ListLayoutContent.swift index 64f00bee8..c430f9b97 100644 --- a/ListableUI/Sources/Layout/ListLayout/ListLayoutContent.swift +++ b/ListableUI/Sources/Layout/ListLayout/ListLayoutContent.swift @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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), diff --git a/ListableUI/Sources/ListView/ListView.DataSource.swift b/ListableUI/Sources/ListView/ListView.DataSource.swift index 8631191f3..efb4cd7d2 100644 --- a/ListableUI/Sources/ListView/ListView.DataSource.swift +++ b/ListableUI/Sources/ListView/ListView.DataSource.swift @@ -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 + ) } }() diff --git a/ListableUI/Sources/ListView/ListView.swift b/ListableUI/Sources/ListView/ListView.swift index c44b50702..cb49055da 100644 --- a/ListableUI/Sources/ListView/ListView.swift +++ b/ListableUI/Sources/ListView/ListView.swift @@ -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 @@ -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 //