Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

set_multiselect in form_helper and Form_validation #1799

Closed
wants to merge 1 commit into from

6 participants

@Zoubi

I found useful to write these functions, to automatically fill a multiselect in a view, using this kind of code:

<?= form_multiselect( 'groups[]', $groups, set_multiselect( 'groups', $selected_groups ) ) ?>

@alexbilbie

Please can you add a changelog and write a unit test for the feature

@cryode cryode commented on the diff
system/libraries/Form_validation.php
@@ -934,6 +934,47 @@ public function set_checkbox($field = '', $value = '', $default = FALSE)
// --------------------------------------------------------------------
/**
+ * Set multiselect
+ *
+ * Enables multiselect to be set to the value the user
+ * selected in the event of an error
+ *
+ * @param string
+ * @param array
+ * @return array
+ */
+ public function set_multiselect( $field = '', $values = array() )
+ {
+ if ( ! isset($this->_field_data[$field]) OR ! isset($this->_field_data[$field]['postdata']))
@cryode
cryode added a note

These should be AND instead of OR. You can add both as parameters to one isset() as the other methods have (keeps it consistent).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@cryode

I'd suggest returning NULL instead of an empty array.

@Dentxinho

@cryode it returns an empty array because of form_dropdown():

function form_dropdown($name = '', $options = array(), $selected = array(), $extra = '')
@cryode

In my opinion, an empty array constitutes a valid value, and should not be returned as the value of POST items that do not exist. And form_dropdown() doesn't require an array, anyway.

Taking a look at the existing form_dropdown() function, the end result of this pull request could easily be achieved by just modifying form_dropdown() itself, placing an existing POST value as greater priority over the $selected parameter. Zero extra additions, and only a few lines of code modified. Since set_multiselect() would likely only be used in this scenario, there's no real point in adding the function.

I encourage someone to prove me wrong :)

Actually, it might be worth discussing if form_dropdown() should even be looking at the POST array in the first place. Not a single other form_* function does. If removing that is the case, then this request would be useful.

Another thought: should rename this function to set_dropdown() instead, since it can be applied to form_dropdown() whether it is a multi-select or not.

@evinw
@narfbg narfbg closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 14, 2012
  1. @Zoubi
This page is out of date. Refresh to see the latest.
View
47 system/helpers/form_helper.php
@@ -831,6 +831,53 @@ function set_radio($field = '', $value = '', $default = FALSE)
// ------------------------------------------------------------------------
+if ( ! function_exists('set_multiselect'))
+{
+ /**
+ * Set Multiselect
+ *
+ * Let's you set the selected value of a multiselect field via info in the POST array.
+ * If Form Validation is active it retrieves the info from the validation class
+ *
+ * @param string
+ * @param array
+ * @return array
+ */
+
+ function set_multiselect( $field = '', $values = array() )
+ {
+ $OBJ =& _get_validation_object();
+
+ if ( $OBJ === FALSE )
+ {
+ if ( ! isset( $_POST[ $field ] ) )
+ {
+ return $values;
+ }
+
+ $field = $_POST[ $field ];
+
+ if ( is_array( $field ) )
+ {
+ return $field;
+ }
+ else
+ {
+ if ( ( $field == '' OR empty( $value ) OR ( ! in_array( $field, $value ) ) ) )
+ {
+ return array();
+ }
+ }
+
+ return $values;
+ }
+
+ return $OBJ->set_multiselect( $field, $values );
+ }
+}
+
+// ------------------------------------------------------------------------
+
if ( ! function_exists('form_error'))
{
View
41 system/libraries/Form_validation.php
@@ -934,6 +934,47 @@ public function set_checkbox($field = '', $value = '', $default = FALSE)
// --------------------------------------------------------------------
/**
+ * Set multiselect
+ *
+ * Enables multiselect to be set to the value the user
+ * selected in the event of an error
+ *
+ * @param string
+ * @param array
+ * @return array
+ */
+ public function set_multiselect( $field = '', $values = array() )
+ {
+ if ( ! isset($this->_field_data[$field]) OR ! isset($this->_field_data[$field]['postdata']))
@cryode
cryode added a note

These should be AND instead of OR. You can add both as parameters to one isset() as the other methods have (keeps it consistent).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ {
+ if ( count( $this->_field_data ) === 0 )
+ {
+ return $values;
+ }
+
+ return array();
+ }
+
+ $field = $this->_field_data[$field]['postdata'];
+
+ if ( is_array( $field ) )
+ {
+ return $field;
+ }
+ else
+ {
+ if ( ( $field == '' OR empty( $value ) OR ( ! in_array( $field, $value ) ) ) )
+ {
+ return array();
+ }
+ }
+
+ return $values;
+ }
+
+ // --------------------------------------------------------------------
+
+ /**
* Required
*
* @param string
Something went wrong with that request. Please try again.