Skip to content

Commit

Permalink
fix: nested shuffle-contents as direct child
Browse files Browse the repository at this point in the history
  • Loading branch information
MHajoha committed Dec 3, 2024
1 parent b8f13d1 commit 20f17f0
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 11 deletions.
9 changes: 5 additions & 4 deletions classes/question_ui_renderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ private function shuffle_contents(): void {
if ($child instanceof DOMElement) {
$child = array_pop($childelements);
$newelement->appendChild($child);
$this->replace_shuffled_indices($child, $i++);
$this->replace_shuffled_indices($newelement, $child, $i++);
} else {
$newelement->appendChild($child);
}
Expand All @@ -188,16 +188,17 @@ private function shuffle_contents(): void {
/**
* Among the descendants of `$element`, finds `qpy:shuffled-index` elements and replaces them with `$index`.
*
* @param DOMNode $element
* @param DOMElement $container the element which has the currently handled `qpy:shuffle-contents` attribute
* @param DOMElement $element the shuffled element whose `qpy:shuffled-index` descendants should be replaced
* @param int $index
* @throws coding_exception
*/
private function replace_shuffled_indices(DOMNode $element, int $index): void {
private function replace_shuffled_indices(DOMElement $container, DOMElement $element, int $index): void {
/** @var DOMElement $indexelement */
foreach (iterator_to_array($this->xpath->query('.//qpy:shuffled-index', $element)) as $indexelement) {
// phpcs:ignore Squiz.ControlStructures.ForLoopDeclaration.SpacingAfterSecond
for (
$ancestor = $indexelement->parentNode; $ancestor !== null && $ancestor !== $element;
$ancestor = $indexelement->parentNode; $ancestor !== null && $ancestor !== $container;
$ancestor = $ancestor->parentNode
) {
assert($ancestor instanceof DOMElement);
Expand Down
18 changes: 11 additions & 7 deletions tests/question_ui_renderer_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -163,15 +163,19 @@ public function test_should_shuffle_correctly_and_replace_indices(): void {

$this->assert_html_string_equals_html_string(<<<EXPECTED
<div xmlns="http://www.w3.org/1999/xhtml">
<span>Element 3, shuffled to a</span>
<span>Element 4, shuffled to II</span>
<span>Element 2, shuffled to 3</span>
<span>Element 1, shuffled to 4</span>
<span>Element 1, shuffled to 1</span>
<span>Element 3, shuffled to b</span>
<div>
Element 5, shuffled to 5
<span>Nested element 1, shuffled to 1</span>
<span>Nested element 2, shuffled to 2</span>
</div>
<span>Element 2, shuffled to 4</span>
<span>Element 4, shuffled to V</span>
<div>
Element 5, shuffled to 6
<div>
<span>Nested element 1, shuffled to 1</span>
<span>Nested element 2, shuffled to 2</span>
<span>Nested element 2, shuffled to 1</span>
<span>Nested element 1, shuffled to 2</span>
</div>
</div>
</div>
Expand Down
5 changes: 5 additions & 0 deletions tests/question_uis/shuffle.xhtml
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,9 @@
<span>Nested element 2, shuffled to <qpy:shuffled-index/></span>
</div>
</div>
<div qpy:shuffle-contents="">
<!-- Nested shuffle being a direct child is an edge case we need to support. -->
<span>Nested element 1, shuffled to <qpy:shuffled-index/></span>
<span>Nested element 2, shuffled to <qpy:shuffled-index/></span>
</div>
</div>

0 comments on commit 20f17f0

Please sign in to comment.