From a73c3574e2d93554a76ae7828bd5c773223bdf10 Mon Sep 17 00:00:00 2001 From: Loz Calver Date: Tue, 7 Jan 2014 16:02:43 +0000 Subject: [PATCH] FieldList->insertBefore/After now accept arguments in either order (fixes #2737) --- docs/en/reference/member.md | 2 +- forms/CompositeField.php | 8 ++++---- forms/FieldList.php | 24 ++++++++++++++++-------- forms/HtmlEditorField.php | 8 ++++---- forms/TabSet.php | 8 ++++---- tests/forms/CompositeFieldTest.php | 2 +- tests/forms/FieldListTest.php | 24 ++++++++++++++++++++++++ 7 files changed, 54 insertions(+), 22 deletions(-) diff --git a/docs/en/reference/member.md b/docs/en/reference/member.md index 7a5d818b3..ce081caf5 100644 --- a/docs/en/reference/member.md +++ b/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; diff --git a/forms/CompositeField.php b/forms/CompositeField.php index f47d3aa2e..408b45908 100644 --- a/forms/CompositeField.php +++ b/forms/CompositeField.php @@ -215,14 +215,14 @@ class CompositeField extends FormField { /** * @uses FieldList->insertBefore() */ - public function insertBefore($field, $insertBefore) { - $ret = $this->children->insertBefore($field, $insertBefore); + public function insertBefore($insertBefore, $field) { + $ret = $this->children->insertBefore($insertBefore, $field); $this->sequentialSet = null; return $ret; } - public function insertAfter($field, $insertAfter) { - $ret = $this->children->insertAfter($field, $insertAfter); + public function insertAfter($insertAfter, $field) { + $ret = $this->children->insertAfter($insertAfter, $field); $this->sequentialSet = null; return $ret; } diff --git a/forms/FieldList.php b/forms/FieldList.php index b7e535d6d..9e3e89ae1 100644 --- a/forms/FieldList.php +++ b/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++; diff --git a/forms/HtmlEditorField.php b/forms/HtmlEditorField.php index ad2a642e1..96c9c2a92 100644 --- a/forms/HtmlEditorField.php +++ b/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); diff --git a/forms/TabSet.php b/forms/TabSet.php index 9d7f73cfb..e6d6181f7 100644 --- a/forms/TabSet.php +++ b/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) { - parent::insertBefore($field, $insertBefore); + public function insertBefore($insertBefore, $field) { + parent::insertBefore($insertBefore, $field); if($field instanceof Tab) $field->setTabSet($this); $this->sequentialSet = null; } - public function insertAfter($field, $insertAfter) { - parent::insertAfter($field, $insertAfter); + public function insertAfter($insertAfter, $field) { + parent::insertAfter($insertAfter, $field); if($field instanceof Tab) $field->setTabSet($this); $this->sequentialSet = null; } diff --git a/tests/forms/CompositeFieldTest.php b/tests/forms/CompositeFieldTest.php index ad405e528..3193b2138 100644 --- a/tests/forms/CompositeFieldTest.php +++ b/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')); diff --git a/tests/forms/FieldListTest.php b/tests/forms/FieldListTest.php index 997bde8ed..31a99a8a5 100644 --- a/tests/forms/FieldListTest.php +++ 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() {