Skip to content

Commit ec28752

Browse files
crydotsnakecrydotsnake
committed
refactor: improve element moving logic
1 parent e8e74cb commit ec28752

6 files changed

Lines changed: 110 additions & 5 deletions

File tree

‎features/admin/page/sorting_content_elements_on_page.feature‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,35 @@ Feature: Sorting content elements on a page
5151
And I add a heading content element with type "h1" and "My Title" content
5252
And I add a textarea content element with "My body text" content
5353
Then the move down button of the 2nd content element should be disabled
54+
55+
@ui @javascript
56+
Scenario: Moving a content element down keeps its content when editing an existing page
57+
Given there is a page in the store with a textarea content element with "First content" content and a textarea content element with "Second content" content
58+
When I want to edit this page
59+
And I move the 1st content element down
60+
Then the 1st content element should contain "Second content"
61+
And the 2nd content element should contain "First content"
62+
63+
@ui @javascript
64+
Scenario: Moving a content element up keeps its content when editing an existing page
65+
Given there is a page in the store with a textarea content element with "First content" content and a textarea content element with "Second content" content
66+
When I want to edit this page
67+
And I move the 2nd content element up
68+
Then the 1st content element should contain "Second content"
69+
And the 2nd content element should contain "First content"
70+
71+
@ui @javascript @quill
72+
Scenario: Moving a content element down keeps its content with the Quill editor
73+
Given there is a page in the store with a textarea content element with "First content" content and a textarea content element with "Second content" content
74+
When I want to edit this page
75+
And I move the 1st content element down
76+
Then the 1st content element should contain "Second content"
77+
And the 2nd content element should contain "First content"
78+
79+
@ui @javascript @quill
80+
Scenario: Moving a content element up keeps its content with the Quill editor
81+
Given there is a page in the store with a textarea content element with "First content" content and a textarea content element with "Second content" content
82+
When I want to edit this page
83+
And I move the 2nd content element up
84+
Then the 1st content element should contain "Second content"
85+
And the 2nd content element should contain "First content"

‎src/Twig/Component/Trait/ContentElementsCollectionFormComponentTrait.php‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,28 @@ public function moveCollectionItem(
7070
return;
7171
}
7272

73-
$swapKey = $keys[$swapPos];
74-
[$data[$index], $data[$swapKey]] = [$data[$swapKey], $data[$index]];
73+
$items = array_values($data);
74+
[$items[$currentPos], $items[$swapPos]] = [$items[$swapPos], $items[$currentPos]];
75+
76+
// Give fresh keys to the two moved rows only, while keeping the visual (insertion)
77+
// order. New keys mean new DOM ids, so the Live Component re-creates exactly those
78+
// two rows instead of patching them in place. This keeps stateful WYSIWYG widgets
79+
// correct regardless of their morphing strategy: Trix opts out of morphing via
80+
// "data-live-ignore", while Quill builds its own DOM the server never renders - in
81+
// both cases an in-place patch would leave stale or corrupted content. Re-creating
82+
// the row triggers the editor's disconnect()/connect() cycle, which is the path it
83+
// is built to support. Untouched rows keep their keys (and initialized editors).
84+
$keys = array_keys($data);
85+
$freshIndex = $this->provideNewCollectionItemIndex($data);
86+
$keys[$currentPos] = $freshIndex;
87+
$keys[$swapPos] = $freshIndex + 1;
88+
89+
$reordered = [];
90+
foreach ($items as $position => $item) {
91+
$reordered[$keys[$position]] = $item;
92+
}
7593

76-
$propertyAccessor->setValue($this->formValues, $propertyPath, $data);
94+
$propertyAccessor->setValue($this->formValues, $propertyPath, $reordered);
7795
}
7896

7997
#[LiveAction]
@@ -105,7 +123,9 @@ public function insertCollectionItem(
105123
$data = [];
106124
}
107125

108-
ksort($data);
126+
// Do not sort by key here: the collection is rendered in insertion order, not key
127+
// order, and moveCollectionItem() intentionally produces non-monotonic keys. Sorting
128+
// would scramble the visual order after a move. array_values() preserves it.
109129
$values = array_values($data);
110130
$newItem = null === $type ? [] : ['type' => $type];
111131

‎tests/Behat/Context/Setup/PageContext.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,27 @@ public function thereIsAPageInTheStoreWithContentElements(string $contentElement
7878
$this->savePage($page);
7979
}
8080

81+
/**
82+
* @Given there is a page in the store with a textarea content element with :firstContent content and a textarea content element with :secondContent content
83+
*/
84+
public function thereIsAPageInTheStoreWithTwoTextareaContentElements(string $firstContent, string $secondContent): void
85+
{
86+
$page = $this->createPage();
87+
88+
foreach ([$firstContent, $secondContent] as $content) {
89+
/** @var ContentConfigurationInterface $contentConfiguration */
90+
$contentConfiguration = new ContentConfiguration();
91+
$contentConfiguration->setType('textarea');
92+
$contentConfiguration->setLocale('en_US');
93+
$contentConfiguration->setConfiguration(['textarea' => $content]);
94+
$contentConfiguration->setPage($page);
95+
96+
$page->addContentElement($contentConfiguration);
97+
}
98+
99+
$this->savePage($page);
100+
}
101+
81102
/**
82103
* @Given there is an existing page with :name name
83104
*/

‎tests/Behat/Context/Ui/Admin/ContentCollectionContext.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,17 @@ public function theContentElementAtPositionShouldBeOfType(string $ordinal, strin
217217
);
218218
}
219219

220+
/**
221+
* @Then the :ordinal content element should contain :content
222+
*/
223+
public function theContentElementAtPositionShouldContain(string $ordinal, string $content): void
224+
{
225+
Assert::contains(
226+
$this->contentElementsCollectionElement->getContentElementContentAtPosition($this->parseOrdinal($ordinal)),
227+
$content,
228+
);
229+
}
230+
220231
/**
221232
* @Then the move up button of the :ordinal content element should be disabled
222233
*/

‎tests/Behat/Element/Admin/ContentElementsCollectionElement.php‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,8 @@ public function moveContentElementUp(int $position): void
161161
{
162162
$button = $this->getSortButton($position, 'up');
163163
$button->click();
164+
165+
$this->waitForFormUpdate();
164166
}
165167

166168
public function getContentElementTypeAtPosition(int $position): string
@@ -174,6 +176,21 @@ public function getContentElementTypeAtPosition(int $position): string
174176
return $selectedOption->getText();
175177
}
176178

179+
public function getContentElementContentAtPosition(int $position): string
180+
{
181+
$elements = $this->getContentElements();
182+
Assert::keyExists($elements, $position - 1, sprintf('No content element at position %d.', $position));
183+
184+
// Read the visible WYSIWYG editor content instead of the (morphed) hidden input,
185+
// so the assertion reflects what the user actually sees after a move. Both supported
186+
// editors render the content into their own element: Trix into <trix-editor>, Quill
187+
// into ".ql-editor".
188+
$element = $elements[$position - 1];
189+
$editor = $element->find('css', 'trix-editor') ?? $element->find('css', '.ql-editor');
190+
191+
return $editor instanceof NodeElement ? $editor->getText() : $element->getText();
192+
}
193+
177194
public function insertContentElementAfterPosition(string $type, int $afterPosition): void
178195
{
179196
$this->insertContentElementAtDividerIndex($type, $afterPosition);
@@ -188,6 +205,8 @@ public function moveContentElementDown(int $position): void
188205
{
189206
$button = $this->getSortButton($position, 'down');
190207
$button->click();
208+
209+
$this->waitForFormUpdate();
191210
}
192211

193212
public function isContentElementMoveUpButtonDisabled(int $position): bool
@@ -223,7 +242,7 @@ private function getSortButton(int $position, string $direction): NodeElement
223242
$elements = $this->getContentElements();
224243
Assert::keyExists($elements, $position - 1, sprintf('No content element at position %d.', $position));
225244

226-
$button = $elements[$position - 1]->find('css', sprintf('[data-sort-direction="%s"]', $direction));
245+
$button = $elements[$position - 1]->find('css', sprintf('[data-live-direction-param="%s"]', $direction));
227246
Assert::notNull($button, sprintf('Sort %s button not found at position %d.', $direction, $position));
228247

229248
return $button;

‎tests/Behat/Element/Admin/ContentElementsCollectionElementInterface.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ public function moveContentElementDown(int $position): void;
4444

4545
public function getContentElementTypeAtPosition(int $position): string;
4646

47+
public function getContentElementContentAtPosition(int $position): string;
48+
4749
public function isContentElementMoveUpButtonDisabled(int $position): bool;
4850

4951
public function isContentElementMoveDownButtonDisabled(int $position): bool;

0 commit comments

Comments
 (0)