When passing in a collection of strings of the label_method should still be able to be applied. #32

Closed
jonnii opened this Issue Aug 11, 2009 · 8 comments

4 participants

@jonnii

For example when doing:

<%= form.input :gender, :as => :select, :collection => Profile::GENDERS, :label_method => :humanize %>

This won't work currently as in find_collection_for_column there's the optimisation:

  # Return if we have an Array of strings, fixnums or arrays
  return collection if collection.instance_of?(Array) &&
                       [Array, Fixnum, String, Symbol].include?(collection.first.class)

If you remove that it works as expected.

It might make more sense to check the existance of the label_method and the value_method and only collect if one is present.

@justinfrench

Good find! I think we should just apply the label method to each member of the collection. The default label method is to_s, which I'm pretty sure each of those classes respond to, even in Ruby 1.9.

@jonnii

Formtastic is pretty fab btw. Keep up the good work!

@josevalim

Calling :to_s will make other things break. For example:

form.input :grade, :as => :select, :collection => Exam::GRADES

Where:

Exam::GRADES #=> [1,2,3,4,5]

If we convert to string, the rails select method will do object.grade == "1", since grade is an integer, it won't be able to mark the proper field. I would recommend humanize the strings in place:

form.input :gender, :as => :select, :collection => Profile::GENDERS.map(&:humanize)
@justinfrench

Ok, very good point, but it's really confusing to have a :label_method that does nothing... I might see if there's a way to warn or raise when this option is used on these arbitrary collections.

Ultimately, I think the :label_method approach is flawed. Xavier Shay had a much nicer method which used a Proc, so this problem will "go away" when we move to that perhaps.

@grimen

I believe this solves the last "but", but maybe I missed something...?

collection.collect! { |v| (v.to_s =~ /^\d$/) ? v : v.to_s }

Note: Updated this, I was too sloppy last time.

@grimen

What's the status on this one? 8)

@justinfrench

I'm leaning towards the Proc idea... thinking ;)

@justinfrench

I'm closing this one with "won't fix". I can't settle on any one answer that I like. The Proc idea was recently implemented, so we have lots of choices. I'm not even sure I want :label_method to hang around long-term.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment