Merge pull request #2759 from kinglozzer/fieldlist-argument-ordering

FieldList->insertBefore/After now accept arguments in either order (fixes #2737)

docs/en/reference/member.md

@@ -69,7 +69,7 @@ parent::getCMSFields() and manipulate the `[api:FieldList]` from there.
 	:::php
 	public function getCMSFields() {
 		$fields = parent::getCMSFields();
-		$fields->insertBefore(new TextField("Age"), "HTMLEmail");
+		$fields->insertBefore("HTMLEmail", new TextField("Age"));
 		$fields->removeByName("JobTitle");
 		$fields->removeByName("Organisation");
 		return $fields;

forms/CompositeField.php

@@ -215,14 +215,14 @@ class CompositeField extends FormField {
 	/**
 	 * @uses FieldList->insertBefore()
 	 */
-	public function insertBefore($field, $insertBefore) {
+	public function insertBefore($insertBefore, $field) {
-		$ret = $this->children->insertBefore($field, $insertBefore);
+		$ret = $this->children->insertBefore($insertBefore, $field);
 		$this->sequentialSet = null;
 		return $ret;
 	}
 
-	public function insertAfter($field, $insertAfter) {
+	public function insertAfter($insertAfter, $field) {
-		$ret = $this->children->insertAfter($field, $insertAfter);
+		$ret = $this->children->insertAfter($insertAfter, $field);
 		$this->sequentialSet = null;
 		return $ret;
 	}

forms/FieldList.php

@@ -106,7 +106,7 @@ class FieldList extends ArrayList {
 		$tab = $this->findOrMakeTab($tabName);
 
 		// Add the field to the end of this set
-		if($insertBefore) $tab->insertBefore($field, $insertBefore);
+		if($insertBefore) $tab->insertBefore($insertBefore, $field);
 		else $tab->push($field);
 	}
 	
@@ -129,7 +129,7 @@ class FieldList extends ArrayList {
 		foreach($fields as $field) {
 			// Check if a field by the same name exists in this tab
 			if($insertBefore) {
-				$tab->insertBefore($field, $insertBefore);
+				$tab->insertBefore($insertBefore, $field);
 			} elseif(($name = $field->getName()) && $tab->fieldByName($name)) {
 				// It exists, so we need to replace the old one
 				$this->replaceField($field->getName(), $field);
@@ -354,10 +354,14 @@ class FieldList extends ArrayList {
 	/**
 	 * Inserts a field before a particular field in a FieldList.
 	 *
-	 * @param FormField $item The form field to insert
 	 * @param string $name Name of the field to insert before
+	 * @param FormField $item The form field to insert
 	 */
-	public function insertBefore($item, $name) {
+	public function insertBefore($name, $item) {
+		// Backwards compatibility for order of arguments
+		if($name instanceof FormField) {
+			list($item, $name) = array($name, $item);
+		}
 		$this->onBeforeInsert($item);
 		$item->setContainerFieldList($this);
 		
@@ -367,7 +371,7 @@ class FieldList extends ArrayList {
 				array_splice($this->items, $i, 0, array($item));
 				return $item;
 			} elseif($child->isComposite()) {
-				$ret = $child->insertBefore($item, $name);
+				$ret = $child->insertBefore($name, $item);
 				if($ret) return $ret;
 			}
 			$i++;
@@ -379,10 +383,14 @@ class FieldList extends ArrayList {
 	/**
 	 * Inserts a field after a particular field in a FieldList.
 	 *
-	 * @param FormField $item The form field to insert
 	 * @param string $name Name of the field to insert after
+	 * @param FormField $item The form field to insert
 	 */
-	public function insertAfter($item, $name) {
+	public function insertAfter($name, $item) {
+		// Backwards compatibility for order of arguments
+		if($name instanceof FormField) {
+			list($item, $name) = array($name, $item);
+		}
 		$this->onBeforeInsert($item);
 		$item->setContainerFieldList($this);
 		
@@ -392,7 +400,7 @@ class FieldList extends ArrayList {
 				array_splice($this->items, $i+1, 0, array($item));
 				return $item;
 			} elseif($child->isComposite()) {
-				$ret = $child->insertAfter($item, $name);
+				$ret = $child->insertAfter($name, $item);
 				if($ret) return $ret;
 			}
 			$i++;

forms/HtmlEditorField.php

@@ -594,16 +594,16 @@ class HtmlEditorField_Toolbar extends RequestHandler {
 		$urlField->dontEscape = true;
 
 		if($file->Type == 'photo') {
-			$fields->insertBefore(new TextField(
+			$fields->insertBefore('CaptionText', new TextField(
 				'AltText', 
 				_t('HtmlEditorField.IMAGEALTTEXT', 'Alternative text (alt) - shown if image can\'t be displayed'), 
 				$file->Title, 
 				80
-			), 'CaptionText');
+			));
-			$fields->insertBefore(new TextField(
+			$fields->insertBefore('CaptionText', new TextField(
 				'Title', 
 				_t('HtmlEditorField.IMAGETITLE', 'Title text (tooltip) - for additional information about the image')
-			), 'CaptionText');
+			));
 		}
 
 		$this->extend('updateFieldsForOembed', $fields, $url, $file);

forms/TabSet.php

@@ -151,14 +151,14 @@ class TabSet extends CompositeField {
 	 * @param FormField $item The form field to insert
 	 * @param string $name Name of the field to insert before
 	 */
-	public function insertBefore($field, $insertBefore) {
+	public function insertBefore($insertBefore, $field) {
-		parent::insertBefore($field, $insertBefore);
+		parent::insertBefore($insertBefore, $field);
 		if($field instanceof Tab) $field->setTabSet($this);
 		$this->sequentialSet = null;
 	}
 	
-	public function insertAfter($field, $insertAfter) {
+	public function insertAfter($insertAfter, $field) {
-		parent::insertAfter($field, $insertAfter);
+		parent::insertAfter($insertAfter, $field);
 		if($field instanceof Tab) $field->setTabSet($this);
 		$this->sequentialSet = null;
 	}

tests/forms/CompositeFieldTest.php

@@ -23,7 +23,7 @@ class CompositeFieldTest extends SapphireTest {
 		$this->assertEquals(0, $compositeInner->fieldPosition('C1'));
 		$this->assertEquals(1, $compositeInner->fieldPosition('C2'));
 		
-		$compositeOuter->insertBefore(new TextField('AB'), 'B');
+		$compositeOuter->insertBefore('B', new TextField('AB'));
 		$this->assertEquals(0, $compositeOuter->fieldPosition('A'));
 		$this->assertEquals(1, $compositeOuter->fieldPosition('AB'));
 		$this->assertEquals(2, $compositeOuter->fieldPosition('B'));

tests/forms/FieldListTest.php

@@ -401,6 +401,18 @@ class FieldListTest extends SapphireTest {
 		
 		/* The position of the Title field is at number 3 */
 		$this->assertEquals('Title', $fields[2]->getName());
+
+		/* Test arguments are accepted in either order */
+		$fields->insertBefore('FirstName', new TextField('Surname'));
+
+		/* The field we just added actually exists in the set */
+		$this->assertNotNull($fields->dataFieldByName('Surname'));
+
+		/* We now have 5 fields in the set */
+		$this->assertEquals(5, $fields->Count());
+
+		/* The position of the Surname field is at number 4 */
+		$this->assertEquals('Surname', $fields[3]->getName());
 	}
 
 	public function testInsertBeforeMultipleFields() {
@@ -451,6 +463,18 @@ class FieldListTest extends SapphireTest {
 		
 		/* The position of the Title field should be at number 2 */
 		$this->assertEquals('Title', $fields[1]->getName());
+
+		/* Test arguments are accepted in either order */
+		$fields->insertAfter('FirstName', new TextField('Surname'));
+
+		/* The field we just added actually exists in the set */
+		$this->assertNotNull($fields->dataFieldByName('Surname'));
+
+		/* We now have 5 fields in the set */
+		$this->assertEquals(5, $fields->Count());
+
+		/* The position of the Surname field is at number 5 */
+		$this->assertEquals('Surname', $fields[4]->getName());
 	}
 
 	public function testrootFieldList() {