diff --git a/docs/en/changelogs/3.0.0.md b/docs/en/changelogs/3.0.0.md index 19a8c8d0e..bd911ce56 100644 --- a/docs/en/changelogs/3.0.0.md +++ b/docs/en/changelogs/3.0.0.md @@ -63,6 +63,27 @@ not when simply using the CMS or developing other CMS functionality. If you want to extend the CMS stylesheets for your own projects without SCSS, please create a new CSS file and link it into the CMS via `[api:LeftAndMain::require_css()]`. +### FormField consistently adds classes to HTML elements ### + +The [api:FormField] API has been refactored to use SilverStripe templates +for constructing the field HTML, as well as new accessors for HTML attributes. +This change makes the HTML a bit more predictable, but it also means that +you need to check any code (CSS, JavaScript, etc) relying on the old inconsistencies. +Particularly, CSS class names applied through [api:FormField->addExtraClass()] +and the "type" class are now consistently added to the container `
` +as well as the HTML form element itself. + + :::html + Before (abbreviated): +
+ +
+ + After (abbreviated): +
+ +
+ ### Restructured files and folders ### In order to make the `sapphire` framework useable without the `cms` module, diff --git a/forms/CheckboxField.php b/forms/CheckboxField.php index b8e12e036..cde44a9a6 100644 --- a/forms/CheckboxField.php +++ b/forms/CheckboxField.php @@ -22,8 +22,6 @@ class CheckboxField extends FormField { return ($this->value) ? 1 : 0; } - } - /** * Returns a restricted field holder used within things like FieldGroups */ @@ -35,6 +33,18 @@ class CheckboxField extends FormField { return $result; } + function getAttributes() { + $attrs = parent::getAttributes(); + $attrs['value'] = 1; + return array_merge( + $attrs, + array( + 'checked' => ($this->Value()) ? 'checked' : null, + 'type' => 'checkbox', + ) + ); + } + /** * Returns a readonly version of this field */ diff --git a/forms/CheckboxSetField.php b/forms/CheckboxSetField.php index 378534377..686b49931 100644 --- a/forms/CheckboxSetField.php +++ b/forms/CheckboxSetField.php @@ -98,12 +98,9 @@ class CheckboxSetField extends OptionsetField { } $odd = 0; - $options = ''; + $options = array(); - if ($source == null) { - $source = array(); - $options = "
  • No options available
    • "; - } + if ($source == null) $source = array(); if($source) { foreach($source as $value => $item) { @@ -122,7 +119,7 @@ class CheckboxSetField extends OptionsetField { $options[] = new ArrayData(array( 'ID' => $itemID, 'Class' => $extraClass, - 'Name' => $this->name, + 'Name' => "{$this->name}[{$value}]", 'Value' => $value, 'Title' => $title, 'isChecked' => in_array($value, $items) || in_array($value, $this->defaultItems), @@ -283,6 +280,10 @@ class CheckboxSetField extends OptionsetField { return $field; } + + function Type() { + return 'optionset checkboxset'; + } function ExtraOptions() { return FormField::ExtraOptions(); diff --git a/forms/CompositeField.php b/forms/CompositeField.php index d862ea986..38209e304 100644 --- a/forms/CompositeField.php +++ b/forms/CompositeField.php @@ -83,15 +83,25 @@ class CompositeField extends FormField { $this->children = $children; } + function extraClasses() { + $classes = array('field', 'CompositeField', parent::extraClasses()); + if($this->columnCount) $classes[] = 'multicolumn'; + return implode(' ', $classes); + } + + function getAttributes() { + return array_merge( + parent::getAttributes(), + array('tabindex' => null, 'type' => null, 'value' => null, 'type' => null) + ); + } + /** * Returns the fields nested inside another DIV */ function FieldHolder() { + $content = ''; $fs = $this->FieldList(); - $idAtt = isset($this->id) ? " id=\"{$this->id}\"" : ''; - $className = ($this->columnCount) ? "field CompositeField {$this->extraClass()} multicolumn" : "field CompositeField {$this->extraClass()}"; - $content = "
    \n"; - foreach($fs as $subfield) { if($this->columnCount) { $className = "column{$this->columnCount}"; @@ -101,9 +111,8 @@ class CompositeField extends FormField { $content .= "\n" . $subfield->FieldHolder() . "\n"; } } - $content .= "
    \n"; - return $content; + $this->createTag('div', $this->getAttributes(), $content); } /** diff --git a/forms/CountryDropdownField.php b/forms/CountryDropdownField.php index 6322c3631..e7309972c 100644 --- a/forms/CountryDropdownField.php +++ b/forms/CountryDropdownField.php @@ -22,14 +22,14 @@ class CountryDropdownField extends DropdownField { function defaultToVisitorCountry($val) { $this->defaultToVisitorCountry = $val; } - - function Field() { + + function Value() { $source = $this->getSource(); - if($this->defaultToVisitorCountry && !$this->value || !isset($source[$this->value])) { - $this->value = ($vc = Geoip::visitor_country()) ? $vc : Geoip::get_default_country_code(); + return ($vc = Geoip::visitor_country()) ? $vc : Geoip::get_default_country_code(); + } else { + return $this->value; } - - return parent::Field(); } + } \ No newline at end of file diff --git a/forms/CreditCardField.php b/forms/CreditCardField.php index b6c7156f9..c6c059d55 100644 --- a/forms/CreditCardField.php +++ b/forms/CreditCardField.php @@ -9,6 +9,8 @@ class CreditCardField extends TextField { function Field() { $parts = explode("\n", chunk_split($this->value,4,"\n")); $parts = array_pad($parts, 4, ""); + + // TODO Mark as disabled/readonly $field = "name}_Holder\" class=\"creditCardField\">" . "name}[0]\" value=\"$parts[0]\" maxlength=\"4\"" . $this->getTabIndexHTML(0) . " /> - " . "name}[1]\" value=\"$parts[1]\" maxlength=\"4\"" . $this->getTabIndexHTML(1) . " /> - " . diff --git a/forms/CurrencyField.php b/forms/CurrencyField.php index 1edfe03ce..e8df11610 100644 --- a/forms/CurrencyField.php +++ b/forms/CurrencyField.php @@ -27,6 +27,11 @@ class CurrencyField extends TextField { return 0.00; } } + + function Type() { + return 'currency text'; + } + /** * Create a new class for this field */ diff --git a/forms/DatalessField.php b/forms/DatalessField.php index bef57bc94..432e4e3dc 100644 --- a/forms/DatalessField.php +++ b/forms/DatalessField.php @@ -18,6 +18,15 @@ class DatalessField extends FormField { * Always returns false. */ function hasData() { return false; } + + function getAttributes() { + return array_merge( + parent::getAttributes(), + array( + 'type' => 'hidden', + ) + ); + } /** * Returns the field's representation in the form. @@ -58,4 +67,8 @@ class DatalessField extends FormField { return $this->allowHTML; } + function Type() { + return 'readonly'; + } + } \ No newline at end of file diff --git a/forms/DateField.php b/forms/DateField.php index d5003f1fe..b67070368 100644 --- a/forms/DateField.php +++ b/forms/DateField.php @@ -174,6 +174,10 @@ class DateField extends TextField { return $html; } + + function Type() { + return 'date text'; + } /** * Sets the internal value to ISO date format. diff --git a/forms/DropdownField.php b/forms/DropdownField.php index fe06042d1..fa84e6d66 100644 --- a/forms/DropdownField.php +++ b/forms/DropdownField.php @@ -162,6 +162,13 @@ class DropdownField extends FormField { return $this->customise($properties)->renderWith($this->getTemplate()); } + function getAttributes() { + return array_merge( + parent::getAttributes(), + array('type' => null) + ); + } + /** * @return boolean */ @@ -230,12 +237,6 @@ class DropdownField extends FormField { return $field; } - function extraClass() { - $ret = parent::extraClass(); - if($this->extraClass) $ret .= " $this->extraClass"; - return $ret; - } - /** * Set form being disabled */ diff --git a/forms/EmailField.php b/forms/EmailField.php index 593511693..3a9a65a2e 100644 --- a/forms/EmailField.php +++ b/forms/EmailField.php @@ -6,6 +6,10 @@ */ class EmailField extends TextField { + function Type() { + return 'email text'; + } + function jsValidation() { $formID = $this->form->FormName(); $error = _t('EmailField.VALIDATIONJS', 'Please enter an email address.'); @@ -40,14 +44,6 @@ if(typeof fromAnOnBlur != 'undefined'){ JS; } - /** - * Returns the field type - used by templates. - * @return string - */ - function Type() { - return 'text'; - } - /** * Validates for RFC 2822 compliant email adresses. * diff --git a/forms/FileField.php b/forms/FileField.php index 27c61d2d1..d63e7217f 100644 --- a/forms/FileField.php +++ b/forms/FileField.php @@ -117,6 +117,13 @@ class FileField extends FormField { return $this->customise($properties)->renderWith($this->getTemplate()); } + function getAttributes() { + return array_merge( + parent::getAttributes(), + array('type' => 'file') + ); + } + public function saveInto(DataObject $record) { if(!isset($_FILES[$this->name])) return false; $fileClass = File::get_class_for_file_extension(pathinfo($_FILES[$this->name]['name'], PATHINFO_EXTENSION)); diff --git a/forms/FormAction.php b/forms/FormAction.php index 5fed7a351..4bf17538f 100644 --- a/forms/FormAction.php +++ b/forms/FormAction.php @@ -91,7 +91,22 @@ class FormAction extends FormField { } public function Type() { - return ($this->useButtonTag) ? 'button' : 'submit'; + return 'action'; + } + + function getAttributes() { + return array_merge( + parent::getAttributes(), + array( + 'disabled' => ($this->isReadonly() || $this->isDisabled()), + 'value' => $this->Title(), + 'type' => ($this->useButtonTag) ? null : 'submit' + ) + ); + } + + function extraClass() { + return 'action ' . parent::extraClass(); } /** diff --git a/forms/FormField.php b/forms/FormField.php index 611e51195..0a72df373 100644 --- a/forms/FormField.php +++ b/forms/FormField.php @@ -89,6 +89,12 @@ class FormField extends RequestHandler { */ protected $fieldHolderTemplate = 'FieldHolder'; + /** + * @var array All attributes on the form field (not the field holder). + * Partially determined based on other instance properties, please use {@link getAttributes()}. + */ + protected $attributes = array(); + /** * Create a new field. * @param name The internal field name, passed to forms. @@ -253,21 +259,22 @@ class FormField extends RequestHandler { * @return String CSS-classnames */ function extraClass() { - $output = ""; - if(is_array($this->extraClasses)) { - $output = " " . implode($this->extraClasses, " "); - } + $classes = array(); + + $classes[] = $this->Type(); + + if($this->extraClasses) $classes = array_merge($classes, array_values($this->extraClasses)); // Allow customization of label and field tag positioning - if(!$this->Title()) $output .= " nolabel"; + if(!$this->Title()) $classes[] = "nolabel"; // Allow custom styling of any element in the container based // on validation errors, e.g. red borders on input tags. // CSS-Class needs to be different from the one rendered // through {@link FieldHolder()} - if($this->Message()) $output .= " holder-" . $this->MessageType(); + if($this->Message()) $classes[] .= "holder-" . $this->MessageType(); - return $output; + return implode(' ', $classes); } /** @@ -288,6 +295,73 @@ class FormField extends RequestHandler { if(isset($this->extraClasses) && array_key_exists($class, $this->extraClasses)) unset($this->extraClasses[$class]); } + /** + * Set an HTML attribute on the field element, mostly an tag. + * + * CAUTION Doesn't work on most fields which are composed of more than one HTML form field: + * AjaxUniqueTextField, CheckboxSetField, ComplexTableField, CompositeField, ConfirmedPasswordField, CountryDropdownField, + * CreditCardField, CurrencyField, DateField, DatetimeField, FieldGroup, GridField, HtmlEditorField, + * ImageField, ImageFormAction, InlineFormAction, ListBoxField, etc. + * + * @param String + * @param String + */ + function setAttribute($name, $value) { + $this->attributes[$name] = $value; + } + + /** + * Get an HTML attribute defined by the field, or added through {@link setAttribute()}. + * Caution: Doesn't work on all fields, see {@link setAttribute()}. + * + * @return String + */ + function getAttribute($name) { + $attrs = $this->getAttributes(); + return @$attrs[$name]; + } + + /** + * @return array + */ + function getAttributes() { + $attrs = array( + 'type' => 'text', + 'name' => $this->getName(), + 'value' => $this->Value(), + 'class' => $this->extraClass(), + 'id' => $this->ID(), + 'tabindex' => $this->getTabIndex(), + 'disabled' => $this->isDisabled(), + ); + return array_merge($attrs, $this->attributes); + } + + /** + * @param Array Custom attributes to process. Falls back to {@link getAttributes()}. + * If at least one argument is passed as a string, all arguments act as excludes by name. + * @return String HTML attributes, ready for insertion into an HTML tag + */ + function getAttributesHTML($attrs = null) { + $exclude = (is_string($attrs)) ? func_get_args() : null; + + if(!$attrs || is_string($attrs)) $attrs = $this->getAttributes(); + + // Remove empty + $attrs = array_filter((array)$attrs, create_function('$v', 'return ($v || $v === 0);')); ; + + // Remove excluded + if($exclude) $attrs = array_diff_key($attrs, array_flip($exclude)); + + // Create markkup + $parts = array(); + foreach($attrs as $name => $value) { + $parts[] = ($value === true) ? "{$name}=\"{$name}\"" : "{$name}=\"" . Convert::raw2att($value) . "\""; + } + + return implode(' ', $parts); + } + /** * Returns a version of a title suitable for insertion into an HTML attribute */ @@ -555,15 +629,13 @@ class FormField extends RequestHandler { /** * Returns the field type - used by templates. * The field type is the class name with the word Field dropped off the end, all lowercase. - * It's handy for assigning HTML classes. + * It's handy for assigning HTML classes. Doesn't signify the attribute, + * see {link getAttributes()}. + * * @return string */ function Type() { - if(get_class($this) == 'FormField') { - return 'hidden'; - } else { - return strtolower(ereg_replace('Field$', '', $this->class)); - } + return strtolower(ereg_replace('Field$', '', $this->class)); } /** diff --git a/forms/GroupedDropdownField.php b/forms/GroupedDropdownField.php index 39f012da3..72703de22 100644 --- a/forms/GroupedDropdownField.php +++ b/forms/GroupedDropdownField.php @@ -40,14 +40,7 @@ class GroupedDropdownField extends DropdownField { function Field() { - // Initialisations $options = ''; - $classAttr = ''; - - if($extraClass = trim($this->extraClass())) { - $classAttr = "class=\"$extraClass\""; - } - foreach($this->getSource() as $value => $title) { if(is_array($title)) { $options .= ""; @@ -62,9 +55,7 @@ class GroupedDropdownField extends DropdownField { } } - $id = $this->id(); - - return ""; + return $this->createTag('select', $this->getAttributes(), $options); } } diff --git a/forms/HeaderField.php b/forms/HeaderField.php index caa44b5e4..a8c90fff5 100644 --- a/forms/HeaderField.php +++ b/forms/HeaderField.php @@ -36,4 +36,18 @@ class HeaderField extends DatalessField { return $this->headingLevel; } + function getAttributes() { + return array_merge( + array( + 'id' => $this->ID(), + 'class' => $this->extraClass() + ), + $this->attributes + ); + } + + function Type() { + return null; + } + } \ No newline at end of file diff --git a/forms/HiddenField.php b/forms/HiddenField.php index f1d799157..1799992a3 100644 --- a/forms/HiddenField.php +++ b/forms/HiddenField.php @@ -22,6 +22,13 @@ class HiddenField extends FormField { return true; } + function getAttributes() { + return array_merge( + parent::getAttributes(), + array('type' => 'hidden') + ); + } + static function create($name) { return new HiddenField($name); } diff --git a/forms/HtmlEditorField.php b/forms/HtmlEditorField.php index 189aa3e70..0bee4d8e4 100644 --- a/forms/HtmlEditorField.php +++ b/forms/HtmlEditorField.php @@ -22,8 +22,6 @@ class HtmlEditorField extends TextareaField { public function __construct($name, $title = null, $rows = 30, $cols = 20, $value = '', $form = null) { parent::__construct($name, $title, $rows, $cols, $value, $form); - $this->addExtraClass('htmleditor'); - self::include_js(); } @@ -47,18 +45,21 @@ class HtmlEditorField extends TextareaField { return $this->createTag ( 'textarea', - array ( - 'class' => $this->extraClass(), - 'rows' => $this->rows, - 'cols' => $this->cols, - 'style' => 'width: 97%; height: ' . ($this->rows * 16) . 'px', // prevents horizontal scrollbars - 'tinymce' => 'true', - 'id' => $this->id(), - 'name' => $this->name - ), + $this->getAttributes(), htmlentities($value->getContent(), ENT_COMPAT, 'UTF-8') ); } + + function getAttributes() { + return array_merge( + parent::getAttributes(), + array( + 'tinymce' => 'true', + 'style' => 'width: 97%; height: ' . ($this->rows * 16) . 'px', // prevents horizontal scrollbars + 'value' => null, + ) + ); + } public function saveInto($record) { if($record->escapeTypeForField($this->name) != 'xml') { diff --git a/forms/ListboxField.php b/forms/ListboxField.php index 3f79e5415..6c1c42002 100644 --- a/forms/ListboxField.php +++ b/forms/ListboxField.php @@ -60,18 +60,12 @@ class ListboxField extends DropdownField { /** * Returns a name\" id=\"$id\">$options"; + $properties = array_merge($properties, array('Options' => new ArrayList($options))); + return $this->customise($properties)->renderWith($this->getTemplate()); + } + + function getAttributes() { + return array_merge( + parent::getAttributes(), + array( + 'multiple' => $this->multiple, + 'size' => $this->size + ) + ); } /** diff --git a/forms/NumericField.php b/forms/NumericField.php index 4f9bd4310..57e97c05a 100644 --- a/forms/NumericField.php +++ b/forms/NumericField.php @@ -13,6 +13,10 @@ class NumericField extends TextField{ return $html; } + + function Type() { + return 'numeric text'; + } function jsValidation() { $formID = $this->form->FormName(); diff --git a/forms/PasswordField.php b/forms/PasswordField.php index d44633ae7..0776e12d7 100644 --- a/forms/PasswordField.php +++ b/forms/PasswordField.php @@ -24,8 +24,11 @@ class PasswordField extends TextField { } - function Type() { - return 'password'; + function getAttributes() { + return array_merge( + parent::getAttributes(), + array('type' => 'password') + ); } /** @@ -39,6 +42,10 @@ class PasswordField extends TextField { $field->setReadonly(true); return $field; } + + function Type() { + return 'text password'; + } } ?> \ No newline at end of file diff --git a/forms/ReadonlyField.php b/forms/ReadonlyField.php index 920deee73..dcea85a73 100644 --- a/forms/ReadonlyField.php +++ b/forms/ReadonlyField.php @@ -14,4 +14,23 @@ class ReadonlyField extends FormField { function performReadonlyTransformation() { return clone $this; } + + function Value() { + if($this->value) return $this->dontEscape ? $this->value : Convert::raw2xml($this->value); + else return '(' . _t('FormField.NONE', 'none') . ')'; + } + + function getAttributes() { + return array_merge( + parent::getAttributes(), + array( + 'type' => 'hidden', + 'value' => null, + ) + ); + } + + function Type() { + return 'readonly'; + } } \ No newline at end of file diff --git a/forms/ResetFormAction.php b/forms/ResetFormAction.php index 6490ad8ee..c502fb2a3 100644 --- a/forms/ResetFormAction.php +++ b/forms/ResetFormAction.php @@ -7,8 +7,15 @@ */ class ResetFormAction extends FormAction { - public function Type() { - return 'reset'; + function getAttributes() { + return array_merge( + parent::getAttributes(), + array('type' => 'reset') + ); + } + + function Type() { + return 'resetformaction'; } } \ No newline at end of file diff --git a/forms/TextField.php b/forms/TextField.php index d0f409516..615ae309f 100644 --- a/forms/TextField.php +++ b/forms/TextField.php @@ -36,16 +36,14 @@ class TextField extends FormField { return $this->maxLength; } - function Field($properties = array()) { - $properties = array_merge( - $properties, + function getAttributes() { + return array_merge( + parent::getAttributes(), array( - 'MaxLength' => ($this->getMaxLength()) ? $this->getMaxLength() : null, - 'Size' => ($this->getMaxLength()) ? min($this->getMaxLength(), 30) : null + 'maxlength' => $this->getMaxLength(), + 'size' => ($this->getMaxLength()) ? min($this->getMaxLength(), 30) : null ) ); - - return parent::Field($properties); } function InternallyLabelledField() { diff --git a/forms/TextareaField.php b/forms/TextareaField.php index 343d5db0d..504ad71ae 100644 --- a/forms/TextareaField.php +++ b/forms/TextareaField.php @@ -42,24 +42,16 @@ class TextareaField extends FormField { parent::__construct($name, $title, $value, $form); } - /** - * Create the \ No newline at end of file + \ No newline at end of file diff --git a/templates/forms/TreeDropdownField.ss b/templates/forms/TreeDropdownField.ss index cfa688843..91344b2b2 100644 --- a/templates/forms/TreeDropdownField.ss +++ b/templates/forms/TreeDropdownField.ss @@ -1,3 +1,3 @@ -
    data-metadata="$Metadata"<% end_if %>> - +
    data-metadata="$Metadata"<% end_if %>> +
    \ No newline at end of file diff --git a/tests/forms/FormFieldTest.php b/tests/forms/FormFieldTest.php index 4105040d8..88020f3e0 100644 --- a/tests/forms/FormFieldTest.php +++ b/tests/forms/FormFieldTest.php @@ -4,6 +4,50 @@ * @subpackage tests */ class FormFieldTest extends SapphireTest { + + function testAttributes() { + $field = new FormField('MyField'); + $field->setAttribute('foo', 'bar'); + $this->assertEquals('bar', $field->getAttribute('foo')); + $attrs = $field->getAttributes(); + $this->assertArrayHasKey('foo', $attrs); + $this->assertEquals('bar', $attrs['foo']); + } + + function testAttributesHTML() { + $field = new FormField('MyField'); + + $field->setAttribute('foo', 'bar'); + $this->assertContains('foo="bar"', $field->getAttributesHTML()); + + $field->setAttribute('foo', null); + $this->assertNotContains('foo=', $field->getAttributesHTML()); + + $field->setAttribute('foo', ''); + $this->assertNotContains('foo=', $field->getAttributesHTML()); + + $field->setAttribute('foo', false); + $this->assertNotContains('foo=', $field->getAttributesHTML()); + + $field->setAttribute('foo', true); + $this->assertContains('foo="foo"', $field->getAttributesHTML()); + + $field->setAttribute('foo', 'false'); + $this->assertContains('foo="false"', $field->getAttributesHTML()); + + $field->setAttribute('foo', 'true'); + $this->assertContains('foo="true"', $field->getAttributesHTML()); + + $field->setAttribute('foo', 0); + $this->assertContains('foo="0"', $field->getAttributesHTML()); + + $field->setAttribute('one', 1); + $field->setAttribute('two', 2); + $field->setAttribute('three', 3); + $this->assertNotContains('one="1"', $field->getAttributesHTML('one', 'two')); + $this->assertNotContains('two="2"', $field->getAttributesHTML('one', 'two')); + $this->assertContains('three="3"', $field->getAttributesHTML('one', 'two')); + } function testEveryFieldTransformsReadonlyAsClone() { $fieldClasses = ClassInfo::subclassesFor('FormField'); diff --git a/tests/model/LabelFieldTest.php b/tests/model/LabelFieldTest.php index 1d420c288..7e5ac740c 100644 --- a/tests/model/LabelFieldTest.php +++ b/tests/model/LabelFieldTest.php @@ -8,6 +8,6 @@ class LabelFieldTest extends SapphireTest { function testFieldHasNoNameAttribute() { $field = new LabelField('MyName', 'MyTitle'); - $this->assertEquals($field->Field(), ''); + $this->assertEquals($field->Field(), ''); } }