From 5aa80f69a3d79a2ed25ddd15c132770402e70129 Mon Sep 17 00:00:00 2001
From: Kyle Van Essen <k@squareup.com>
Date: Tue, 9 Mar 2021 17:39:51 -0800
Subject: [PATCH] Code review from kb

---
 .../Sources/Item/ItemContentCoordinator.swift     | 15 ++++++++++-----
 ListableUI/Sources/ViewAnimation.swift            |  8 ++++----
 .../Tests/Item/ItemContentCoordinatorTests.swift  |  6 +++++-
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/ListableUI/Sources/Item/ItemContentCoordinator.swift b/ListableUI/Sources/Item/ItemContentCoordinator.swift
index d6c1e97ae..6a4a123c3 100644
--- a/ListableUI/Sources/Item/ItemContentCoordinator.swift
+++ b/ListableUI/Sources/Item/ItemContentCoordinator.swift
@@ -150,6 +150,9 @@ public final class ItemContentCoordinatorActions<Content:ItemContent>
     /// Allows you to update the displayed item via the provided closure, with an optional
     /// animation or delay.
     ///
+    /// Note that the `update` callback is invoked after the provided `delay`, and
+    /// is passed the value of the `Item` at that point in time.
+    ///
     /// ```
     /// func wasSelected() {
     ///    self.update(animation: .animated(0.15), after: 1.0) { item in
@@ -160,17 +163,19 @@ public final class ItemContentCoordinatorActions<Content:ItemContent>
     public func update(
         animation: ViewAnimation = .default,
         after delay: TimeInterval = 0,
-        update : (inout Item<Content>) -> ()
+        update : @escaping (inout Item<Content>) -> ()
     ) {
-        var new = self.currentProvider()
-        
-        update(&new)
-        
         if delay > 0 {
             Timer.scheduledTimer(withTimeInterval: delay, repeats: false) { _ in
+                var new = self.currentProvider()
+                update(&new)
+                
                 self.updateCallback(new, animation)
             }
         } else {
+            var new = self.currentProvider()
+            update(&new)
+            
             self.updateCallback(new, animation)
         }
     }
diff --git a/ListableUI/Sources/ViewAnimation.swift b/ListableUI/Sources/ViewAnimation.swift
index 94c3a76fa..398bca8e6 100644
--- a/ListableUI/Sources/ViewAnimation.swift
+++ b/ListableUI/Sources/ViewAnimation.swift
@@ -23,8 +23,8 @@ public enum ViewAnimation {
     case animated(TimeInterval = 0.25, options : Set<AnimationOptions> = .default)
     
     /// A spring based animation is performed.
-    /// The default parameters are 0.25 seconds and `UISpringTimingParameters()` timing.
-    case spring(TimeInterval = 0.25, timing : UISpringTimingParameters = .init())
+    /// The default value is `UISpringTimingParameters()`.
+    case spring(UISpringTimingParameters = .init())
     
     /// Ands the animation with the provided bool, returning the animation if true, and `.none` if false.
     public func and(with animated : Bool) -> ViewAnimation {
@@ -58,8 +58,8 @@ public enum ViewAnimation {
                 }
             )
             
-        case .spring(let duration, let timing):
-            let animator = UIViewPropertyAnimator(duration: duration, timingParameters: timing)
+        case .spring(let timing):
+            let animator = UIViewPropertyAnimator(duration: 0, timingParameters: timing)
             
             animator.addAnimations(animations)
             
diff --git a/ListableUI/Tests/Item/ItemContentCoordinatorTests.swift b/ListableUI/Tests/Item/ItemContentCoordinatorTests.swift
index 7bfa1ddf6..5a177381f 100644
--- a/ListableUI/Tests/Item/ItemContentCoordinatorTests.swift
+++ b/ListableUI/Tests/Item/ItemContentCoordinatorTests.swift
@@ -29,7 +29,11 @@ class ItemContentCoordinatorActionsTests : XCTestCase
                 $0.content.value = "update2"
             }
             
-            XCTAssertEqual(item.content.value, "update2")
+            actions.update {
+                $0.content.value = "update3"
+            }
+            
+            XCTAssertEqual(item.content.value, "update3")
             XCTAssertEqual(callbackCount, 2)
         }
     }