tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-06-28 23:49:26 +02:00
Code Issues Projects Releases Wiki Activity

FIX: Append any fields that don’t match name in insertBefore/insertAfter

Browse Source 
Previous behaviour was to return false, which has been described as
a confusing bug on https://github.com/silverstripe/silverstripe-framework/issues/1397
where the issue was identified.
This commit is contained in:
Sam Minnee 2018-10-01 18:20:04 +13:00
parent 755907d117
commit 71dad5f685
4 changed files with 42 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/Forms/CompositeField.php
View File

@ -347,9 +347,9 @@ class CompositeField extends FormField
* @param FormField $field * @param FormField $field
* @return false|FormField * @return false|FormField
*/ */
public function insertBefore($insertBefore, $field) public function insertBefore($insertBefore, $field, $appendIfMissing = true)
{ {
return $this->children->insertBefore($insertBefore, $field); return $this->children->insertBefore($insertBefore, $field, $appendIfMissing);
} }
/** /**
@ -358,9 +358,9 @@ class CompositeField extends FormField
* @param FormField $field * @param FormField $field
* @return false|FormField * @return false|FormField
*/ */
public function insertAfter($insertAfter, $field) public function insertAfter($insertAfter, $field, $appendIfMissing = true)
{ {
return $this->children->insertAfter($insertAfter, $field); return $this->children->insertAfter($insertAfter, $field, $appendIfMissing);
} }
/** /**

28
src/Forms/FieldList.php
View File

@ -563,12 +563,14 @@ class FieldList extends ArrayList
/** /**
* Inserts a field before a particular field in a FieldList. * Inserts a field before a particular field in a FieldList.
* Will traverse CompositeFields depth-first to find the maching $name, and insert before the first match
* *
* @param string $name Name of the field to insert before * @param string $name Name of the field to insert before
* @param FormField $item The form field to insert * @param FormField $item The form field to insert
* @return FormField|false * @param bool $appendIfMissing Append to the end of the list if $name isn't found
* @return FormField|false Field if it was successfully inserted, false if not inserted
*/ */
public function insertBefore($name, $item) public function insertBefore($name, $item, $appendIfMissing = true)
{ {
// Backwards compatibility for order of arguments // Backwards compatibility for order of arguments
if ($name instanceof FormField) { if ($name instanceof FormField) {
@ -584,7 +586,7 @@ class FieldList extends ArrayList
array_splice($this->items, $i, 0, array($item)); array_splice($this->items, $i, 0, array($item));
return $item; return $item;
} elseif ($child instanceof CompositeField) { } elseif ($child instanceof CompositeField) {
$ret = $child->insertBefore($name, $item); $ret = $child->insertBefore($name, $item, false);
if ($ret) { if ($ret) {
return $ret; return $ret;
} }
@ -592,17 +594,25 @@ class FieldList extends ArrayList
$i++; $i++;
} }
// $name not found, append if needed
if ($appendIfMissing) {
$this->push($item);
return $item;
}
return false; return false;
} }
/** /**
* Inserts a field after a particular field in a FieldList. * Inserts a field after a particular field in a FieldList.
* Will traverse CompositeFields depth-first to find the maching $name, and insert after the first match
* *
* @param string $name Name of the field to insert after * @param string $name Name of the field to insert after
* @param FormField $item The form field to insert * @param FormField $item The form field to insert
* @return FormField|false * @param bool $appendIfMissing Append to the end of the list if $name isn't found
* @return FormField|false Field if it was successfully inserted, false if not inserted
*/ */
public function insertAfter($name, $item) public function insertAfter($name, $item, $appendIfMissing = true)
{ {
// Backwards compatibility for order of arguments // Backwards compatibility for order of arguments
if ($name instanceof FormField) { if ($name instanceof FormField) {
@ -618,7 +628,7 @@ class FieldList extends ArrayList
array_splice($this->items, $i+1, 0, array($item)); array_splice($this->items, $i+1, 0, array($item));
return $item; return $item;
} elseif ($child instanceof CompositeField) { } elseif ($child instanceof CompositeField) {
$ret = $child->insertAfter($name, $item); $ret = $child->insertAfter($name, $item, false);
if ($ret) { if ($ret) {
return $ret; return $ret;
} }
@ -626,6 +636,12 @@ class FieldList extends ArrayList
$i++; $i++;
} }
// $name not found, append if needed
if ($appendIfMissing) {
$this->push($item);
return $item;
}
return false; return false;
} }

8
src/Forms/TabSet.php
View File

@ -221,12 +221,12 @@ class TabSet extends CompositeField
* @param FormField $field The form field to insert * @param FormField $field The form field to insert
* @return FormField|null * @return FormField|null
*/ */
public function insertBefore($insertBefore, $field) public function insertBefore($insertBefore, $field, $appendIfMissing = true)
{ {
if ($field instanceof Tab || $field instanceof TabSet) { if ($field instanceof Tab || $field instanceof TabSet) {
$field->setTabSet($this); $field->setTabSet($this);
} }
return parent::insertBefore($insertBefore, $field); return parent::insertBefore($insertBefore, $field, $appendIfMissing);
} }
/** /**
@ -236,12 +236,12 @@ class TabSet extends CompositeField
* @param FormField $field The form field to insert * @param FormField $field The form field to insert
* @return FormField|null * @return FormField|null
*/ */
public function insertAfter($insertAfter, $field) public function insertAfter($insertAfter, $field, $appendIfMissing = true)
{ {
if ($field instanceof Tab || $field instanceof TabSet) { if ($field instanceof Tab || $field instanceof TabSet) {
$field->setTabSet($this); $field->setTabSet($this);
} }
return parent::insertAfter($insertAfter, $field); return parent::insertAfter($insertAfter, $field, $appendIfMissing);
} }
/** /**

12
tests/php/Forms/FieldListTest.php
View File

@ -615,6 +615,12 @@ class FieldListTest extends SapphireTest
/* The position of the Surname field is at number 4 */ /* The position of the Surname field is at number 4 */
$this->assertEquals('Surname', $fields[3]->getName()); $this->assertEquals('Surname', $fields[3]->getName());
/* Test that inserting before a field that doesn't exist simply appends
* Confirm that a composite field doesn't break this */
$fields->push(new CompositeField([ new TextField('Nested1'), new TextField('Nested2')]));
$this->assertTrue((bool)$fields->insertBefore('DoesNotExist', new TextField('MyName')));
$this->assertEquals('MyName', $fields->Last()->Name);
} }
public function testInsertBeforeMultipleFields() public function testInsertBeforeMultipleFields()
@ -688,6 +694,12 @@ class FieldListTest extends SapphireTest
/* The position of the Surname field is at number 5 */ /* The position of the Surname field is at number 5 */
$this->assertEquals('Surname', $fields[4]->getName()); $this->assertEquals('Surname', $fields[4]->getName());
/* Test that inserting before a field that doesn't exist simply appends
* Confirm that a composite field doesn't break this */
$fields->push(new CompositeField([ new TextField('Nested1'), new TextField('Nested2')]));
$this->assertTrue((bool)$fields->insertAfter('DoesNotExist', new TextField('MyName')));
$this->assertEquals('MyName', $fields->Last()->Name);
} }
public function testrootFieldList() public function testrootFieldList()