on single selects, multi-selects and having a first empty option

[maniqui]

While investigating this bug, I noticed that there is some kind of flaw in the logic behind the inclusion (or not) of a first empty option on single selects and multi-selects.

In both single selects and multi-selects, the code logic assumes that if there is a {default} value ($default_value var), then there is no need for having a first empty option.

Consequences for single selects

If an option is selected by default (by wrapping it on {}), then, there is no first empty option.. Thus, the user can't select "nothing". It's "obligated" to select one of the options on the select. IMO, it's incorrect to assume that because a default value was set, then the select doesn't have to include the first empty option, making it impossible for the user to select "nothing".

Consequences for multi selects

First, let me state: as I tried to explain on this comment to previous bug report, there is no need for a first empty option on multi-selects. Users can "select" an "empty option" by not selecting any option in a multi-select input. I can understand that it may be easier for non-tech-savvy users to click on the first empty option ("to select nothing") than having to un-select an option by doing Ctrl+click. But that doesn't make it correct.

In any case, if the first empty option is to stay, at least, don't make it dependent on the current condition ("if there is no $default_value, a first empty option is added. Else, if there is a $default_value, the first empty option isn't added").

Bottom line: $default_value or not, a first empty option should be controlled by another variable (for example, an $has_empty_option variable).

Next comment will include some little hacks to glz_selectInput that try to address this issue.

[maniqui]

This is the original glz_selectInput function:

function glz_selectInput($name = '', $id = '', $arr_values = '', $custom_value = '', $default_value = '', $multi = '') {
  if ( is_array($arr_values) ) {
    global $prefs;
    $out = array();

    // if there is no custom_value coming from the article, let's use our default one
    if ( empty($custom_value) )
      $custom_value = $default_value;

    foreach ($arr_values as $key => $value) {
      $selected = glz_selected_checked('selected', $key, $custom_value, $default_value);
      $out[] = "<option value=\"$key\"{$selected}>$value</option>";
    }

    // we'll need the extra attributes as well as a name that will produce an array
    if ($multi) {
      $multi = ' multiple="multiple" size="'.$prefs['multiselect_size'].'"';
      $name .= "[]";
    }

    return "<select id=\"".glz_idify($id)."\" name=\"$name\" class=\"list\"$multi>".
      ($default_value ? '' : "<option value=\"\"$selected>&nbsp;</option>").
      ( $out ? join('', $out) : '').
      "</select>";
  }
  else
    return glz_custom_fields_gTxt('field_problems', array('{custom_set_name}' => $name));
}

The line related to the issue I'm trying to address is this one:

($default_value ? '' : "<option value=\"\"$selected>&nbsp;</option>").

That's the line that says: "if there is a default value, don't add a first empty option. Else, add it." Disregard the $selected bit, as it is, imo, part of this bug and I think it should be removed.

As stated on the opening post, I think that the inclusion (or not) of this first empty option should be controlled by some other flag. Maybe via a new extra parameter $include_empty_option?

This is my first take at addressing this issue:

function glz_selectInput($name = '', $id = '', $arr_values = '', $custom_value = '', $default_value = '', $multi = '') {
  if ( is_array($arr_values) ) {
    global $prefs;
    $out = array();
    $include_empty_option = true;

    // if there is no custom_value coming from the article, let's use our default one
    if ( empty($custom_value) )
      $custom_value = $default_value;

    foreach ($arr_values as $key => $value) {
      $selected = glz_selected_checked('selected', $key, $custom_value);
      $out[] = "<option value=\"$key\"{$selected}>$value</option>";
    }

    // we'll need the extra attributes as well as a name that will produce an array
    if ($multi) {
      $multi = ' multiple="multiple" size="'.$prefs['multiselect_size'].'"';
      $name .= "[]";
      $include_empty_option = false;
    }

    return "<select id=\"".glz_idify($id)."\" name=\"$name\" class=\"list\"$multi>".
      ( $include_empty_option ? "<option value=\"\">&nbsp;</option>" : '' ).
      ( $out ? join('', $out) : '').
      "</select>";
  }
  else
    return glz_custom_fields_gTxt('field_problems', array('{custom_set_name}' => $name));
}

As you can see, I created a $include_empty_option boolean flag and set it to true by default. Then, it is set to false if this is a multi-select. Finally, in the return statement I replaced:

( $default_value ? '' : "<option value=\"\"$selected>&nbsp;</option>").

With:

( $include_empty_option ? "<option value=\"\">&nbsp;</option>" : '' ).

This version doesn't provide a way to enable the first empty option for multi-selects, nor to disable it for single selects.

[maniqui]

And this is a second take, that relies on declaring an extra parameter

function glz_selectInput($name = '', $id = '', $arr_values = '', $custom_value = '', $default_value = '', $multi = '',         $include_empty_option = true) {
  if ( is_array($arr_values) ) {
    global $prefs;
    $out = array();

    // if there is no custom_value coming from the article, let's use our default one
    if ( empty($custom_value) )
      $custom_value = $default_value;

    foreach ($arr_values as $key => $value) {
      $selected = glz_selected_checked('selected', $key, $custom_value);
      $out[] = "<option value=\"$key\"{$selected}>$value</option>";
    }

    // we'll need the extra attributes as well as a name that will produce an array
    if ($multi) {
      $multi = ' multiple="multiple" size="'.$prefs['multiselect_size'].'"';
      $name .= "[]";
    }

    return "<select id=\"".glz_idify($id)."\" name=\"$name\" class=\"list\"$multi>".
      ( $include_empty_option ? "<option value=\"\">&nbsp;</option>" : '' ).
      ( $out ? join('', $out) : '').
      "</select>";
  }
  else
    return glz_custom_fields_gTxt('field_problems', array('{custom_set_name}' => $name));
}

This version does provide a way to enable/disable the first empty option for single/multi-selects created via custom script. For those single/multi-select created via the "Extensions -> Custom Fields" tab, it will default to enable the first empty option. Ideally, at least for single selects, the "Add new custom field" form in the "Extensions -> Custom Fields" tab should provide an extra radio button to enable/disable the first empty option (in other words, a way to set $include_empty_option' totrueorfalse`).

What do you think?