Merge pull request #183 from iZettle/tablekit-reconfigure-fix

Fix issue #182 where a visible cell won't get reconfigured

CHANGELOG.md

@@ -1,4 +1,5 @@
-# 3.2.1
+# 3.3.4
+- Bug fix for a TableKit update issue [#182](https://github.com/iZettle/Form/issues/182)
 
 # 3.3.3
 - Xcode 13.2 compatibility

Form/Info.plist

@@ -15,7 +15,7 @@
 	<key>CFBundlePackageType</key>
 	<string>FMWK</string>
 	<key>CFBundleShortVersionString</key>
-	<string>$(MARKETING_VERSION)</string>
+	<string>3.3.4</string>
 	<key>CFBundleSignature</key>
 	<string>????</string>
 	<key>CFBundleVersion</key>

Form/TableKit.swift

@@ -363,35 +363,64 @@ extension TableKit: TableAnimatable {
                                    rowIdentifier: rowIdentifier,
                                    rowNeedsUpdate: rowNeedsUpdate ?? { _, _ in true })
 
+        // Apply the updates to the currently visible rows before animating other changes (see issue #182)
+        let handledUpdates = reconfigureVisibleRowsWithChanges()
+
         view.animate(changes: changes, animation: animation)
 
-        var hasReconfiguredCells = false
-        for indexPath in view.indexPathsForVisibleRows ?? [] {
-            guard let tableIndex = TableIndex(indexPath, in: self.table) else { continue }
-            let row = table[tableIndex]
-            guard let index = from.firstIndex(where: { rowIdentifier(row) == rowIdentifier($0) }) else { continue }
+        if reconfigureVisibleRowsIfNeeded() {
+            // To refresh cells where the cell height has changed.
+            view.beginUpdates()
+            view.endUpdates()
+        }
 
-            if let cell = view.cellForRow(at: indexPath) {
-                cell.updateBackground(forStyle: style, tableView: view, at: indexPath)
+        changesCallbacker.callAll(with: changes)
+        callbacker.callAll(with: table)
 
-                let old = from[index]
-                guard rowNeedsUpdate?(old, row) != false else {
-                    continue
+        // Returns the table indexes for which the cells were reconfigured
+        func reconfigureVisibleRowsWithChanges() -> [TableIndex] {
+            var handledUpdates: [TableIndex] = []
+            for indexPath in view.indexPathsForVisibleRows ?? [] {
+                for change in changes {
+                    guard case let .row(.update(new, index)) = change,
+                          indexPath == IndexPath(row: index.row, section: index.section),
+                          let cell = view.cellForRow(at: indexPath) else {
+                              continue
+                          }
+
+                    cell.updateBackground(forStyle: style, tableView: view, at: indexPath)
+
+                    let old = from[indexPath]
+                    cell.reconfigure(old: old, new: new)
+                    handledUpdates.append(index)
+                    break
                 }
-
-                cell.reconfigure(old: old, new: row)
-                hasReconfiguredCells = true
             }
+            return handledUpdates
         }
 
-        /// To to refresh cells where the cell height has changed.
-        if hasReconfiguredCells {
-            view.beginUpdates()
-            view.endUpdates()
-        }
+        func reconfigureVisibleRowsIfNeeded() -> Bool {
+            var hasReconfiguredCells = false
+            for indexPath in view.indexPathsForVisibleRows ?? [] {
+                guard let tableIndex = TableIndex(indexPath, in: self.table) else { continue }
+                guard !handledUpdates.contains(tableIndex) else { continue }
+                let row = table[tableIndex]
+                guard let index = from.firstIndex(where: { rowIdentifier(row) == rowIdentifier($0) }) else { continue }
 
-        changesCallbacker.callAll(with: changes)
-        callbacker.callAll(with: table)
+                if let cell = view.cellForRow(at: indexPath) {
+                    cell.updateBackground(forStyle: style, tableView: view, at: indexPath)
+
+                    let old = from[index]
+                    guard rowNeedsUpdate?(old, row) != false else {
+                        continue
+                    }
+
+                    cell.reconfigure(old: old, new: row)
+                    hasReconfiguredCells = true
+                }
+            }
+            return hasReconfiguredCells
+        }
     }
 
     /// Applies given changes to the Table and animates the changes using the provided parameters.

FormFramework.podspec

@@ -1,6 +1,6 @@
 Pod::Spec.new do |s|
   s.name         = "FormFramework"
-  s.version      = "3.3.2"
+  s.version      = "3.3.4"
   s.module_name  = "Form"
   s.summary      = "Powerful iOS layout and styling"
   s.description  = <<-DESC

FormTests/Info.plist

@@ -15,7 +15,7 @@
 	<key>CFBundlePackageType</key>
 	<string>BNDL</string>
 	<key>CFBundleShortVersionString</key>
-	<string>3.3.2</string>
+	<string>3.3.4</string>
 	<key>CFBundleSignature</key>
 	<string>????</string>
 	<key>CFBundleVersion</key>

FormTests/TableChangeTests.swift

@@ -172,6 +172,42 @@ class TableChangeTests: XCTestCase {
         tableKit.set(Table(rows: rows), rowIdentifier: { $0.id }, rowNeedsUpdate: { $0.value != $1.value })
         XCTAssertEqual(prevs, [nil, nil, 4, 5])
     }
+
+    func testReconfigure_isCalled_whenUpdatedRowIsPushedOutOfTheVisiblePaths() {
+        let bag = DisposeBag()
+        let row1 = ReconfigureItem(id: 1, value: 4)
+        let row2 = ReconfigureItem(id: 2, value: 5)
+        let row3 = ReconfigureItem(id: 3, value: 6)
+
+        var previousValues = [Int?]()
+        bag += merge([row1, row2, row3].map { Signal(callbacker: $0.callbacker) }).onValue { previousValues.append($0) }
+
+        let tableKit = TableKit<(), ReconfigureItem>()
+        UIWindow().addSubview(tableKit.view)
+        tableKit.view.frame.origin = CGPoint(x: 100, y: 100)
+        tableKit.view.frame.size = CGSize(width: 1000, height: 1000)
+
+        func applyRowsToKit(_ rows: [ReconfigureItem]) {
+            tableKit.set(Table(rows: rows), rowIdentifier: { $0.id }, rowNeedsUpdate: { $0.value != $1.value })
+        }
+
+        applyRowsToKit([row1, row2])
+        let initiaLoadPreviousValues = previousValues
+
+        tableKit.view.configureSizeForVisibleRows(2)
+
+        var row2Updated = row2
+        row2Updated.value = 55
+        applyRowsToKit([row1, row3, row2Updated]) // update (5 -> 55) and insert (nil -> 6) at the same time
+        XCTAssertEqual(previousValues, initiaLoadPreviousValues + [5, nil])
+    }
+}
+
+extension UITableView {
+    func configureSizeForVisibleRows(_ visibleRows: Int) {
+        self.frame.size = self.systemLayoutSizeFitting(UIView.layoutFittingExpandedSize)
+        self.frame.size.height = (self.visibleCells.first?.frame.size.height ?? 0) * CGFloat(visibleRows)
+    }
 }
 
 private struct ReconfigureItem: Reusable {