Clarify grouped_options_for_select method API, add changelog entry

Make the method API more clear by explicitly showing the expected arguments. This means that the options cannot be passed as second argument because we are not relying on extract_options! anymore, you are expected to give a selected key or `nil` if you want to pass options, as it is the last argument. Notice that this does not change the current method arguments contract available in 3.2, it just brings back the same functionality with the divider addition.
pinetops · May 19, 2012 · 0e207a4 · 0e207a4
1 parent 3e70706
commit 0e207a4
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 16 deletions.
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
@@ -1,5 +1,9 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Add `divider` option to `grouped_options_for_select` to generate a separator
+    `optgroup` automatically, and deprecate `prompt` as third argument, in favor
+    of using an options hash. *Nicholas Greenfield*
+
 *   Add `time_field` and `time_field_tag` helpers which render an `input[type="time"]` tag. *Alex Soulim*
 
 *   Removed old text_helper apis for highlight, excerpt and word_wrap *Jeremy Walker*

diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb
@@ -477,8 +477,8 @@ def option_groups_from_collection_for_select(collection, group_method, group_lab
       #
       # Sample usage (Hash):
       #   grouped_options = {
-      #    'North America' => [['United States','US'], 'Canada'],
-      #    'Europe' => ['Denmark','Germany','France']
+      #     'North America' => [['United States','US'], 'Canada'],
+      #     'Europe' => ['Denmark','Germany','France']
       #   }
       #   grouped_options_for_select(grouped_options)
       #
@@ -495,10 +495,10 @@ def option_groups_from_collection_for_select(collection, group_method, group_lab
       #
       # Sample usage (divider):
       #   grouped_options = [
-      #    [['United States','US'], 'Canada'],
-      #    ['Denmark','Germany','France']
+      #     [['United States','US'], 'Canada'],
+      #     ['Denmark','Germany','France']
       #   ]
-      #   grouped_options_for_select(grouped_options, divider: '---------')
+      #   grouped_options_for_select(grouped_options, nil, divider: '---------')
       #
       # Possible output:
       #   <optgroup label="---------">
@@ -513,15 +513,14 @@ def option_groups_from_collection_for_select(collection, group_method, group_lab
       #
       # <b>Note:</b> Only the <tt><optgroup></tt> and <tt><option></tt> tags are returned, so you still have to
       # wrap the output in an appropriate <tt><select></tt> tag.
-      def grouped_options_for_select(*args)
-        grouped_options = args.shift
-        options = args.extract_options!
-        selected_key = args.shift
-        if prompt = args.shift
-          ActiveSupport::Deprecation.warn 'Passing the prompt to grouped_options_for_select as an argument is deprecated. Please pass it in an options hash.'
-        else
-          prompt = options[:prompt]
+      def grouped_options_for_select(grouped_options, selected_key = nil, options = {})
+        if options.is_a?(Hash)
+          prompt  = options[:prompt]
           divider = options[:divider]
+        else
+          prompt  = options
+          options = {}
+          ActiveSupport::Deprecation.warn "Passing the prompt to grouped_options_for_select as an argument is deprecated. Please use an options hash like `{ prompt: #{prompt.inspect} }`."
         end
 
         body = "".html_safe
@@ -534,7 +533,7 @@ def grouped_options_for_select(*args)
 
         grouped_options.each do |container|
           if divider
-            label, container = divider, container
+            label = divider
           else
             label, container = container
           end

diff --git a/actionpack/test/template/form_options_helper_test.rb b/actionpack/test/template/form_options_helper_test.rb
@@ -300,12 +300,12 @@ def test_grouped_options_for_select_with_optional_divider
     assert_dom_equal(
       "<optgroup label=\"----------\"><option value=\"US\">US</option>\n<option value=\"Canada\">Canada</option></optgroup><optgroup label=\"----------\"><option value=\"GB\">GB</option>\n<option value=\"Germany\">Germany</option></optgroup>",
 
-      grouped_options_for_select([['US',"Canada"] , ["GB", "Germany"]], divider: "----------")
+      grouped_options_for_select([['US',"Canada"] , ["GB", "Germany"]], nil, divider: "----------")
     )
   end
 
   def test_grouped_options_for_select_with_selected_and_prompt_deprecated
-    assert_deprecated 'Passing the prompt to grouped_options_for_select as an argument is deprecated. Please pass it in an options hash.' do
+    assert_deprecated 'Passing the prompt to grouped_options_for_select as an argument is deprecated. Please use an options hash like `{ prompt: "Choose a product..." }`.' do
       assert_dom_equal(
         "<option value=\"\">Choose a product...</option><optgroup label=\"Hats\"><option value=\"Baseball Cap\">Baseball Cap</option>\n<option selected=\"selected\" value=\"Cowboy Hat\">Cowboy Hat</option></optgroup>",
         grouped_options_for_select([["Hats", ["Baseball Cap","Cowboy Hat"]]], "Cowboy Hat", "Choose a product...")