From dd1867e73725e5bf8b1cbff32a990cf3d7f73b4a Mon Sep 17 00:00:00 2001 From: Nataliya Patsovska Date: Mon, 9 May 2022 23:21:04 +0200 Subject: [PATCH 1/3] Fix issue #182 where a visible cell won't get reconfigured https://github.com/iZettle/Form/issues/182 --- Form/TableKit.swift | 65 ++++++++++++++++++++++---------- FormTests/TableChangeTests.swift | 36 ++++++++++++++++++ 2 files changed, 81 insertions(+), 20 deletions(-) diff --git a/Form/TableKit.swift b/Form/TableKit.swift index e98661e..855ddda 100644 --- a/Form/TableKit.swift +++ b/Form/TableKit.swift @@ -363,35 +363,60 @@ 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 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 { + if case let .row(.update(new, index)) = change, + indexPath == IndexPath(row: index.row, section: index.section), + let cell = view.cellForRow(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. diff --git a/FormTests/TableChangeTests.swift b/FormTests/TableChangeTests.swift index c122ff9..c124875 100644 --- a/FormTests/TableChangeTests.swift +++ b/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 { From f27844c9b71ba6bee5fce7459617b233bfafcfc1 Mon Sep 17 00:00:00 2001 From: Nataliya Patsovska Date: Tue, 10 May 2022 10:27:58 +0200 Subject: [PATCH 2/3] Add missing `updateBackground` when reconfiguring cells before animation --- Form/TableKit.swift | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/Form/TableKit.swift b/Form/TableKit.swift index 855ddda..0560a37 100644 --- a/Form/TableKit.swift +++ b/Form/TableKit.swift @@ -369,7 +369,7 @@ extension TableKit: TableAnimatable { view.animate(changes: changes, animation: animation) if reconfigureVisibleRowsIfNeeded() { - // To to refresh cells where the cell height has changed. + // To refresh cells where the cell height has changed. view.beginUpdates() view.endUpdates() } @@ -382,14 +382,18 @@ extension TableKit: TableAnimatable { var handledUpdates: [TableIndex] = [] for indexPath in view.indexPathsForVisibleRows ?? [] { for change in changes { - if case let .row(.update(new, index)) = change, - indexPath == IndexPath(row: index.row, section: index.section), - let cell = view.cellForRow(at: indexPath) { - let old = from[indexPath] - cell.reconfigure(old: old, new: new) - handledUpdates.append(index) - break - } + 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 } } return handledUpdates From daa59dfe1b70d35fba06bf4df3428883dd30111d Mon Sep 17 00:00:00 2001 From: Nataliya Patsovska Date: Mon, 23 May 2022 11:00:39 +0200 Subject: [PATCH 3/3] Bump version to 3.3.4 --- CHANGELOG.md | 3 ++- Form/Info.plist | 2 +- FormFramework.podspec | 2 +- FormTests/Info.plist | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6da219..b79dee5 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/Form/Info.plist b/Form/Info.plist index ca23c84..6bac8db 100644 --- a/Form/Info.plist +++ b/Form/Info.plist @@ -15,7 +15,7 @@ CFBundlePackageType FMWK CFBundleShortVersionString - $(MARKETING_VERSION) + 3.3.4 CFBundleSignature ???? CFBundleVersion diff --git a/FormFramework.podspec b/FormFramework.podspec index 3394447..8c37708 100644 --- a/FormFramework.podspec +++ b/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 diff --git a/FormTests/Info.plist b/FormTests/Info.plist index 470e0e7..b2a9012 100644 --- a/FormTests/Info.plist +++ b/FormTests/Info.plist @@ -15,7 +15,7 @@ CFBundlePackageType BNDL CFBundleShortVersionString - 3.3.2 + 3.3.4 CFBundleSignature ???? CFBundleVersion