Skip to content

Commit d582598

Browse files
committed
Fixed reordering, insertion and removal in ForEach where Element: Identifiable
No non-identifiable support in this commit # Conflicts: # Examples/Bundler.toml # Examples/Package.swift # Examples/Sources/ForEachExample/ForEachApp.swift # Sources/SwiftCrossUI/Views/ForEach.swift
1 parent 25c6e66 commit d582598

File tree

3 files changed

+54
-5
lines changed

3 files changed

+54
-5
lines changed

Examples/Package.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,6 @@ let package = Package(
8080
.executableTarget(
8181
name: "ForEachExample",
8282
dependencies: exampleDependencies
83-
)
83+
)
8484
]
8585
)

Examples/Sources/ForEachExample/ForEachApp.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ struct ForEachApp: App {
6969
}
7070
.padding(10)
7171
}
72-
.focusable()
7372
}
7473
}
7574
.defaultSize(width: 400, height: 800)

Sources/SwiftCrossUI/Views/ForEach.swift

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,15 +254,64 @@ extension ForEach: TypeSafeView, View where Child: View {
254254
// changed whereas it was actually just moved to a new slot in the array. Probably not
255255
// a huge issue, but definitely something to keep an eye on.
256256
var layoutableChildren: [LayoutSystem.LayoutableChild] = []
257-
for (i, node) in children.nodes.enumerated() {
258-
guard i < elements.count else {
259-
break
257+
258+
let oldNodes = children.nodes
259+
let oldMap = children.nodeIdentifierMap
260+
let oldIdentifiers = children.identifiers
261+
let identifiersStart = oldIdentifiers.startIndex
262+
263+
children.nodes = []
264+
children.nodeIdentifierMap = [:]
265+
children.identifiers = []
266+
267+
// Once this is true, every node that existed in the previous update and
268+
// still exists in the new one is reinserted to ensure that items are
269+
// rendered in the correct order.
270+
var requiresOngoingReinsertion = false
271+
272+
for (i, element) in elements.enumerated() {
273+
let childContent = child(element)
274+
let node: AnyViewGraphNode<Child>
275+
276+
if let oldNode = oldMap[element.id] {
277+
node = oldNode
278+
279+
// Checks if there is a preceding item that was not preceding in
280+
// the previous update. If such an item exists, it means that
281+
// the order of the collection has changed or that an item was
282+
// inserted somewhere in the middle, rather than simply appended.
283+
requiresOngoingReinsertion =
284+
requiresOngoingReinsertion
285+
|| {
286+
guard
287+
let ownOldIndex = oldIdentifiers.firstIndex(of: element.id)
288+
else { return false }
289+
290+
let subset = oldIdentifiers[identifiersStart..<ownOldIndex]
291+
return !children.identifiers.subtracting(subset).isEmpty
292+
}()
293+
294+
if requiresOngoingReinsertion {
295+
removeChild(oldNode.widget.into())
296+
addChild(oldNode.widget.into())
297+
}
298+
} else {
299+
// New Items need ongoing reinsertion to get
300+
// displayed at the correct location.
301+
requiresOngoingReinsertion = true
302+
node = AnyViewGraphNode(
303+
for: childContent,
304+
backend: backend,
305+
environment: environment
306+
)
307+
addChild(node.widget.into())
260308
}
261309
let index = elements.index(elements.startIndex, offsetBy: i)
262310
let childContent = child(elements[index])
263311
if children.isFirstUpdate {
264312
addChild(node.widget.into())
265313
}
314+
266315
layoutableChildren.append(
267316
LayoutSystem.LayoutableChild(
268317
update: { proposedSize, environment, dryRun in
@@ -276,6 +325,7 @@ extension ForEach: TypeSafeView, View where Child: View {
276325
)
277326
)
278327
}
328+
279329
children.isFirstUpdate = false
280330

281331
let nodeCount = children.nodes.count

0 commit comments

Comments
 (0)