Fix incorrect option selection logic and improve data binding for Select and Checkable fields (#617)

Author: AleksandrSpicyn

src/Former/Form/Fields/Select.php

@@ -95,6 +95,8 @@ public function render()
 			$this->value = (array) $this->value;
 		}
 
+		$this->clearSelected();
+
 		// Mark selected values as selected
 		if ($this->hasChildren() and !empty($this->value)) {
 			foreach ($this->value as $value) {
@@ -131,9 +133,9 @@ protected function selectValue($value, $parent = null)
 		}
 
 		foreach ($parent->getChildren() as $child) {
-			// Search by value
+			$optionValue = $child->getAttribute('value');
 
-			if ($child->getAttribute('value') === $value || is_numeric($value) && $child->getAttribute('value') === (int)$value ) {
+			if ($optionValue === $value || (is_numeric($value) && is_numeric($optionValue) && (int)$optionValue === (int)$value)) {
 				$child->selected('selected');
 			}
 
@@ -239,7 +241,11 @@ public function addOption($text = null, $value = null, $attributes = array())
 		if (is_array($text)) {
 			$this->children[$childrenKey] = Element::create('optgroup')->label($value);
 			foreach ($text as $key => $value) {
-				$option = Element::create('option', $value)->setAttribute('value', $key);
+				if (is_array($value)) {
+					$option = Element::create('option', $key)->setAttributes($value);
+				} else {
+					$option = Element::create('option', $value)->setAttribute('value', $key);
+				}
 				$this->children[$childrenKey]->nest($option);
 			}
 			// Else if it's a simple option
@@ -254,6 +260,29 @@ public function addOption($text = null, $value = null, $attributes = array())
 		return $this;
 	}
 
+	/**
+	 * Clear selected attribute for select options
+	 *
+	 * @param Element $parent
+	 *
+	 * @return void
+	 */
+	public function clearSelected($parent = null)
+	{
+		// If no parent element defined, use direct children
+		if (!$parent) {
+			$parent = $this;
+		}
+
+		foreach ($parent->getChildren() as $child) {
+			$child->removeAttribute('selected');
+
+			if ($child->hasChildren()) {
+				$this->clearSelected($child);
+			}
+		}
+	}
+
 	/**
 	 * Use the results from a Fluent/Eloquent query as options
 	 *

src/Former/Helpers.php

@@ -29,7 +29,7 @@ public static function setApp(Container $app)
 	/**
 	 * Encodes HTML
 	 *
-	 * @param string|null $value The string to encode
+	 * @param string|array|null $value The string to encode
 	 *
 	 * @return string
 	 */
@@ -39,6 +39,10 @@ public static function encode($value)
 			return '';
 		}
 
+		if (is_array($value)) {
+			$value = '';
+		}
+
 		return htmlentities($value, ENT_QUOTES, 'UTF-8', true);
 	}
 

src/Former/Traits/Checkable.php

@@ -257,6 +257,24 @@ public function check($checked = true)
 		return $this;
 	}
 
+	/**
+	 * Creates a series of checkable items from query with attributes
+	 *
+	 * @param mixed $query
+	 * @param mixed $text
+	 * @param mixed $attributes
+	 * @return $this
+	 */
+	public function fromQuery($query, $text = null, $attributes = null)
+	{
+		if ($this->isGrouped()) {
+			// Remove any possible items added by the Populator.
+			$this->items = array();
+		}
+		$this->items($query, $text, $attributes);
+
+		return $this;
+	}
 
 	/**
 	 * Check if the checkables are inline
@@ -275,9 +293,11 @@ public function isInline()
 	/**
 	 * Creates a series of checkable items
 	 *
-	 * @param array $_items Items to create
+	 * @param array|Collection $_items Items to create
+	 * @param string|function  $text The value to use as text
+	 * @param string|array     $attributes The data to use as attributes
 	 */
-	protected function items($_items)
+	protected function items($_items, $text = null, $attributes = null)
 	{
 		// If passing an array
 		if (sizeof($_items) == 1 and
@@ -288,9 +308,11 @@ protected function items($_items)
 		}
 
 		// Fetch models if that's what we were passed
-		if (isset($_items[0]) and is_object($_items[0])) {
-			$_items = Helpers::queryToArray($_items);
-			$_items = array_flip($_items);
+		if ((isset($_items[0]) and is_object($_items[0])) or ($_items instanceof Collection)) {
+			$_items = Helpers::queryToArray($_items, $text, $attributes);
+			if (is_null($text) && is_null($attributes)) {
+				$_items = array_flip($_items);
+			}
 		}
 
 		// Iterate through items, assign a name and a label to each
@@ -542,7 +564,7 @@ protected function isChecked($item = null, $value = null)
 
 		if (!is_null($post) and $post !== $this->app['former']->getOption('unchecked_value')) {
 			$isChecked = ($post == $value);
-		} elseif (!is_null($static)) {
+		} elseif (!is_null($static) && !(is_array($static) && empty($static))) {
 			$isChecked = ($static == $value);
 		} else {
 			$isChecked = $checked;

tests/Fields/SelectTest.php

@@ -542,4 +542,57 @@ public function testSelectCanPickRightOptionWithOptgroups()
 
 		$this->assertStringContainsString($matcher, $select->render());
 	}
+
+	public function testCanClearPreviouslySelectedOptions()
+	{
+		$select = $this->former->select('foo')->options([
+			'One' => ['value' => 1, 'selected' => 'selected'],
+			'Two' => ['value' => 2],
+		])->select(2);
+
+		$matcher = $this->controlGroup(
+			'<select id="foo" name="foo">' .
+			'<option value="1">One</option>' .
+			'<option value="2" selected="selected">Two</option>' .
+			'</select>'
+		);
+
+		$this->assertEquals($matcher, $select->__toString());
+	}
+
+	public function testOptgroupSupportsNestedOptionsWithAttributes()
+	{
+		$select = $this->former->select('foo')->addOption([
+			'A' => ['value' => 1, 'data-type' => 'x'],
+			'B' => ['value' => 2, 'data-type' => 'y'],
+		], 'Group 1');
+
+		$matcher = $this->controlGroup(
+			'<select id="foo" name="foo">' .
+			'<optgroup label="Group 1">' .
+			'<option value="1" data-type="x">A</option>' .
+			'<option value="2" data-type="y">B</option>' .
+			'</optgroup>' .
+			'</select>'
+		);
+
+		$this->assertEquals($matcher, $select->__toString());
+	}
+
+	public function testSelectHandlesLooseTypeMatchingForValues()
+	{
+		$select = $this->former->select('foo')->options([
+			'Zero' => ['value' => 0],
+			'One' => ['value' => 1],
+		])->select('0');
+
+		$matcher = $this->controlGroup(
+			'<select id="foo" name="foo">' .
+			'<option value="0" selected="selected">Zero</option>' .
+			'<option value="1">One</option>' .
+			'</select>'
+		);
+
+		$this->assertEquals($matcher, $select->__toString());
+	}
 }