Unbound lists are made empty (#127)

* Isolate bug for #126 * Update tests to use latest DOM release * Tidy up template next/prev sibling usage * Assert nested lists are emptied with no data * Assert nested lists are emptied with empty nested data Closes #126
PhpGt · Jan 28, 2020 · eaef00d · eaef00d
1 parent ec45af4
commit eaef00d
Show file tree

Hide file tree

Showing 6 changed files with 106 additions and 47 deletions.
diff --git a/composer.lock b/composer.lock
diff --git a/src/DocumentFragment.php b/src/DocumentFragment.php
@@ -16,23 +16,23 @@ class DocumentFragment extends BaseDocumentFragment {
 	/** @var BaseElement */
 	public $templateParentNode;
 	/** @var BaseElement */
-	public $templateNextSibling;
+	public $templateNextElementSibling;
 	/** @var BaseElement */
-	public $templatePreviousSibling;
+	public $templatePreviousElementSibling;
 
 	/**
 	 * @param BaseNode $parentNode
-	 * @param BaseNode $nextSibling
-	 * @param BaseNode $previousSibling
+	 * @param BaseNode $nextElementSibling
+	 * @param BaseNode $previousElementSibling
 	 */
 	public function setTemplateProperties(
 		$parentNode = null,
-		$nextSibling = null,
-		$previousSibling = null
+		$nextElementSibling = null,
+		$previousElementSibling = null
 	):void {
 		$this->templateParentNode = $parentNode;
-		$this->templateNextSibling = $nextSibling;
-		$this->templatePreviousSibling = $previousSibling;
+		$this->templateNextElementSibling = $nextElementSibling;
+		$this->templatePreviousElementSibling = $previousElementSibling;
 	}
 
 	/**
@@ -43,7 +43,7 @@ public function insertTemplate(DOMNode $insertInto = null) {
 
 		if(is_null($insertInto)) {
 			$insertInto = $this->templateParentNode;
-			$insertBefore = $this->templateNextSibling;
+			$insertBefore = $this->templateNextElementSibling;
 		}
 		if(is_null($insertInto)) {
 			throw new TemplateHasNoParentException();
@@ -59,16 +59,4 @@ public function insertTemplate(DOMNode $insertInto = null) {
 
 		return $inserted;
 	}
-
-	public function prop_get_templateNextSibling() {
-		return $this->templateNextSibling;
-	}
-
-	public function prop_get_templatePreviousSibling() {
-		return $this->templatePreviousSibling;
-	}
-
-	public function prop_get_templateParentNode() {
-		return $this->templateParentNode;
-	}
 }
diff --git a/src/HTMLDocument.php b/src/HTMLDocument.php
@@ -61,8 +61,8 @@ public function getNamedTemplate(string $name):?DocumentFragment {
 			$clone = $fragment->cloneNode(true);
 			$clone->setTemplateProperties(
 				$fragment->templateParentNode,
-				$fragment->templateNextSibling,
-				$fragment->templatePreviousSibling
+				$fragment->templateNextElementSibling,
+				$fragment->templatePreviousElementSibling
 			);
 
 			return $clone;
@@ -105,8 +105,8 @@ public function getUnnamedTemplate(
 		$clone = $fragment->cloneNode(true);
 		$clone->setTemplateProperties(
 			$fragment->templateParentNode,
-			$fragment->templateNextSibling,
-			$fragment->templatePreviousSibling
+			$fragment->templateNextElementSibling,
+			$fragment->templatePreviousElementSibling
 		);
 
 		return $clone;
@@ -120,7 +120,6 @@ public function getParentOfUnnamedTemplate(
 // Unnamed templates can't have sibling elements of the same path, otherwise
 // they would need to be named. Remove any index from the path.
 		$path = preg_replace("/\[\d+\]/", "", $path);
-		$matches = [];
 		$pathToReturn = null;
 
 		foreach($this->templateFragmentMap as $name => $t) {
@@ -149,6 +148,7 @@ public function getParentOfUnnamedTemplate(
 			return null;
 		}
 
+		/** @var Element[] $matchingElements */
 		$matchingElements =  $this->xPath($pathToReturn);
 		return $matchingElements[count($matchingElements) - 1];
 	}

diff --git a/src/TemplateParent.php b/src/TemplateParent.php
@@ -24,8 +24,8 @@ public function extractTemplates(BaseElement $context = null):int {
 			$name = $this->getTemplateNameFromElement($templateElement);
 
 			$parentNode = $templateElement->parentNode;
-			$nextSibling = $templateElement->nextSibling;
-			$previousSibling = $templateElement->previousSibling;
+			$nextElementSibling = $templateElement->nextElementSibling;
+			$previousElementSibling = $templateElement->previousElementSibling;
 			$templateNodePath = $templateElement->getNodePath();
 
 			$nestedTemplateElementList = $templateElement->querySelectorAll(
@@ -45,8 +45,8 @@ public function extractTemplates(BaseElement $context = null):int {
 			);
 			$fragment->setTemplateProperties(
 				$parentNode,
-				$nextSibling,
-				$previousSibling
+				$nextElementSibling,
+				$previousElementSibling
 			);
 			$fragment->expandComponents();
 
@@ -67,6 +67,10 @@ public function extractTemplates(BaseElement $context = null):int {
 				$parentNode->removeChild($templateElement);
 			}
 
+			if(count($parentNode->children) === 0) {
+				$parentNode->innerHTML = "";
+			}
+
 			if($name[0] !== "/") {
 				$templateElement->classList->add("t-$name");
 			}
@@ -88,11 +92,17 @@ public function getTemplate(
 		string $templateDirectory = null,
 		bool $addTemplatePrefix = true
 	):DocumentFragment {
+		/** @var Element $element */
+		$element = $this;
+
 		/** @var HTMLDocument $rootDocument */
-		$rootDocument = $this->getRootDocument();
+		$rootDocument = $element->getRootDocument();
 
 		if(is_null($name)) {
-			$docTemplate = $rootDocument->getUnnamedTemplate($this, false);
+			$docTemplate = $rootDocument->getUnnamedTemplate(
+				$element,
+				false
+			);
 		}
 		else {
 			$docTemplate = $rootDocument->getNamedTemplate($name);
@@ -102,7 +112,7 @@ public function getTemplate(
 		}
 
 		if(is_null($templateDirectory)) {
-			$templateDirectory = $this->componentDirectory;
+			$templateDirectory = $element->componentDirectory;
 		}
 
 		if(is_dir($templateDirectory)) {
@@ -115,7 +125,7 @@ public function getTemplate(
 				$fileName = strtok($fileName, ".");
 
 				if($name === $fileName) {
-					$component = $this->loadComponent(
+					$component = $element->loadComponent(
 						$name,
 						dirname($fileInfo->getRealPath())
 					);

diff --git a/test/unit/Helper/Helper.php b/test/unit/Helper/Helper.php
@@ -356,6 +356,26 @@ class Helper {
 </main>
 HTML;
 
+	const HTML_NESTED_LIST = <<<HTML
+<!doctype html>
+<meta charset="utf-8" />
+<title>Nested list</title>
+<main>
+	<ul>
+		<li data-template class="outer">
+			<h1 data-bind:text>0:0</h1>
+			
+			<ol>
+				<li data-template class="inner">
+					<h2 data-bind:text>Inner title goes here</h2>				
+				</li>			
+			</ol>		
+		</li>
+	</ul>
+</main>
+HTML;
+
+
 	const HTML_PARENT_HAS_DATA_BIND_ATTR = <<<HTML
 <!doctype html>
 <meta charset="utf-8" />

diff --git a/test/unit/TemplateParentTest.php b/test/unit/TemplateParentTest.php
@@ -446,4 +446,45 @@ public function testTemplateWithinComponentIsAddedCorrectly() {
 
 		self::assertCount(3, $componentElement->children);
 	}
+
+	public function testExtractTemplatesSetsParentInnerHTMLToEmpty() {
+		$document = new HTMLDocument(Helper::HTML_TODO_LIST);
+		$document->extractTemplates();
+		$todoListElement = $document->getElementById("todo-list");
+		self::assertEmpty($todoListElement->innerHTML);
+	}
+
+	public function testExtractTemplatesSetsNestedParentInnerHTMLToEmpty() {
+		$document = new HTMLDocument(Helper::HTML_NESTED_LIST);
+		$document->extractTemplates();
+		$outerListElement = $document->querySelector("ul");
+		self::assertEmpty($outerListElement->innerHTML);
+	}
+
+	public function testExtractTemplatesSetsNestedParentInnerHTMLPartiallyToEmpty() {
+		$document = new HTMLDocument(Helper::HTML_NESTED_LIST);
+		$document->extractTemplates();
+		$outerListElement = $document->querySelector("ul");
+		$document->bindNestedList([
+			"Outer 1" => [
+				"1:1" => [],
+				"1:2" => [],
+				"1:3" => [],
+			],
+			"Outer 2" => [],
+			"Outer 3" => [
+				"3:1" => [
+					"Example" => (object)[
+						"name" => "Here I am!"
+					]
+				],
+				"3:2" => [],
+			],
+		]);
+
+		self::assertNotEmpty($outerListElement->innerHTML);
+		self::assertNotEmpty($outerListElement->querySelectorAll("ol")[0]->innerHTML);
+		self::assertNotEmpty($outerListElement->querySelectorAll("ol")[2]->innerHTML);
+		self::assertEmpty($outerListElement->querySelectorAll("ol")[1]->innerHTML);
+	}
 }