Merge pull request #151 from reflexxion/fix/renderable-order

BUGFIX: Reindex renderables on remove
neos · Mar 25, 2022 · ed43392 · ed43392
2 parents b40e709 + 45bc3c1
commit ed43392
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 13 deletions.
diff --git a/Classes/Core/Model/Renderable/AbstractCompositeRenderable.php b/Classes/Core/Model/Renderable/AbstractCompositeRenderable.php
@@ -70,22 +70,18 @@ protected function moveRenderableBefore(RenderableInterface $renderableToMove, R
         }
 
         $reorderedRenderables = [];
-        $i = 0;
         foreach ($this->renderables as $renderable) {
             if ($renderable === $renderableToMove) {
                 continue;
             }
 
             if ($renderable === $referenceRenderable) {
                 $reorderedRenderables[] = $renderableToMove;
-                $renderableToMove->setIndex($i);
-                $i++;
             }
             $reorderedRenderables[] = $renderable;
-            $renderable->setIndex($i);
-            $i++;
         }
         $this->renderables = $reorderedRenderables;
+        $this->reindexRenderables();
     }
 
     /**
@@ -107,23 +103,19 @@ protected function moveRenderableAfter(RenderableInterface $renderableToMove, Re
         }
 
         $reorderedRenderables = [];
-        $i = 0;
         foreach ($this->renderables as $renderable) {
             if ($renderable === $renderableToMove) {
                 continue;
             }
 
             $reorderedRenderables[] = $renderable;
-            $renderable->setIndex($i);
-            $i++;
 
             if ($renderable === $referenceRenderable) {
                 $reorderedRenderables[] = $renderableToMove;
-                $renderableToMove->setIndex($i);
-                $i++;
             }
         }
         $this->renderables = $reorderedRenderables;
+        $this->reindexRenderables();
     }
 
     /**
@@ -170,10 +162,25 @@ protected function removeRenderable(RenderableInterface $renderableToRemove)
             $updatedRenderables[] = $renderable;
         }
         $this->renderables = $updatedRenderables;
+        $this->reindexRenderables();
 
         $renderableToRemove->onRemoveFromParentRenderable();
     }
 
+    /**
+     * Iterate renderables and set index
+     *
+     * @return void
+     */
+    protected function reindexRenderables()
+    {
+        $i = 0;
+        foreach ($this->renderables as $renderable) {
+            $renderable->setIndex($i);
+            $i++;
+        }
+    }
+
     /**
      * Register this element at the parent form, if there is a connection to the parent form.
      *

diff --git a/Tests/Unit/Core/Model/FormDefinitionTest.php b/Tests/Unit/Core/Model/FormDefinitionTest.php
@@ -583,12 +583,16 @@ public function removePageRemovesPageFromForm()
         $formDefinition = new FormDefinition('foo1');
         $page1 = new Page('bar1');
         $page2 = new Page('bar2');
+        $page3 = new Page('bar3');
         $formDefinition->addPage($page1);
         $formDefinition->addPage($page2);
+        $formDefinition->addPage($page3);
 
-        $formDefinition->removePage($page1);
-        $this->assertNull($page1->getParentRenderable());
-        Assert::assertSame([$page2], $formDefinition->getPages());
+        $formDefinition->removePage($page2);
+        $this->assertNull($page2->getParentRenderable());
+        Assert::assertSame(0, $page1->getIndex());
+        Assert::assertSame(1, $page3->getIndex());
+        Assert::assertSame([$page1, $page3], $formDefinition->getPages());
     }
 
     /**