Hi there,
Nice mod. I'm integrating the FormAPI element_info type stuff. Pretty neat.

I think I found a bit of a bug though in 'select_or_other.module' line 199:

Existing...
<?php
  // Also add in a custom class for each.
  if (isset($element[$sub_element]['#attributes']['class'])) {
    $element[$sub_element]['#attributes'] .= array('class' => array('select-or-other-' . $sub_element));
  }
  else {
    $element[$sub_element]['#attributes'] = array('class' => array('select-or-other-' . $sub_element));
  }
?>
Should be...
<?php
  // Also add in a custom class for each.
  if (isset($element[$sub_element]['#attributes']['class'])) {
    $element[$sub_element]['#attributes']['class'][] = 'select-or-other-' . $sub_element;
  }
  else {
    $element[$sub_element]['#attributes'] = array('class' => array('select-or-other-' . $sub_element));
  }
?>

If I try to add attributes to the 'select' or 'other' form element I use the '#select_attributes' or '#other_attributes'. When this is specified the first conditions executes and you perform a string concat on arrays. I expect you meant to use += instead of .= .
Like the module and hope this helps.

John

Comments

danielb’s picture

danielb CreditAttribution: danielb commented

ooh yeah that's all wrong, it can be simplified a bit too I think

jonlhouse’s picture

jonlhouse CreditAttribution: jonlhouse commented

No worries.

If you are taking potential feature requests I have an idea:

Make an option to start with the "Other" field visible to start with. Right now you append "Other:" (or user-supplied label to the end of the options). Maybe you could optionally prepend "Other:" and make the textfield by default (e.g. "#other_default => TRUE|FALSE").

Right now I'm using the widget for a combo-box where I'm having the user input a numeric value. For mobile devices I'm using the select part as a short-cut to some pre-set numbers. Having an option to show the "other" textfield by default would eliminate the extra step of opening the drop-down, selecting "Other:" than entering a value in.

I totally understand your default ux since for things like taxonomy your way make more sense.

On a side note, I doubt most users are using this as developers in custom forms but you could improve your documentation about the Form API properties. I had to big through source to find #other_unknown_defaults and #other_deliminater as well as that #select_
and #other_
go to their respective form elements. However, I like the way you did that, I works great for me!

Thanks for the hard work,

danielb’s picture

danielb CreditAttribution: danielb commented

not even gonna read that mate

danielb’s picture

danielb CreditAttribution: danielb commented
Status: Active » Fixed

fixed in the repository, and will appear in the next development snapshot (check the date).

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

  • Commit 21a1a07 on 7.x-2.x, 7.x-3.x, 8.x-3.x by danielb: 
    Issue #1259272 by jonlhouse, danielb: CSS attribute fix.