Permalink
Browse files

refactor options_for_select

  • Loading branch information...
1 parent 421025f commit decff3f43e5549cbc8b89f974460de3d71407042 @nashby nashby committed Feb 20, 2012
View
15 actionpack/lib/action_view/helpers/form_options_helper.rb
@@ -330,9 +330,12 @@ def options_for_select(container, selected = nil)
container.map do |element|
html_attributes = option_html_attributes(element)
text, value = option_text_and_value(element).map { |item| item.to_s }
- selected_attribute = ' selected="selected"' if option_value_selected?(value, selected)
- disabled_attribute = ' disabled="disabled"' if disabled && option_value_selected?(value, disabled)
- %(<option value="#{ERB::Util.html_escape(value)}"#{selected_attribute}#{disabled_attribute}#{html_attributes}>#{ERB::Util.html_escape(text)}</option>)
+
+ html_attributes[:selected] = 'selected' if option_value_selected?(value, selected)
+ html_attributes[:disabled] = 'disabled' if disabled && option_value_selected?(value, disabled)
+ html_attributes[:value] = value
+
+ content_tag(:option, text, html_attributes)
end.join("\n").html_safe
end
@@ -649,11 +652,9 @@ def collection_check_boxes(object, method, collection, value_method, text_method
private
def option_html_attributes(element)
- return "" unless Array === element
+ return {} unless Array === element
- element.select { |e| Hash === e }.reduce({}, :merge).map do |k, v|
- " #{k}=\"#{ERB::Util.html_escape(v.to_s)}\""
- end.join
+ Hash[element.select { |e| Hash === e }.reduce({}, :merge).map { |k, v| [k, ERB::Util.html_escape(v.to_s)] }]
end
def option_text_and_value(option)
View
32 actionpack/test/template/form_options_helper_test.rb
@@ -182,16 +182,16 @@ def test_array_options_for_string_include_in_other_string_bug_fix
def test_hash_options_for_select
assert_dom_equal(
- "<option value=\"&lt;Kroner&gt;\">&lt;DKR&gt;</option>\n<option value=\"Dollar\">$</option>",
- options_for_select("$" => "Dollar", "<DKR>" => "<Kroner>").split("\n").sort.join("\n")
+ "<option value=\"Dollar\">$</option>\n<option value=\"&lt;Kroner&gt;\">&lt;DKR&gt;</option>",
+ options_for_select("$" => "Dollar", "<DKR>" => "<Kroner>").split("\n").join("\n")
)
assert_dom_equal(
- "<option value=\"&lt;Kroner&gt;\">&lt;DKR&gt;</option>\n<option value=\"Dollar\" selected=\"selected\">$</option>",
- options_for_select({ "$" => "Dollar", "<DKR>" => "<Kroner>" }, "Dollar").split("\n").sort.join("\n")
+ "<option value=\"Dollar\" selected=\"selected\">$</option>\n<option value=\"&lt;Kroner&gt;\">&lt;DKR&gt;</option>",
+ options_for_select({ "$" => "Dollar", "<DKR>" => "<Kroner>" }, "Dollar").split("\n").join("\n")
)
assert_dom_equal(
- "<option value=\"&lt;Kroner&gt;\" selected=\"selected\">&lt;DKR&gt;</option>\n<option value=\"Dollar\" selected=\"selected\">$</option>",
- options_for_select({ "$" => "Dollar", "<DKR>" => "<Kroner>" }, [ "Dollar", "<Kroner>" ]).split("\n").sort.join("\n")
+ "<option value=\"Dollar\" selected=\"selected\">$</option>\n<option value=\"&lt;Kroner&gt;\" selected=\"selected\">&lt;DKR&gt;</option>",
+ options_for_select({ "$" => "Dollar", "<DKR>" => "<Kroner>" }, [ "Dollar", "<Kroner>" ]).split("\n").join("\n")
)
end
@@ -1056,36 +1056,36 @@ def test_options_for_select_with_element_attributes_and_selection_array
end
def test_option_html_attributes_from_without_hash
- assert_dom_equal(
- "",
+ assert_equal(
+ {},
option_html_attributes([ 'foo', 'bar' ])
)
end
def test_option_html_attributes_with_single_element_hash
- assert_dom_equal(
- " class=\"fancy\"",
+ assert_equal(
+ {:class => 'fancy'},
option_html_attributes([ 'foo', 'bar', { :class => 'fancy' } ])
)
end
def test_option_html_attributes_with_multiple_element_hash
- assert_dom_equal(
- " class=\"fancy\" onclick=\"alert('Hello World');\"",
+ assert_equal(
+ {:class => 'fancy', 'onclick' => "alert('Hello World');"},
option_html_attributes([ 'foo', 'bar', { :class => 'fancy', 'onclick' => "alert('Hello World');" } ])
)
end
def test_option_html_attributes_with_multiple_hashes
- assert_dom_equal(
- " class=\"fancy\" onclick=\"alert('Hello World');\"",
+ assert_equal(
+ {:class => 'fancy', 'onclick' => "alert('Hello World');"},
option_html_attributes([ 'foo', 'bar', { :class => 'fancy' }, { 'onclick' => "alert('Hello World');" } ])
)
end
def test_option_html_attributes_with_special_characters
- assert_dom_equal(
- " onclick=\"alert(&quot;&lt;code&gt;&quot;)\"",
+ assert_equal(
+ {:onclick => "alert(&quot;&lt;code&gt;&quot;)"},
option_html_attributes([ 'foo', 'bar', { :onclick => %(alert("<code>")) } ])
)
end

0 comments on commit decff3f

Please sign in to comment.