BUG Updating node would position wrong, Sort isnt === offset

silverstripe · Jul 23, 2012 · 22d6c7a · 22d6c7a
1 parent 120de7c
commit 22d6c7a
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 5 deletions.
diff --git a/admin/code/LeftAndMain.php b/admin/code/LeftAndMain.php
@@ -748,12 +748,25 @@ public function updatetreenodes($request) {
 		foreach($ids as $id) {
 			$record = $this->getRecord($id);
 			$recordController = ($this->stat('tree_class') == 'SiteTree') ?  singleton('CMSPageEditController') : $this;
+
+			// Find the next & previous nodes, for proper positioning (Sort isn't good enough - it's not a raw offset)
+			// TODO: These methods should really be in hierarchy - for a start it assumes Sort exists
+			$next = $prev = null;
+
+			$className = $this->stat('tree_class');
+			$next = DataObject::get($className, 'ParentID = '.$record->ParentID.' AND Sort > '.$record->Sort)->first();
+			if (!$next) {
+				$prev = DataObject::get($className, 'ParentID = '.$record->ParentID.' AND Sort < '.$record->Sort)->reverse()->first();
+			}
+
 			$link = Controller::join_links($recordController->Link("show"), $record->ID);
 			$html = LeftAndMain_TreeNode::create($record, $link, $this->isCurrentPage($record))->forTemplate() . '</li>';
+
 			$data[$id] = array(
 				'html' => $html, 
 				'ParentID' => $record->ParentID,
-				'Sort' => $record->Sort
+				'NextID' => $next ? $next->ID : null,
+				'PrevID' => $prev ? $prev->ID : null
 			);
 		}
 		$this->response->addHeader('Content-Type', 'text/json');

diff --git a/admin/javascript/LeftAndMain.Tree.js b/admin/javascript/LeftAndMain.Tree.js
@@ -271,8 +271,11 @@
 			 *  (Object) Map of additional data, e.g. ParentID
 			 */
 			updateNode: function(node, html, data) {
-				var self = this, newNode = $(html), origClasses = node.attr('class'),
-					parentNode = data.ParentID ? this.find('li[data-id='+data.ParentID+']') : false;
+				var self = this, newNode = $(html), origClasses = node.attr('class');
+
+				var nextNode = data.NextID ? this.find('li[data-id='+data.NextID+']') : false;
+				var prevNode = data.PrevID ? this.find('li[data-id='+data.PrevID+']') : false;
+				var parentNode = data.ParentID ? this.find('li[data-id='+data.ParentID+']') : false;
 
 				// Copy attributes. We can't replace the node completely
 				// without removing or detaching its children nodes.
@@ -284,8 +287,15 @@
 				// Replace inner content
 				node.addClass(origClasses).html(newNode.html());
 
-				// Set correct parent
-				this.jstree('move_node', node, parentNode.length ? parentNode : -1, data.Sort);
+				if (nextNode && nextNode.length) {
+					this.jstree('move_node', node, nextNode, 'before');
+				}
+				else if (prevNode && prevNode.length) {
+					this.jstree('move_node', node, prevNode, 'after');
+				}
+				else {
+					this.jstree('move_node', node, parentNode.length ? parentNode : -1);
+				}
 			},
 
 			/**