Merge pull request #402 from ualch9/list_prep

Fixes list view regressions

OBAKit/Controls/ListView/Delegates/OBAListViewCollapsibleSectionsDelegate.swift

@@ -26,7 +26,7 @@ public protocol OBAListViewCollapsibleSectionsDelegate: AnyObject {
     ///     - headerView: The header view that was selected. Due to the nature of `OBAListView`'s
     ///         handling of header views, you should update the view directly in this method.
     ///     - section: The section data.
-    func didTap(_ listView: OBAListView, headerView: OBAListRowViewHeader, section: OBAListViewSection)
+    func didTap(_ listView: OBAListView, section: OBAListViewSection)
 
     /// A helper function that handles toggling the collapsed states of sections. This method also updates the provided `headerView`.
     ///
@@ -50,7 +50,6 @@ public protocol OBAListViewCollapsibleSectionsDelegate: AnyObject {
     ///     - section: The section view model that was selected
     ///     - collapsedSections: The pointer to the collapsed sections of the current list view.
     func handleSectionCollapsibleState(
-        didTapHeader header: OBAListRowViewHeader,
         forSection section: OBAListViewSection,
         collapsedSections: inout Set<OBAListViewSection.ID>)
 }
@@ -61,22 +60,17 @@ extension OBAListViewCollapsibleSectionsDelegate {
         return true
     }
 
-    public func didTap(_ listView: OBAListView, headerView: OBAListRowViewHeader, section: OBAListViewSection) {
+    public func didTap(_ listView: OBAListView, section: OBAListViewSection) {
         guard canCollapseSection(listView, section: section) else { return }
 
         handleSectionCollapsibleState(
-            didTapHeader: headerView,
             forSection: section,
             collapsedSections: &collapsedSections)
 
-        // TODO: Animate changes. If we do animate changes right now, the fade animation is really confusing.
-        listView.applyData(animated: false)
-
         selectionFeedbackGenerator?.selectionChanged()
     }
 
     public func handleSectionCollapsibleState(
-        didTapHeader header: OBAListRowViewHeader,
         forSection section: OBAListViewSection,
         collapsedSections: inout Set<OBAListViewSection.ID>) {
 
@@ -88,8 +82,5 @@ extension OBAListViewCollapsibleSectionsDelegate {
             modifiedSection.collapseState = .collapsed
             collapsedSections.insert(section.id)
         }
-
-        // Update the view with the new state
-        header.section = modifiedSection
     }
 }

OBAKit/Controls/ListView/OBAListView.swift

@@ -99,6 +99,12 @@ public class OBAListView: UICollectionView, UICollectionViewDelegate {
             }
         }
 
+        dataSource.sectionSnapshotHandlers.shouldCollapseItem = { [unowned self] item in self.canCollapseOrExpandSection(item) }
+        dataSource.sectionSnapshotHandlers.shouldExpandItem   = { [unowned self] item in self.canCollapseOrExpandSection(item) }
+
+        dataSource.sectionSnapshotHandlers.willCollapseItem = { [unowned self] item in self.notifyDelegateOfCollapsingOrExpandingSection(item) }
+        dataSource.sectionSnapshotHandlers.willExpandItem   = { [unowned self] item in self.notifyDelegateOfCollapsingOrExpandingSection(item) }
+
         return dataSource
     }
 
@@ -132,16 +138,36 @@ public class OBAListView: UICollectionView, UICollectionViewDelegate {
     public func collectionView(_ collectionView: UICollectionView, didSelectItemAt indexPath: IndexPath) {
         var correctedItemIndex = indexPath.item
         if lastDataSourceSnapshot[indexPath.section].hasHeader {
-            if indexPath.item == 0 {
-                // TODO: inform collapsible section delegate
-                return
-            }
-
+            // If index path is zero, the user selected the header item.
+            // dataSource.sectionSnapshotHandler will handle notifying the
+            // collapsibleSectionDelegate of a collapsing section.
+            guard indexPath.item != 0 else { return }
             correctedItemIndex -= 1
         }
 
         let item = lastDataSourceSnapshot[indexPath.section][correctedItemIndex]
         item.onSelectAction?(item)
+
+        // Fixes #399 -- List view cells appears selected after presenting/pushing view controller
+        // This shouldn't be necessary and is a duct tape fix.
+        // Supporting native cell behavior is ideal, where the cell remains
+        // highlighted until after the view controller is popped off the stack.
+        // I suspect that OBAListView should be a UICollectionViewController
+        // instead of a UICollectionView for this to work elegantly.
+        collectionView.deselectItem(at: indexPath, animated: true)
+    }
+
+    // MARK: - Section collapse configuration
+    fileprivate func canCollapseOrExpandSection(_ item: AnyOBAListViewItem) -> Bool {
+        guard let header = item.as(OBAListViewHeader.self),
+              let section = self.lastDataSourceSnapshot.first(where: { $0.id == header.id }) else { return false }
+        return self.collapsibleSectionsDelegate?.canCollapseSection(self, section: section) ?? false
+    }
+
+    fileprivate func notifyDelegateOfCollapsingOrExpandingSection(_ item: AnyOBAListViewItem) {
+        guard let header = item.as(OBAListViewHeader.self),
+              let section = self.lastDataSourceSnapshot.first(where: { $0.id == header.id }) else { return }
+        self.collapsibleSectionsDelegate?.didTap(self, section: section)
     }
 
     // MARK: - Context menu configuration
@@ -278,6 +304,8 @@ public class OBAListView: UICollectionView, UICollectionViewDelegate {
 
             diffableDataSource.apply(sectionSnapshot, to: section, animatingDifferences: false)
         }
+
+        self.obaDelegate?.didApplyData(self)
     }
 
     fileprivate func emptyDataConfiguration(isEmpty: Bool) {
@@ -325,8 +353,11 @@ public class OBAListView: UICollectionView, UICollectionViewDelegate {
         self.register(reuseIdentifierProviding: cellType)
     }
 
-    public func didTap(_ headerView: OBAListRowViewHeader, section: OBAListViewSection) {
-        collapsibleSectionsDelegate?.didTap(self, headerView: headerView, section: section)
+    public func scrollTo(section: OBAListViewSection, at position: UICollectionView.ScrollPosition, animated: Bool) {
+        guard let lastItemOfSection = section.contents.last,
+              let indexPath = diffableDataSource.indexPath(for: lastItemOfSection) else { return }
+
+        self.scrollToItem(at: indexPath, at: position, animated: animated)
     }
 }
 

OBAKit/Recent/RecentStopsViewController.swift

@@ -141,7 +141,7 @@ public class RecentStopsViewController: UIViewController,
         }
 
         let title = application.userDataStore.alarms.count > 0 ? Strings.recentStops : nil
-        return OBAListViewSection(id: "recent_stops", title: title, contents: Set(rows).allObjects)
+        return OBAListViewSection(id: "recent_stops", title: title, contents: rows)
     }
 
     // MARK: - OBAListView

OBAKit/Stops/StopViewController.swift

@@ -451,24 +451,37 @@ public class StopViewController: UIViewController,
     }
 
     public func didApplyData(_ listView: OBAListView) {
+        // Due to an OBAListView bug, applying data causes the entire list view
+        // to reload, scrolling the user back to the top of the page.
+        // If the user initiated the applyData call from the "LOAD MORE" button,
+        // manually scroll the user back to the bottom of the arrDeps section
+        // to maintain UX continuity.
+        // Related 1: #389 -- OBAListView still has identity problems, causing crashes
+        // Related 2: https://github.com/OneBusAway/OBAKit/issues/389#issuecomment-867014676
+
+        if self.shouldScrollToBottomOfArrivalsDeparuresOnDataLoad {
+            listView.scrollTo(section: loadMoreSection, at: .top, animated: false)
+            shouldScrollToBottomOfArrivalsDeparuresOnDataLoad = false
+        }
         // This method will set up a UI affordance for showing the user how
         // they can swipe on a stop arrival cell to see more options.
         //
         // If the user has already seen the nudge, as determined by user
         // defaults, it will do nothing. Otherwise, an `AwesomeSpotlightView`
         // will be displayed one second after the stop data finishes loading.
 
-        guard application.userDefaults.bool(forKey: UserDefaultsKeys.shouldShowArrivalNudge) else {
-            return
-        }
-
-        for cell in listView.sortedVisibleCells {
-            if let cell = cell as? StopArrivalCell,
-               !cell.isShowingPastArrivalDeparture {
-                self.showSwipeOptionsNudge(on: cell)
-                return
-            }
-        }
+        // Disabled code: see #401 -- List view show nudge action doesn't work
+//        guard application.userDefaults.bool(forKey: UserDefaultsKeys.shouldShowArrivalNudge) else {
+//            return
+//        }
+//
+//        for cell in listView.sortedVisibleCells {
+//            if let cell = cell as? StopArrivalCell,
+//               !cell.isShowingPastArrivalDeparture {
+//                self.showSwipeOptionsNudge(on: cell)
+//                return
+//            }
+//        }
     }
 
     // MARK: - Data/Stop Header
@@ -686,13 +699,14 @@ public class StopViewController: UIViewController,
     }
 
     // MARK: - Data/Load More
-
+    private var shouldScrollToBottomOfArrivalsDeparuresOnDataLoad = false
     private var loadMoreSection: OBAListViewSection {
         let beforeTime = Date().addingTimeInterval(Double(minutesBefore) * -60.0)
         let afterTime = Date().addingTimeInterval(Double(minutesAfter) * 60.0)
         let footerText = application.formatters.formattedDateRange(from: beforeTime, to: afterTime)
 
         let item = MessageButtonItem(asLoadMoreButtonWithID: "load_more_item", error: operationError, footerText: footerText, showActivityIndicatorOnSelect: true) { [weak self] _ in
+            self?.shouldScrollToBottomOfArrivalsDeparuresOnDataLoad = true
             self?.loadMoreDepartures()
         }