ENHANCEMENT Added DropdownField->setEmptyString() and DropdownField->setHasEmptyDefault() to decrease constructor arguments and allow for modification of behaviour after calling construtor. Added DropdownField->getSource() to dynamically include empty defaults at rendering-time rather than construction time. Adjusted DropdownField subclasses to new behaviour

ENHANCEMENT Added DropdownFieldTest

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@64304 467b73ca-7a2a-4603-9d3b-597d59a354a9

forms/CountryDropdownField.php

@@ -8,11 +8,11 @@
 class CountryDropdownField extends DropdownField {
 	protected $defaultToVisitorCountry = true;
 	
-	function __construct($name, $title, $source = null, $value = "", $form=null, $emptyString="--select--") {
+	function __construct($name, $title, $source = null, $value = "", $form=null) {
 		if(!is_array($source)) {
 			$source = Geoip::getCountryDropDown();
 		}
-		parent::__construct($name, $title, $source, $value, $form, $emptyString);
+		parent::__construct($name, $title, $source, $value, $form);
 	}
 	
 	function defaultToVisitorCountry($val) {
@@ -20,7 +20,8 @@ class CountryDropdownField extends DropdownField {
 	}
 	
 	function Field() {
-		if($this->defaultToVisitorCountry && !$this->value || !isset($this->source[$this->value])) {
+		$source = $this->getSource();
+		if($this->defaultToVisitorCountry && !$this->value || !isset($source[$this->value])) {
 			$this->value = ($vc = Geoip::visitor_country()) ? $vc : Geoip::$default_country_code;
 		}
 		return parent::Field();

forms/DropdownField.php

@@ -5,8 +5,39 @@
  * @subpackage fields-basic
  */
 class DropdownField extends FormField {
+	
+	/**
+	 * @var boolean $source Associative or numeric array of all dropdown items,
+	 * with array key as the submitted field value, and the array value as a
+	 * natural language description shown in the interface element.
+	 */
 	protected $source;
-	protected $isSelected, $disabled;
+	
+	/**
+	 * @var boolean $isSelected Determines if the field was selected
+	 * at the time it was rendered, so if {@link $value} matches on of the array
+	 * values specified in {@link $source}
+	 */
+	protected $isSelected;
+	
+	/**
+	 * @var boolean $disabled
+	 */
+	protected $disabled;
+	
+	/**
+	 * @var boolean $hasEmptyDefault Show the first <option> element as
+	 * empty (not having a value), with an optional label defined through
+	 * {@link $emptyString}. By default, the <select> element will be
+	 * rendered with the first option from {@link $source} selected.
+	 */
+	protected $hasEmptyDefault = false;
+	
+	/**
+	 * @var string $emptyString The title shown for an empty default selection,
+	 * e.g. "Select...".
+	 */
+	protected $emptyString = '';
 	
 	/**
 	 * Creates a new dropdown field.
@@ -15,17 +46,16 @@ class DropdownField extends FormField {
 	 * @param $source An map of the dropdown items
 	 * @param $value The current value
 	 * @param $form The parent form
-	 * @param $emptyString mixed Add an empty selection on to of the {source}-Array 
+	 * @param $emptyString mixed Add an empty selection on to of the {@link $source}-Array 
 	 * 	(can also be boolean, which results in an empty string)
+	 *  Argument is deprecated in 2.3, please use {@link setHasEmptyDefault()} and {@link setEmptyString()} instead.
 	 */
 	function __construct($name, $title = null, $source = array(), $value = "", $form = null, $emptyString = null) {
-		if(is_string($emptyString)) {
-			$source = is_array($source) ? array(""=>$emptyString) + $source : array(""=>$emptyString);
-		} elseif($emptyString === true) {
-			$source = is_array($source) ? array(""=>"") + $source : array(""=>"");
-		}
 		$this->source = $source;
 		
+		if($emptyString) $this->setHasEmptyDefault(true);
+		if(is_string($emptyString)) $this->setEmptyString($emptyString);
+	
 		parent::__construct($name, ($title===null) ? $name : $title, $value, $form);
 	}
 	
@@ -39,8 +69,9 @@ class DropdownField extends FormField {
 	function Field() {
 		$options = '';
 
-		if($this->source) foreach($this->source as $value => $title) {
+		$source = $this->getSource();
-			$selected = $value == $this->value ? 'selected' : null;
+		foreach($source as $value => $title) {
+			$selected = ($value == $this->value) ? 'selected' : null;
 			if($selected && $this->value != 0) {
 				$this->isSelected = true;
 			}
@@ -66,18 +97,66 @@ class DropdownField extends FormField {
 		return $this->createTag('select', $attributes, $options);
 	}
 	
+	/**
+	 * @return boolean
+	 */
 	function isSelected(){
 		return $this->isSelected;
 	}
   
+	/**
+	 * Gets the source array including any empty default values.
+	 * 
+	 * @return array
+	 */
 	function getSource() {
-		return $this->source;
+		if($this->getHasEmptyDefault()) {
+			return array(""=>$this->emptyString) + (array)$this->source;
+		} else {
+			return (array)$this->source;
+		}
 	}
   
+	/**
+	 * @param array $source
+	 */
 	function setSource($source) {
 		$this->source = $source;
 	}
 	
+	/**
+	 * @param boolean $bool
+	 */
+	function setHasEmptyDefault($bool) {
+		$this->hasEmptyDefault = $bool;
+	}
+	
+	/**
+	 * @return boolean
+	 */
+	function getHasEmptyDefault() {
+		return $this->hasEmptyDefault;
+	}
+	
+	/**
+	 * Set the default selection label, e.g. "select...".
+	 * Defaults to an empty string. Automatically sets
+	 * {@link $hasEmptyDefault} to true.
+	 * 
+	 * @param string $str
+	 */
+	function setEmptyString($str) {
+		$this->setHasEmptyDefault(true);
+		$this->emptyString = $str;
+	}
+	
+	/**
+	 * @return string
+	 */
+	function getEmptyString() {
+		return $this->emptyString;
+	}
+
 	function performReadonlyTransformation() {
 		$field = new LookupField($this->name, $this->title, $this->source);
 		$field->setValue($this->value);

forms/GroupedDropdownField.php

@@ -22,19 +22,18 @@ class GroupedDropdownField extends DropdownField {
 		if($extraClass = trim($this->extraClass())) {
 			$classAttr = "class=\"$extraClass\"";
 		}
-		if($this->source) {
+		
-			foreach($this->source as $value => $title) {
+		foreach($this->getSource() as $value => $title) {
-				if(is_array($title)) {
+			if(is_array($title)) {
-					$options .= "<optgroup label=\"$value\">";
+				$options .= "<optgroup label=\"$value\">";
-					foreach($title as $value2 => $title2) {
+				foreach($title as $value2 => $title2) {
-						$selected = $value2 == $this->value ? " selected=\"selected\"" : "";
+					$selected = $value2 == $this->value ? " selected=\"selected\"" : "";
-						$options .= "<option$selected value=\"$value2\">$title2</option>";
+					$options .= "<option$selected value=\"$value2\">$title2</option>";
-					}
-					$options .= "</optgroup>";
-				} else { // Fall back to the standard dropdown field
-					$selected = $value == $this->value ? " selected=\"selected\"" : "";
-					$options .= "<option$selected value=\"$value\">$title</option>";
 				}
+				$options .= "</optgroup>";
+			} else { // Fall back to the standard dropdown field
+				$selected = $value == $this->value ? " selected=\"selected\"" : "";
+				$options .= "<option$selected value=\"$value\">$title</option>";
 			}
 		}
 

forms/ListboxField.php

@@ -43,7 +43,7 @@ class ListboxField extends DropdownField {
 		if(is_array($this->value)){
 			// Loop through and figure out which values were selected.
 					
-			foreach($this->source as $value => $title) {
+			foreach($this->getSource() as $value => $title) {
 				// Loop through the array of values to find out if this value is selected.
 				$selected = "";
 				foreach($this->value as $v){
@@ -56,11 +56,9 @@ class ListboxField extends DropdownField {
 			}
 		}else{
 			// Listbox was based a singlular value, so treat it like a dropdown.
-			if($this->source){
+			foreach($this->getSource() as $value => $title) {
-				foreach($this->source as $value => $title) {
+				$selected = $value == $this->value ? " selected=\"selected\"" : ""; 
-					$selected = $value == $this->value ? " selected=\"selected\"" : ""; 
+				$options .= "<option$selected value=\"$value\">$title</option>";
-					$options .= "<option$selected value=\"$value\">$title</option>";
-				}
 			}
 		}
 		

forms/LookupField.php

@@ -16,16 +16,15 @@ class LookupField extends DropdownField {
 		
 		if(trim($this->value) || $this->value === '0') {
 			$this->value = trim($this->value);
-			if(is_array($this->source)) {
+			$source = $this->getSource();
-				$mappedValue = isset($this->source[$this->value]) ? $this->source[$this->value] : null;
+			if(is_array($source)) {
-			} else {
+				$mappedValue = isset($source[$this->value]) ? $source[$this->value] : null;
-				$mappedValue = $this->source->getItem($this->value);
+			} elseif($source instanceof SQLMap) {
+				$mappedValue = $source->getItem($this->value);
 			}
 		}
 		
-		if(!isset($mappedValue)) {
+		if(!isset($mappedValue)) $mappedValue = "<i>(none)</i>";
-			$mappedValue = "<i>(none)</i>";
-		}
 
 		if($this->value) {
 			$val = $this->dontEscape
@@ -35,9 +34,7 @@ class LookupField extends DropdownField {
 			$val = '<i>(none)</i>';
 		}
 
-		$valforInput = $this->value
+		$valforInput = $this->value ? Convert::raw2att($val) : "";
-			? Convert::raw2att($val)
-			: "";
 
 		return "<span class=\"readonly\" id=\"" . $this->id() .
 			"\">$mappedValue</span><input type=\"hidden\" name=\"" . $this->name .
@@ -51,6 +48,13 @@ class LookupField extends DropdownField {
 	function Type() { 
 		return "lookup readonly";
 	}
+	
+	/**
+	 * Override parent behaviour by not merging arrays.
+	 */
+	function getSource() {
+		return $this->source;
+	}
 }
 
 ?>

forms/OptionsetField.php

@@ -28,7 +28,8 @@ class OptionsetField extends DropdownField {
 	function Field() {
 		$options = '';
 		$odd = 0;
-		foreach($this->source as $key => $value) {
+		$source = $this->getSource();
+		foreach($source as $key => $value) {
 			$itemID = $this->id() . "_" . ereg_replace('[^a-zA-Z0-9]+','',$key);
 		
 			if($key == $this->value/* || $useValue */) {
@@ -56,7 +57,7 @@ class OptionsetField extends DropdownField {
 	
 	function performReadonlyTransformation() {
 		// Source and values are DataObject sets.
-		$items = $this->source;
+		$items = $this->getSource();
 		$field = new LookupField($this->name,$this->title ? $this->title : "" ,$items,$this->value);
 		$field->setForm($this->form);
 		$field->setReadonly(true);

tests/forms/DropdownFieldTest.php

@@ -0,0 +1,85 @@
+<?php
+/**
+ * @package sapphire
+ * @subpackage tests
+ */
+class DropdownFieldTest extends SapphireTest {
+	
+	function testGetSource() {
+		$source = array(1=>'one');
+		$field = new DropdownField('Field', null, $source);
+		$this->assertEquals(
+			$field->getSource(),
+			array(
+				1 => 'one'
+			)
+		);
+	}
+	
+	function testEmptyStringAsBooleanConstructorArgument() {
+		$source = array(1=>'one');
+		$field = new DropdownField('Field', null, $source, null, null, true);
+		$this->assertEquals(
+			$field->getSource(),
+			array(
+				'' => '',
+				1 => 'one'
+			)
+		);
+	}
+	
+	function testEmptyStringAsLiteralConstructorArgument() {
+		$source = array(1=>'one');
+		$field = new DropdownField('Field', null, $source, null, null, 'select...');
+		$this->assertEquals(
+			$field->getSource(),
+			array(
+				"" => 'select...',
+				1 => 'one'
+			)
+		);
+	}
+	
+	function testHasEmptyDefault() {
+		$source = array(1=>'one');
+		$field = new DropdownField('Field', null, $source);
+		$field->setHasEmptyDefault(true);
+		$this->assertEquals(
+			$field->getSource(),
+			array(
+				'' => '',
+				1 => 'one'
+			)
+		);
+	}
+	
+	function testEmptyDefaultStringThroughSetter() {
+		$source = array(1=>'one');
+		$field = new DropdownField('Field', null, $source);
+		$field->setEmptyString('select...');
+		$this->assertEquals(
+			$field->getSource(),
+			array(
+				'' => 'select...',
+				1 => 'one'
+			)
+		);
+		$this->assertTrue(
+			$field->getHasEmptyDefault()
+		);
+	}
+	
+	function testZeroArraySourceNotOverwrittenByEmptyString() {
+		$source = array(0=>'zero');
+		$field = new DropdownField('Field', null, $source);
+		$field->setEmptyString('select...');
+		$this->assertEquals(
+			$field->getSource(),
+			array(
+				'' => 'select...',
+				0 => 'zero'
+			)
+		);
+	}
+}
+?>