Fixed ForEach [MenuItem] compatibility, documentation improvement & cleanup

sadly an argument name seems to be required on menuitem forEach initializer, the compiler is apparently unable to infer the right child from the context.

Author: MiaKoring

Sources/SwiftCrossUI/Builders/MenuItemsBuilder.swift

@@ -17,8 +17,8 @@ public struct MenuItemsBuilder {
         first.items
     }
 
-    public static func buildPartialBlock<Items: Collection>(
-        first: ForEach<Items, MenuItem, [MenuItem]>
+    public static func buildPartialBlock<Items: Collection, ID: Hashable>(
+        first: ForEach<Items, ID, [MenuItem]>
     ) -> [MenuItem] {
         first.elements.map(first.child).flatMap { $0 }
     }
@@ -51,9 +51,9 @@ public struct MenuItemsBuilder {
         accumulated + buildPartialBlock(first: next)
     }
 
-    public static func buildPartialBlock<Items: Collection>(
+    public static func buildPartialBlock<Items: Collection, ID: Hashable>(
         accumulated: [MenuItem],
-        next: ForEach<Items, MenuItem, [MenuItem]>
+        next: ForEach<Items, ID, [MenuItem]>
     ) -> [MenuItem] {
         accumulated + buildPartialBlock(first: next)
     }

Sources/SwiftCrossUI/Views/Button.swift

@@ -1,5 +1,3 @@
-import Foundation
-
 /// A control that initiates an action.
 public struct Button: Sendable {
     /// The label to show on the button.
@@ -23,8 +21,7 @@ public struct Button: Sendable {
     }
 }
 
-extension Button: View {
-}
+extension Button: View {}
 
 extension Button: ElementaryView {
     public func asWidget<Backend: AppBackend>(backend: Backend) -> Backend.Widget {

Sources/SwiftCrossUI/Views/ForEach.swift

@@ -83,6 +83,8 @@ extension ForEach: TypeSafeView, View where Child: View {
             children.queuedChanges = []
         }
 
+        // Use the previous update Method when no keyPath is set on a
+        // [Hashable] Collection to optionally keep the old behaviour.
         guard let idKeyPath else {
             return deprecatedUpdate(
                 widget,
@@ -109,24 +111,32 @@ extension ForEach: TypeSafeView, View where Child: View {
         // still exists in the new one is reinserted to ensure that items are
         // rendered in the correct order.
         var requiresOngoingReinsertion = false
+
+        // Forces node recreation when enabled (expensive on large Collections).
+        // Use only when idKeyPath yields non-unique values. Prefer Identifiable
+        // or guaranteed unique, constant identifiers for optimal performance.
+        // Node caching and diffing require unique, stable IDs.
         var ongoingNodeReusingDisabled = false
+
+        // Avoid reallocation
         var inserted = false
 
         for element in elements {
             let childContent = child(element)
             let node: AnyViewGraphNode<Child>
 
+            // Track duplicates: inserted=false if ID exists.
+            // Disables node reuse if any duplicate gets found.
             (inserted, _) = children.identifiers.append(element[keyPath: idKeyPath])
             ongoingNodeReusingDisabled = ongoingNodeReusingDisabled || !inserted
 
             if !ongoingNodeReusingDisabled {
                 if let oldNode = oldMap[element[keyPath: idKeyPath]] {
                     node = oldNode
 
-                    // Checks if there is a preceding item that was not preceding in
-                    // the previous update. If such an item exists, it means that
-                    // the order of the collection has changed or that an item was
-                    // inserted somewhere in the middle, rather than simply appended.
+                    // Detects reordering or mid-collection insertion:
+                    // Checks if there is a preceding item that was not
+                    // preceding in the previous update.
                     requiresOngoingReinsertion =
                         requiresOngoingReinsertion
                         || {
@@ -139,20 +149,24 @@ extension ForEach: TypeSafeView, View where Child: View {
                             return !children.identifiers.subtracting(subset).isEmpty
                         }()
 
-                    if requiresOngoingReinsertion {
+                    // Removes node from its previous position and
+                    // re-adds it at the new correct one.
+                    if requiresOngoingReinsertion, !children.isFirstUpdate {
                         removeChild(oldNode.widget.into())
                         addChild(oldNode.widget.into())
                     }
                 } else {
                     // New Items need ongoing reinsertion to get
-                    // displayed at the correct locat ion.
+                    // displayed at the correct location.
                     requiresOngoingReinsertion = true
                     node = AnyViewGraphNode(
                         for: childContent,
                         backend: backend,
                         environment: environment
                     )
-                    addChild(node.widget.into())
+                    if !children.isFirstUpdate {
+                        addChild(node.widget.into())
+                    }
                 }
                 children.nodeIdentifierMap[element[keyPath: idKeyPath]] = node
             } else {
@@ -165,10 +179,6 @@ extension ForEach: TypeSafeView, View where Child: View {
 
             children.nodes.append(node)
 
-            if children.isFirstUpdate, !ongoingNodeReusingDisabled {
-                addChild(node.widget.into())
-            }
-
             layoutableChildren.append(
                 LayoutSystem.LayoutableChild(
                     update: { proposedSize, environment, dryRun in
@@ -183,9 +193,11 @@ extension ForEach: TypeSafeView, View where Child: View {
             )
         }
 
-        children.isFirstUpdate = false
-
-        if !ongoingNodeReusingDisabled {
+        if children.isFirstUpdate {
+            for nodeToAdd in children.nodes {
+                addChild(nodeToAdd.widget.into())
+            }
+        } else if !ongoingNodeReusingDisabled {
             for removed in oldMap.filter({
                 !children.identifiers.contains($0.key)
             }).values {
@@ -200,6 +212,8 @@ extension ForEach: TypeSafeView, View where Child: View {
             }
         }
 
+        children.isFirstUpdate = false
+
         return LayoutSystem.updateStackLayout(
             container: widget,
             children: layoutableChildren,
@@ -434,7 +448,7 @@ extension ForEach where Child == [MenuItem], Items.Element: Hashable, ID == Item
     )
     @_disfavoredOverload
     public init(
-        _ elements: Items,
+        menuItems elements: Items,
         @MenuItemsBuilder _ child: @escaping (Items.Element) -> [MenuItem]
     ) {
         self.elements = elements
@@ -447,7 +461,7 @@ extension ForEach where Child == [MenuItem] {
     /// Creates a view that creates child views on demand based on a collection of data.
     @_disfavoredOverload
     public init(
-        _ elements: Items,
+        menuItems elements: Items,
         id keyPath: KeyPath<Items.Element, ID>,
         @MenuItemsBuilder _ child: @escaping (Items.Element) -> [MenuItem]
     ) {

Sources/SwiftCrossUI/Views/Menu.swift

@@ -26,11 +26,11 @@ public struct Menu: Sendable {
         ResolvedMenu(
             items: items.map { item in
                 switch item {
-                    case .button(let button, _):
+                    case .button(let button):
                         .button(button.label, button.action)
-                    case .text(let text, _):
+                    case .text(let text):
                         .button(text.string, nil)
-                    case .submenu(let submenu, _):
+                    case .submenu(let submenu):
                         .submenu(submenu.resolve())
                 }
             }

Sources/SwiftCrossUI/Views/MenuItem.swift

@@ -1,33 +1,6 @@
 /// An item of a ``Menu`` or ``CommandMenu``.
 public enum MenuItem: Sendable {
-    case button(Button, Int? = nil)
-    case text(Text, Int? = nil)
-    case submenu(Menu, Int? = nil)
-}
-
-extension MenuItem: Hashable {
-    public static func == (lhs: MenuItem, rhs: MenuItem) -> Bool {
-        lhs.hashValue == rhs.hashValue
-    }
-
-    public func hash(into hasher: inout Hasher) {
-        hasher.combine(
-            {
-                switch self {
-                    case .button(_, let int), .text(_, let int), .submenu(_, let int):
-                        return int ?? 0
-                }
-            }())
-    }
-
-    package func addingIDIfNeeded(id: Int) -> Self {
-        switch self {
-            case .button(let button, let int):
-                return .button(button, int ?? id)
-            case .text(let text, let int):
-                return .text(text, int ?? id)
-            case .submenu(let menu, let int):
-                return .submenu(menu, int ?? id)
-        }
-    }
+    case button(Button)
+    case text(Text)
+    case submenu(Menu)
 }