From 559abecd56eec1c9a153a515c0645f1c84088e39 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Thu, 13 Dec 2012 13:51:28 +0100 Subject: [PATCH] API Copying instance props on FormField readonly/disabled transformations Introduced new FormField->castedCopy() method which tries to replicate the existing form field instance as closely as possible. Primarily, the fix was targeted at consistently passing through FormField->description to all of its variations. --- forms/CheckboxField.php | 6 ---- forms/CheckboxSetField.php | 6 ++-- forms/CompositeField.php | 7 ++++ forms/ConfirmedPasswordField.php | 6 ++-- forms/CurrencyField.php | 4 +-- forms/DateField.php | 16 +++++++-- forms/DatetimeField.php | 18 ++++++++-- forms/DropdownField.php | 6 ++-- forms/FormField.php | 59 +++++++++++++++++++++++++------ forms/HtmlEditorField.php | 4 +-- forms/InlineFormAction.php | 2 +- forms/LookupField.php | 2 +- forms/OptionsetField.php | 5 ++- forms/PasswordField.php | 8 ++--- forms/TextareaField.php | 14 -------- forms/TimeField.php | 14 +++++++- forms/TreeDropdownField.php | 51 ++++++++++++++++++++++++-- forms/TreeMultiselectField.php | 12 +++---- tests/forms/CheckboxFieldTest.php | 1 + 19 files changed, 172 insertions(+), 69 deletions(-) diff --git a/forms/CheckboxField.php b/forms/CheckboxField.php index c90341037..6adf1a325 100644 --- a/forms/CheckboxField.php +++ b/forms/CheckboxField.php @@ -40,12 +40,6 @@ class CheckboxField extends FormField { $field->setForm($this->form); return $field; } - - public function performDisabledTransformation() { - $clone = clone $this; - $clone->setDisabled(true); - return $clone; - } } diff --git a/forms/CheckboxSetField.php b/forms/CheckboxSetField.php index 9d174706c..e7af85412 100644 --- a/forms/CheckboxSetField.php +++ b/forms/CheckboxSetField.php @@ -276,10 +276,8 @@ class CheckboxSetField extends OptionsetField { } } - $title = ($this->title) ? $this->title : ''; - - $field = new ReadonlyField($this->name, $title, $values); - $field->setForm($this->form); + $field = $this->castedCopy('ReadonlyField'); + $field->setValue($values); return $field; } diff --git a/forms/CompositeField.php b/forms/CompositeField.php index a41c4555c..23e66a20d 100644 --- a/forms/CompositeField.php +++ b/forms/CompositeField.php @@ -265,6 +265,8 @@ class CompositeField extends FormField { $clone->children = $newChildren; $clone->readonly = true; + $clone->addExtraClass($this->extraClass()); + $clone->setDescription($this->getDescription()); return $clone; } @@ -285,6 +287,11 @@ class CompositeField extends FormField { $clone->children = $newChildren; $clone->readonly = true; + $clone->addExtraClass($this->extraClass()); + $clone->setDescription($this->getDescription()); + foreach($this->attributes as $k => $v) { + $clone->setAttribute($k, $v); + } return $clone; } diff --git a/forms/ConfirmedPasswordField.php b/forms/ConfirmedPasswordField.php index 844e98367..e92cb468a 100644 --- a/forms/ConfirmedPasswordField.php +++ b/forms/ConfirmedPasswordField.php @@ -320,10 +320,10 @@ class ConfirmedPasswordField extends FormField { * Makes a pretty readonly field with some stars in it */ public function performReadonlyTransformation() { - $stars = '*****'; + $field = $this->castedCopy('ReadonlyField') + ->setTitle($this->title ? $this->title : _t('Member.PASSWORD')) + ->setValue('*****'); - $field = new ReadonlyField($this->name, $this->title ? $this->title : _t('Member.PASSWORD'), $stars); - $field->setForm($this->form); return $field; } } diff --git a/forms/CurrencyField.php b/forms/CurrencyField.php index 440e926f9..9d934a3ba 100644 --- a/forms/CurrencyField.php +++ b/forms/CurrencyField.php @@ -40,9 +40,7 @@ class CurrencyField extends TextField { * Create a new class for this field */ public function performReadonlyTransformation() { - $field = new CurrencyField_Readonly($this->name, $this->title, $this->value); - $field -> addExtraClass($this->extraClass()); - return $field; + return $this->castedCopy('CurrencyField_Readonly'); } public function validate($validator) { diff --git a/forms/DateField.php b/forms/DateField.php index 0a8c4e191..a6a1dba99 100644 --- a/forms/DateField.php +++ b/forms/DateField.php @@ -279,13 +279,25 @@ class DateField extends TextField { } public function performReadonlyTransformation() { - $field = new DateField_Disabled($this->name, $this->title, $this->dataValue()); - $field->setForm($this->form); + $field = $this->castedCopy('DateField_Disabled'); + $field->setValue($this->dataValue()); $field->readonly = true; return $field; } + public function castedCopy($class) { + $copy = new $class($this->name); + if($copy->hasMethod('setConfig')) { + $config = $this->getConfig(); + foreach($config as $k => $v) { + $copy->setConfig($k, $v); + } + } + + return parent::castedCopy($copy); + } + /** * Validate an array with expected keys 'day', 'month' and 'year. * Used because Zend_Date::isDate() doesn't provide this. diff --git a/forms/DatetimeField.php b/forms/DatetimeField.php index 10a5775c4..13ce16a53 100644 --- a/forms/DatetimeField.php +++ b/forms/DatetimeField.php @@ -283,8 +283,22 @@ class DatetimeField extends FormField { } public function performReadonlyTransformation() { - $field = new DatetimeField_Readonly($this->name, $this->title, $this->dataValue()); - $field->setForm($this->form); + $field = $this->castedCopy('DatetimeField_Readonly'); + $field->setValue($this->dataValue()); + + $dateFieldConfig = $this->getDateField()->getConfig(); + if($dateFieldConfig) { + foreach($dateFieldConfig as $k => $v) { + $field->getDateField()->setConfig($k, $v); + } + } + + $timeFieldConfig = $this->getTimeField()->getConfig(); + if($timeFieldConfig) { + foreach($timeFieldConfig as $k => $v) { + $field->getTimeField()->setConfig($k, $v); + } + } return $field; } diff --git a/forms/DropdownField.php b/forms/DropdownField.php index 449747d93..e1d018936 100644 --- a/forms/DropdownField.php +++ b/forms/DropdownField.php @@ -239,10 +239,10 @@ class DropdownField extends FormField { } public function performReadonlyTransformation() { - $field = new LookupField($this->name, $this->title, $this->getSource()); - $field->setValue($this->value); - $field->setForm($this->form); + $field = $this->castedCopy('LookupField'); + $field->setSource($this->getSource()); $field->setReadonly(true); + return $field; } } diff --git a/forms/FormField.php b/forms/FormField.php index 9fc412470..b5f73a535 100644 --- a/forms/FormField.php +++ b/forms/FormField.php @@ -357,7 +357,7 @@ class FormField extends RequestHandler { 'id' => $this->ID(), 'disabled' => $this->isDisabled(), ); - + return array_merge($attrs, $this->attributes); } @@ -709,10 +709,9 @@ class FormField extends RequestHandler { * Returns a readonly version of this field */ public function performReadonlyTransformation() { - $field = new ReadonlyField($this->name, $this->title, $this->value); - $field->addExtraClass($this->extraClass()); - $field->setForm($this->form); - return $field; + $copy = $this->castedCopy('ReadonlyField'); + $copy->setReadonly(true); + return $copy; } /** @@ -723,14 +722,15 @@ class FormField extends RequestHandler { * @return FormField */ public function performDisabledTransformation() { - $clone = clone $this; - $disabledClassName = $clone->class . '_Disabled'; + $disabledClassName = $this->class . '_Disabled'; if(ClassInfo::exists($disabledClassName)) { - return new $disabledClassName($this->name, $this->title, $this->value); + $clone = $this->castedCopy($disabledClassName); } else { + $clone = clone $this; $clone->setDisabled(true); - return $clone; } + + return $clone; } public function transform(FormTransformation $trans) { @@ -774,7 +774,8 @@ class FormField extends RequestHandler { /** * Describe this field, provide help text for it. - * By default, renders as a "title" attribute on the form field. + * By default, renders as a + * underneath the form field. * * @return string Description */ @@ -828,5 +829,43 @@ class FormField extends RequestHandler { if(is_object($this->containerFieldList)) return $this->containerFieldList->rootFieldList(); else user_error("rootFieldList() called on $this->class object without a containerFieldList", E_USER_ERROR); } + + /** + * Returns another instance of this field, but "cast" to a different class. + * The logic tries to retain all of the instance properties, + * and may be overloaded by subclasses to set additional ones. + * + * Assumes the standard FormField parameter signature with + * its name as the only mandatory argument. Mainly geared towards + * creating *_Readonly or *_Disabled subclasses of the same type, + * or casting to a {@link ReadonlyField}. + * + * Does not copy custom field templates, since they probably won't apply to + * the new instance. + * + * @param String $classOrCopy Class name for copy, or existing copy instance to update + * @return FormField + */ + public function castedCopy($classOrCopy) { + $field = (is_object($classOrCopy)) ? $classOrCopy : new $classOrCopy($this->name); + $field + ->setValue($this->value) // get value directly from property, avoid any conversions + ->setForm($this->form) + ->setTitle($this->Title()) + ->setLeftTitle($this->LeftTitle()) + ->setRightTitle($this->RightTitle()) + ->addExtraClass($this->extraClass()) + ->setDescription($this->getDescription()); + + // Only include built-in attributes, ignore anything + // set through getAttributes(), since those might change important characteristics + // of the field, e.g. its "type" attribute. + foreach($this->attributes as $k => $v) { + $field->setAttribute($k, $v); + } + $field->dontEscape = $this->dontEscape; + + return $field; + } } diff --git a/forms/HtmlEditorField.php b/forms/HtmlEditorField.php index e4c0a7fa6..c4df22b65 100644 --- a/forms/HtmlEditorField.php +++ b/forms/HtmlEditorField.php @@ -204,9 +204,9 @@ class HtmlEditorField extends TextareaField { * @return HtmlEditorField_Readonly */ public function performReadonlyTransformation() { - $field = new HtmlEditorField_Readonly($this->name, $this->title, $this->value); - $field->setForm($this->form); + $field = $this->castedCopy('HtmlEditorField_Readonly'); $field->dontEscape = true; + return $field; } diff --git a/forms/InlineFormAction.php b/forms/InlineFormAction.php index 25a0b6811..58d7de909 100644 --- a/forms/InlineFormAction.php +++ b/forms/InlineFormAction.php @@ -24,7 +24,7 @@ class InlineFormAction extends FormField { } public function performReadonlyTransformation() { - return new InlineFormAction_ReadOnly( $this->name, $this->title ); + return $this->castedCopy('InlineFormAction_ReadOnly'); } public function Field($properties = array()) { diff --git a/forms/LookupField.php b/forms/LookupField.php index 1a9ef2be4..82a5d52ea 100644 --- a/forms/LookupField.php +++ b/forms/LookupField.php @@ -32,7 +32,7 @@ class LookupField extends DropdownField { // Don't check if string arguments are matching against the source, // as they might be generated HTML diff views instead of the actual values - if($this->value && !$mapped) { + if($this->value && !is_array($this->value) && !$mapped) { $mapped = array(trim($this->value)); $values = array(); } diff --git a/forms/OptionsetField.php b/forms/OptionsetField.php index f223aaf23..5e2e36e12 100644 --- a/forms/OptionsetField.php +++ b/forms/OptionsetField.php @@ -92,9 +92,8 @@ class OptionsetField extends DropdownField { public function performReadonlyTransformation() { // Source and values are DataObject sets. - $items = $this->getSource(); - $field = new LookupField($this->name, $this->title ? $this->title : '', $items, $this->value); - $field->setForm($this->form); + $field = $this->castedCopy('LookupField'); + $field->setValue($this->getSource()); $field->setReadonly(true); return $field; diff --git a/forms/PasswordField.php b/forms/PasswordField.php index 10cc45715..156deb8be 100644 --- a/forms/PasswordField.php +++ b/forms/PasswordField.php @@ -31,11 +31,9 @@ class PasswordField extends TextField { * Makes a pretty readonly field with some stars in it */ public function performReadonlyTransformation() { - $stars = '*****'; - - $field = new ReadonlyField($this->name, $this->title ? $this->title : '', $stars); - $field->setForm($this->form); - $field->setReadonly(true); + $field = $this->castedCopy('ReadonlyField'); + $field->setValue('*****'); + return $field; } diff --git a/forms/TextareaField.php b/forms/TextareaField.php index 92863c6ca..3dfe30971 100644 --- a/forms/TextareaField.php +++ b/forms/TextareaField.php @@ -44,20 +44,6 @@ class TextareaField extends FormField { ); } - /** - * Performs a disabled transformation on this field. You shouldn't be able to - * copy from this field, and it should not send any data when you submit the - * form it's attached to. - * - * The element shouldn't be both disabled and readonly at the same time. - */ - public function performDisabledTransformation() { - $clone = clone $this; - $clone->setDisabled(true); - $clone->setReadonly(false); - return $clone; - } - public function Type() { return parent::Type() . ($this->readonly ? ' readonly' : ''); } diff --git a/forms/TimeField.php b/forms/TimeField.php index b1e5e427d..33c0fa079 100644 --- a/forms/TimeField.php +++ b/forms/TimeField.php @@ -191,7 +191,19 @@ class TimeField extends TextField { * Creates a new readonly field specified below */ public function performReadonlyTransformation() { - return new TimeField_Readonly($this->name, $this->title, $this->dataValue(), $this->getConfig('timeformat')); + return $this->castedCopy('TimeField_Readonly'); + } + + public function castedCopy($class) { + $copy = parent::castedCopy($class); + if($copy->hasMethod('setConfig')) { + $config = $this->getConfig(); + foreach($config as $k => $v) { + $copy->setConfig($k, $v); + } + } + + return $copy; } } diff --git a/forms/TreeDropdownField.php b/forms/TreeDropdownField.php index 3d7874e8f..01112b9cc 100644 --- a/forms/TreeDropdownField.php +++ b/forms/TreeDropdownField.php @@ -291,6 +291,48 @@ class TreeDropdownField extends FormField { return true; } + + /** + * @param String $field + */ + public function setLabelField($field) { + $this->labelField = $field; + } + + /** + * @return String + */ + public function getLabelField() { + return $this->labelField; + } + + /** + * @param String $field + */ + public function setKeyField($field) { + $this->keyField = $field; + } + + /** + * @return String + */ + public function getKeyField() { + return $this->keyField; + } + + /** + * @param String $field + */ + public function setSourceObject($class) { + $this->sourceObject = $class; + } + + /** + * @return String + */ + public function getSourceObject() { + return $this->sourceObject; + } /** * Populate $this->searchIds with the IDs of the pages matching the searched parameter and their parents. @@ -342,9 +384,14 @@ class TreeDropdownField extends FormField { * Changes this field to the readonly field. */ public function performReadonlyTransformation() { - return new TreeDropdownField_Readonly($this->name, $this->title, $this->sourceObject, $this->keyField, - $this->labelField); + $copy = $this->castedCopy('TreeDropdownField_Readonly'); + $copy->setKeyField($this->keyField); + $copy->setLabelField($this->labelField); + $copy->setSourceObject($this->sourceObject); + + return $copy; } + } /** diff --git a/forms/TreeMultiselectField.php b/forms/TreeMultiselectField.php index f651e0ad2..e8c53a299 100644 --- a/forms/TreeMultiselectField.php +++ b/forms/TreeMultiselectField.php @@ -175,14 +175,12 @@ class TreeMultiselectField extends TreeDropdownField { * Changes this field to the readonly field. */ public function performReadonlyTransformation() { - $field = new TreeMultiselectField_Readonly($this->name, $this->title, $this->sourceObject, - $this->keyField, $this->labelField); + $copy = $this->castedCopy('TreeMultiselectField_Readonly'); + $copy->setKeyField($this->keyField); + $copy->setLabelField($this->labelField); + $copy->setSourceObject($this->sourceObject); - $field->addExtraClass($this->extraClass()); - $field->setForm($this->form); - $field->setValue($this->value); - - return $field; + return $copy; } } diff --git a/tests/forms/CheckboxFieldTest.php b/tests/forms/CheckboxFieldTest.php index 28fac29a4..13e007683 100644 --- a/tests/forms/CheckboxFieldTest.php +++ b/tests/forms/CheckboxFieldTest.php @@ -122,6 +122,7 @@ class CheckboxFieldTest extends SapphireTest { // Test 1: a checked checkbox goes to "Yes" $field1 = new CheckboxField('IsChecked', 'Checked'); $field1->setValue('on'); + $copy = $field1->performReadonlyTransformation(); $this->assertEquals(_t('CheckboxField.YES', 'Yes'), trim(strip_tags($field1->performReadonlyTransformation()->Field())));