[PanModal] Fix handleScrollViewTopBounce calls (slackhq#56)

* [PanModal] Fix calls to handleScrollViewTopBounce if scrollView is not decelerating * [PanModal] Replace `setContentOffset` with more generic `performUpdates` * [PanModal] Add appropriate comments
joekw · Oct 9, 2019 · af264eb · af264eb
1 parent 9d73db3
commit af264eb
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 30 deletions.
diff --git a/PanModal/Controller/PanModalPresentationController.swift b/PanModal/Controller/PanModalPresentationController.swift
@@ -258,35 +258,27 @@ public extension PanModalPresentationController {
     }
 
     /**
-     Set the content offset of the scroll view
+     Operations on the scroll view, such as content height changes,
+     or when inserting/deleting rows can cause the pan modal to jump,
+     caused by the pan modal responding to content offset changes.
 
-     Due to content offset observation, its not possible to programmatically
-     set the content offset directly on the scroll view while in the short form.
-
-     This method pauses the content offset KVO, performs the content offset change
-     and then resumes content offset observation.
+     To avoid this, you can call this method to perform scroll view updates,
+     with scroll observation temporarily disabled.
      */
-    func setContentOffset(offset: CGPoint) {
+    func performUpdates(_ updates: () -> Void) {
 
         guard let scrollView = presentable?.panScrollable
             else { return }
 
-        /**
-         Invalidate scroll view observer
-         to prevent its overriding the content offset change
-         */
+        // Pause scroll observer
         scrollObserver?.invalidate()
         scrollObserver = nil
 
-        /**
-         Set scroll view offset & track scrolling
-         */
-        scrollView.setContentOffset(offset, animated:false)
-        trackScrolling(scrollView)
+        // Perform updates
+        updates()
 
-        /**
-         Add the scroll view observer
-         */
+        // Resume scroll observer
+        trackScrolling(scrollView)
         observe(scrollView: scrollView)
     }
 
@@ -370,6 +362,8 @@ private extension PanModalPresentationController {
         panContainerView.frame.size = frame.size
 
         if ![shortFormYPosition, longFormYPosition].contains(panFrame.origin.y) {
+            // if the container is already in the correct position, no need to adjust positioning
+            // (rotations & size changes cause positioning to be out of sync)
             adjust(toYPosition: panFrame.origin.y - panFrame.height + frame.height)
         }
         panContainerView.frame.origin.x = frame.origin.x
@@ -789,7 +783,7 @@ private extension PanModalPresentationController {
      */
     func handleScrollViewTopBounce(scrollView: UIScrollView, change: NSKeyValueObservedChange<CGPoint>) {
 
-        guard let oldYValue = change.oldValue?.y
+        guard let oldYValue = change.oldValue?.y, scrollView.isDecelerating
             else { return }
 
         let yOffset = scrollView.contentOffset.y

diff --git a/PanModal/Presentable/PanModalPresentable+UIViewController.swift b/PanModal/Presentable/PanModalPresentable+UIViewController.swift
@@ -29,16 +29,6 @@ public extension PanModalPresentable where Self: UIViewController {
         presentedVC?.transition(to: state)
     }
 
-    /**
-     Programmatically set the content offset of the pan scrollable.
-
-     This is required to use while in the short form presentation state,
-     as due to content offset observation, setting the content offset directly would fail
-     */
-    func panModalSetContentOffset(offset: CGPoint) {
-        presentedVC?.setContentOffset(offset: offset)
-    }
-
     /**
      A function wrapper over the `setNeedsLayoutUpdate()`
      function in the PanModalPresentationController.
@@ -49,6 +39,16 @@ public extension PanModalPresentable where Self: UIViewController {
         presentedVC?.setNeedsLayoutUpdate()
     }
 
+    /**
+     Operations on the scroll view, such as content height changes, or when inserting/deleting rows can cause the pan modal to jump,
+     caused by the pan modal responding to content offset changes.
+
+     To avoid this, you can call this method to perform scroll view updates, with scroll observation temporarily disabled.
+     */
+    func panModalPerformUpdates(_ updates: () -> Void) {
+        presentedVC?.performUpdates(updates)
+    }
+
     /**
      A function wrapper over the animate function in PanModalAnimator.