Ruby on Rails

Comments and changes to this ticket

• 

  Mislav September 5th, 2008 @ 09:33 PM

  +1

  I would like to see better naming:

  
  groups = ['North America','Europe']
  options = [, ]
  grouped_options_for_select(groups, options)
  
  # "option_groups_for_select" is also a good method name
  

  And how about this:

  
  grouped_options_for_select(
    'North America' => ['United States','Canada'],
    'Europe' => ['Denmark','Germany','France']
  )
  # => iterates over keys in alphabetical order ("Europe" would come first)
  

• 

  Mislav September 5th, 2008 @ 09:34 PM

  I ruined the 2nd line of my example by using cut instead of copy :/

• 

  Scott Raymond September 5th, 2008 @ 10:09 PM

  +1

• 

  Jon Crawford September 5th, 2008 @ 10:22 PM

  I replied to some of Mislav's thoughts in the IRC channel but I'll repeat here.

  I avoided the options argument name because it is often used in many other helpers to mean an option set of arguments that may be passed. And I used the labels argument name because of the attribute in <optgroup label="Europe''>. I could easily budge on the labels one, but I personally think the options should be avoided. Note: options_for_select uses container.

  And to your point about using a hash and iterating in alphabetical order: In my very first implementation of this helper, I needed the groups in a specific order ("Mens, Womens, Children, Other"). Using alphabetical sorting means you lose a good deal of control (although it's much cleaner on first blush).

• 

  Yaroslav Markin September 6th, 2008 @ 04:10 PM

  +1

• 

  Eloy Duran September 6th, 2008 @ 04:18 PM

  +1

• 

  Mislav September 8th, 2008 @ 01:08 AM

  Jon,

  Re: hash argument and alphabetical order---I've only suggested that this could be an alternative to passing 2 arrays. Array arguments should be used when the user is in need of custom ordering.

• 

  Tom Lea September 8th, 2008 @ 09:55 AM

  The two arrays required to be equal length is a bit of a no no for me, how about accepting:

  
  labels = ['North America',]
  groups = [
    ['North America',
      ['United States','Canada']],
    ['Europe',
      ['Denmark','Germany','France']]
  ]
  
  grouped_options_for_select(labels,groups)
  

  This accepts things in order and will keep them in order, ala Jon's requirements.

  Code designed for this should be able to handle a hash too ([[:a, :b]].each{|c| break c} == {:a => :b}.each{|c| break c}), as per Mislav's ideas.

  This also avoids the slightly icky Error: Sizes don't match, as this is now an impossible state to reach.

  There is no basic issue with the idea, it's good, but needs some more work on the method signature, just does not feel railsy enough.

  Any chance of an updated patch Jon?

• 

  Tom Lea September 8th, 2008 @ 09:57 AM

  1's example is wrong... should be

  
  grouped_options = [
    ['North America',
      ['United States','Canada']],
    ['Europe',
      ['Denmark','Germany','France']]
  ]
  
  grouped_options_for_select(grouped_options)
  

• 

  Jon Crawford September 8th, 2008 @ 05:13 PM

  Tom-

  I made the changes you suggested. I never liked that error at the beginning, but I didn't want to raise an exception either. Maybe I'm too nice? :-D

  It now accepts either a nested Array or a Hash.

  I also changed the helper name to grouped_options_for_select.

  Let me know if you guys have any other suggestions.

  • optgroups_with_options_for_select.diff 15.8 KB
• 

  Jon Crawford September 9th, 2008 @ 01:52 AM

  • → Title changed from “[PATCH] Added optgroups_with_options_for_select helper to form_options_helper for wrapping option tags in optgroups.” to “[PATCH] Added grouped_options_for_select helper to form_options_helper for wrapping option tags in optgroups.”

  Further updated patch including better testing and improved documentation.

  • optgroups_with_options_for_select.diff 19.8 KB
• 

  grosser September 17th, 2008 @ 12:11 PM

  • → Tag changed from “actionpack enhancement helper patch tested” to “actionpack enhancement helper patch tested”

  What i am using atm:

  continents = [['Africa',['Norway','NO']]] select_tag('country',option_groups_for_select(continents,'NO'))

  #continentes = [['Africa',['NA','Namibia'],['XX']...],...] def option_groups_for_select(collection,selected = nil)

  collection.inject('') do |options_for_select,group|
    group_label_string = group[0]
    options_for_select = "<optgroup label=\"#{html_escape(group_label_string)}\">"
    options_for_select += options_for_select(group[1..-1],selected)
    options_for_select += '</optgroup>'
  end
  
  

  end

• 

  grosser September 17th, 2008 @ 12:12 PM

  once again(weird formattting)...

  #continentes = [['Africa',['NA','Namibia'],['XX']...],...]
  def option_groups_for_select(collection,selected = nil)
    collection.inject('') do |options_for_select,group|
      group_label_string = group[0]
      options_for_select = "<optgroup label=\"#{html_escape(group_label_string)}\">"
      options_for_select += options_for_select(group[1..-1],selected)
      options_for_select += '</optgroup>'
    end
  end
  
  

• 

  Sean Cribbs October 3rd, 2008 @ 09:10 PM

  +1

• 

  grosser.michael+lighthouse (at gmail) October 4th, 2008 @ 08:02 AM

  There was a small errror in my above post:

  options_for_select = "<optgroup label=\"#{html_escape(group_label_string)}\">"
  
  

  should be

  options_for_select += "<optgroup label=\"#{html_escape(group_label_string)}\">"