fix(layout): stable child order for Layout, flatten keyed :for in get…

src/component/base/utils.js

@@ -19,6 +19,21 @@ import symbols from '../../lib/symbols.js'
 
 import { renderer } from '../../launch.js'
 
+/** Keyed `:for` elms entries are plain objects of Blits elements; other POJOs stay as single items. */
+const flattenChildBuckets = (children) =>
+  (children || []).flatMap((child) => {
+    if (child == null || Object.getPrototypeOf(child) !== Object.prototype) return [child]
+    const vals = Object.values(child)
+    const isBucket =
+      vals.length > 0 &&
+      vals.every((v) => v != null && typeof v === 'object' && v.component !== undefined)
+    if (!isBucket) return [child]
+    return vals.map((c) => {
+      c.forComponent = c[Symbol.for('config')] && c[Symbol.for('config')][symbols.parent].component
+      return c
+    })
+  })
+
 export default {
   $size: {
     value: function (dimensions = { w: 0, h: 0 }) {
@@ -52,30 +67,17 @@ export default {
   [symbols.getChildren]: {
     value() {
       const parent = this[symbols.rootParent] || this[symbols.parent]
-      return (this[symbols.children] || []).concat(
+      return flattenChildBuckets(this[symbols.children] || []).concat(
         (parent &&
-          parent[symbols.getChildren]()
-            .map((child) => {
-              if (Object.getPrototypeOf(child) === Object.prototype) {
-                return Object.values(child).map((c) => {
-                  // ugly hack .. but the point is to reference the right component
-                  c.forComponent =
-                    c[Symbol.for('config')] && c[Symbol.for('config')][symbols.parent].component
-                  return c
-                })
-              }
-              return child
-            })
-            .flat()
-            .filter((child) => {
-              // problem is that component of a forloop in a slot has component of root component
-              if (child && child.component) {
-                return (
-                  (child.component && child.component.$componentId === this.$componentId) ||
-                  (child.forComponent && child.forComponent.$componentId === this.$componentId)
-                )
-              }
-            })) ||
+          flattenChildBuckets(parent[symbols.getChildren]()).filter((child) => {
+            // for-loop in slot: child may use forComponent instead of component
+            if (child && child.component) {
+              return (
+                child.component.$componentId === this.$componentId ||
+                (child.forComponent && child.forComponent.$componentId === this.$componentId)
+              )
+            }
+          })) ||
           []
       )
     },

src/engines/L3/element.js

@@ -99,17 +99,14 @@ const layoutFn = function (config) {
 
   let offset = padding.start
 
-  const children = this.node.children
-  const childrenLength = children.length
   const elementChildren = this.children
-
+  const n = elementChildren.length
   let otherDimension = 0
   const gap = config.gap || 0
-  for (let i = 0; i < childrenLength; i++) {
-    if (elementChildren[i] !== undefined && elementChildren[i].props.raw.show === false) {
-      continue
-    }
-    const node = children[i]
+  for (let i = 0; i < n; i++) {
+    const el = elementChildren[i]
+    if (el === undefined || el.props.raw.show === false) continue
+    const node = el.node
     node[position] = offset
     node[oppositePosition] = padding.oppositeStart
     // todo: temporary text check, due to 1px width of empty text node
@@ -134,8 +131,10 @@ const layoutFn = function (config) {
   }[config['align-items'] || 'start']
 
   if (align !== 0) {
-    for (let i = 0; i < childrenLength; i++) {
-      const node = children[i]
+    for (let i = 0; i < n; i++) {
+      const el = elementChildren[i]
+      if (el === undefined) continue
+      const node = el.node
       node[oppositePosition] = otherDimension * align
       node[oppositeMount] = align
     }

src/engines/L3/element.test.js

@@ -675,14 +675,7 @@ test('Element - Layout with horizontal direction layout use cases', (assert) =>
   // Creating two elements which will be children to parentEl
   const child1 = element({ parent: layoutEl }, {})
   const child2 = element({ parent: layoutEl }, {})
-
-  // To bind layout function to each children
-  child1.populate({})
-  child2.populate({})
-
-  // Adding children element nodes to layout Element node
-  layoutEl.node.children.push(child1.node)
-  layoutEl.node.children.push(child2.node)
+  registerLayoutChildren(layoutEl, child1, child2)
 
   // Initial width, x, y, height of each element is, 0
   // Setting child1 width to 500, should effect child 2 X position
@@ -746,14 +739,7 @@ test('Element - Layout with vertical direction use case', (assert) => {
   // Creating two elements which will be children to parentEl
   const child1 = element({ parent: layoutEl }, {})
   const child2 = element({ parent: layoutEl }, {})
-
-  // To bind layout function to each children
-  child1.populate({})
-  child2.populate({})
-
-  // Adding children element nodes to layout Element node
-  layoutEl.node.children.push(child1.node)
-  layoutEl.node.children.push(child2.node)
+  registerLayoutChildren(layoutEl, child1, child2)
 
   // Initial w, x, y, height of each element is, 0
   // Setting Child1 height to 500, should effect Child 2 Y position
@@ -1111,37 +1097,30 @@ function createElement(props = {}) {
 }
 
 function createLayoutElement(direction, gap, layoutUpdateSpy) {
-  let layoutElNode
-
-  // creating component object to fake children
   const comp = {
-    [symbols.getChildren]: () => {
-      return [
-        {
-          props: { raw: { show: true } },
-          parent: layoutElNode,
-        },
-        {
-          props: { raw: { show: true } },
-          parent: layoutElNode,
-        },
-      ]
+    __layoutChildElements: [],
+    [symbols.getChildren]() {
+      return this.__layoutChildElements
     },
   }
-
-  // element to break chain of parent element layoutFn trigger
   const grandParent = element({ parent: new EventEmitter(), props: {} }, {})
-
-  // Layout Element
   const layoutEl = element({ parent: grandParent }, comp)
-
-  // Populating layout element with configuration
   layoutEl.populate({ __layout: true, direction: direction, gap: gap, '@updated': layoutUpdateSpy })
-  layoutElNode = layoutEl.node
-
   return layoutEl
 }
 
+/** Registers layout test children: getChildren list, populate, mock renderer parent link. */
+function registerLayoutChildren(layoutEl, ...kids) {
+  const list = layoutEl.component.__layoutChildElements
+  for (let i = 0; i < kids.length; i++) {
+    const k = kids[i]
+    list.push(k)
+    k.populate({ parent: layoutEl })
+    layoutEl.node.children.push(k.node)
+    k.node.parent = layoutEl.node
+  }
+}
+
 // Test for lines 522-524: isSlot symbol handling
 test('Element - Set isSlot symbol', (assert) => {
   assert.capture(renderer, 'createNode', () => new EventEmitter())
@@ -1215,8 +1194,7 @@ test('Element - Transition with layout parent', (assert) => {
   const layoutSpy = sinon.spy()
   const layoutEl = createLayoutElement('horizontal', 10, layoutSpy)
   const childEl = element({ parent: layoutEl }, {})
-  childEl.populate({ parent: layoutEl })
-  layoutEl.node.children.push(childEl.node)
+  registerLayoutChildren(layoutEl, childEl)
   childEl.set('w', { transition: { value: 100, duration: 50 } })
   setTimeout(() => {
     assert.ok(layoutSpy.callCount > 0, 'Layout should be triggered on ticks')