API: Change DropdownField::getSource() to not return the emptyString value.

forms/DropdownField.php

@@ -149,29 +149,24 @@ class DropdownField extends FormField {
 	public function Field($properties = array()) {
 		$source = $this->getSource();
 		$options = array();
-		if($source) {
-			// SQLMap needs this to add an empty value to the options
-			if(is_object($source) && $this->emptyString) {
-				$options[] = new ArrayData(array(
-					'Value' => '',
-					'Title' => $this->emptyString,
-				));
-			}
 
+		if ($this->getHasEmptyDefault()) {
+			$selected = ($this->value === '' || $this->value === null);
+			$disabled = (in_array('', $this->disabledItems, true)) ? 'disabled' : false;
+
+			$options[] = new ArrayData(array(
+				'Value' => '',
+				'Title' => $this->getEmptyString(),
+				'Selected' => $selected,
+				'Disabled' => $disabled
+			));
+		}
+
+		if ($source) {
 			foreach($source as $value => $title) {
-				$selected = false;
+				// check against value, fallback to a type check comparison when !value
-				if($value === '' && ($this->value === '' || $this->value === null)) {
+				$selected = ($value) ? ($value == $this->value) : ($value === $this->value);
-					$selected = true;
+				$disabled = (in_array($value, $this->disabledItems, true)) ? 'disabled' : false;
-				} else {
-					// check against value, fallback to a type check comparison when !value
-					$selected = ($value) ? $value == $this->value : $value === $this->value;
-					$this->isSelected = $selected;
-				}
-				
-				$disabled = false;
-				if(in_array($value, $this->disabledItems) && $title != $this->emptyString ){
-					$disabled = 'disabled';
-				}
 
 				$options[] = new ArrayData(array(
 					'Title' => $title,
@@ -182,33 +177,42 @@ class DropdownField extends FormField {
 			}
 		}
 
-		$properties = array_merge($properties, array('Options' => new ArrayList($options)));
+		$properties = array_merge($properties, array(
+			'Options' => new ArrayList($options)
+		));
 
 		return parent::Field($properties);
 	}
 	
 	/**
-	 * Mark certain elements as disabled,
+	 * Mark certain elements as disabled, regardless of the 
-	 * regardless of the {@link setDisabled()} settings.
+	 * {@link setDisabled()} settings.
 	 * 
 	 * @param array $items Collection of array keys, as defined in the $source array
 	 */
-	public function setDisabledItems($items){
+	public function setDisabledItems($items) {
 		$this->disabledItems = $items;
+
 		return $this;
 	}
 	
 	/**
-	 * @return Array
+	 * @return array
 	 */
-	public function getDisabledItems(){
+	public function getDisabledItems() {
 		return $this->disabledItems;
 	}
 
+	/**
+	 * @return array
+	 */
 	public function getAttributes() {
 		return array_merge(
 			parent::getAttributes(),
-			array('type' => null, 'value' => null)
+			array(
+				'type' => null, 
+				'value' => null
+			)
 		);
 	}
 
@@ -225,11 +229,7 @@ class DropdownField extends FormField {
 	 * @return array
 	 */
 	public function getSource() {
-		if(is_array($this->source) && $this->getHasEmptyDefault()) {
+		return $this->source;
-			return array('' => $this->emptyString) + (array) $this->source;
-		} else {
-			return $this->source;
-		}
 	}
 
 	/**
@@ -237,6 +237,7 @@ class DropdownField extends FormField {
 	 */
 	public function setSource($source) {
 		$this->source = $source;
+
 		return $this;
 	}
 	
@@ -245,6 +246,7 @@ class DropdownField extends FormField {
 	 */
 	public function setHasEmptyDefault($bool) {
 		$this->hasEmptyDefault = $bool;
+
 		return $this;
 	}
 	
@@ -257,14 +259,16 @@ class DropdownField extends FormField {
 
 	/**
 	 * Set the default selection label, e.g. "select...".
-	 * Defaults to an empty string. Automatically sets
+	 * 
-	 * {@link $hasEmptyDefault} to true.
+	 * Defaults to an empty string. Automatically sets {@link $hasEmptyDefault} 
+	 * to true.
 	 *
 	 * @param string $str
 	 */
 	public function setEmptyString($str) {
 		$this->setHasEmptyDefault(true);
 		$this->emptyString = $str;
+
 		return $this;
 	}
 
@@ -275,6 +279,9 @@ class DropdownField extends FormField {
 		return $this->emptyString;
 	}
 
+	/**
+	 * @return LookupField
+	 */
 	public function performReadonlyTransformation() {
 		$field = $this->castedCopy('LookupField');
 		$field->setSource($this->getSource());

templates/forms/DropdownField.ss

@@ -1,5 +1,5 @@
 <select $AttributesHTML>
 <% loop $Options %>
-	<option value="$Value.XML"<% if $Selected %> selected="selected"<% end_if %><% if $Disabled %> disabled="disabled"<% end_if %>>$Title.XML</option>
+	<option value="$Value.XML"<% if $Selected %> selected="selected"<% end_if %><% if $Disabled %> disabled="disabled"<% end_if %>><% if Title %>$Title.XML<% else %>&nbsp;<% end_if %></option>
 <% end_loop %>
 </select>

tests/forms/DropdownFieldTest.php

@@ -25,45 +25,52 @@ class DropdownFieldTest extends SapphireTest {
 		$this->assertEquals($matches[0], 'Yes');
 	}
 
-	public function testEmptyStringAsLiteralConstructorArgument() {
-		$source = array(1 => 'one');
-		$field = new DropdownField('Field', null, $source);
-		$field->setEmptyString('select...');
-		$this->assertEquals(
-			$field->getSource(),
-			array(
-				'' => 'select...',
-				1 => 'one'
-			)
-		);
-	}
-
 	public function testHasEmptyDefault() {
 		$source = array(1 => 'one');
+
+		// Test getSource with empty
 		$field = new DropdownField('Field', null, $source);
 		$field->setHasEmptyDefault(true);
-		$this->assertEquals(
-			$field->getSource(),
-			array(
-				'' => '',
-				1 => 'one'
-			)
-		);
-	}
 
-	public 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()
+		// Test that an empty option comes through in the markup however
+		$options = $this->findOptionElements($field->Field());
+
+		$this->assertEquals(
+			2,
+			count($options),
+			'Two options exist in the markup, one for the source, one for empty'
+		);
+
+		// the default value should be first
+		$first = array_shift($options);
+		$attrs = $first->attributes();
+
+		$this->assertNotEquals(
+			1,
+			$attrs['value'],
+			'First value is the not value (not the source value)'
+		);
+
+		// Test Field Without Empty
+		$FieldWithoutEmpty = new DropdownField('Field', null, $source);
+		$this->assertEquals(
+			$FieldWithoutEmpty->getSource(),
+			array(
+				1 => 'one'
+			)
+		);
+
+		$this->assertEquals(
+			1,
+			count($options),
+			'As hasEmptyDefault is not provided, then no default option.'
 		);
 	}
 
@@ -74,10 +81,17 @@ class DropdownFieldTest extends SapphireTest {
 		$this->assertEquals(
 			$field->getSource(),
 			array(
-				'' => 'select...',
 				0 => 'zero'
 			)
 		);
+
+		$options = $this->findOptionElements($field->Field());
+
+		$this->assertEquals(
+			2,
+			count($options),
+			'Two options exist in the markup, one for the source, one for empty'
+		);
 	}
 	
 	public function testNumberOfSelectOptionsAvailable() {
@@ -181,7 +195,10 @@ class DropdownFieldTest extends SapphireTest {
 		);
 		
 		$field = new DropdownField('Field', null, $source, $value);
-		if($emptyString !== null) $field->setEmptyString($emptyString);
+
+		if($emptyString !== null) {
+			$field->setEmptyString($emptyString);
+		}
 
 		return $field;
 	}
@@ -248,4 +265,4 @@ class DropdownFieldTest extends SapphireTest {
 		return $foundDisabled;
 	}
 	
-}
+}