Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Refactor select helper and remove eval #4802

Merged
merged 2 commits into from

2 participants

@carlosantoniodasilva

Some refactor on select/collection_select helpers. Also removes eval calls from grouped options helper.

@josevalim josevalim merged commit 40c287c into rails:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
6 actionpack/lib/action_view/helpers/form_options_helper.rb
@@ -334,7 +334,7 @@ def options_for_select(container, selected = nil)
end.join("\n").html_safe
end
- # Returns a string of option tags that have been compiled by iterating over the +collection+ and assigning
+ # Returns a string of option tags that have been compiled by iterating over the +collection+ and assigning
# the result of a call to the +value_method+ as the option value and the +text_method+ as the option text.
# Example:
# options_from_collection_for_select(@people, 'id', 'name')
@@ -418,9 +418,9 @@ def options_from_collection_for_select(collection, value_method, text_method, se
# wrap the output in an appropriate <tt><select></tt> tag.
def option_groups_from_collection_for_select(collection, group_method, group_label_method, option_key_method, option_value_method, selected_key = nil)
collection.map do |group|
- group_label_string = eval("group.#{group_label_method}")
+ group_label_string = group.send(group_label_method)
"<optgroup label=\"#{ERB::Util.html_escape(group_label_string)}\">" +
- options_from_collection_for_select(eval("group.#{group_method}"), option_key_method, option_value_method, selected_key) +
+ options_from_collection_for_select(group.send(group_method), option_key_method, option_value_method, selected_key) +
'</optgroup>'
end.join.html_safe
end
View
9 actionpack/lib/action_view/helpers/tags/collection_select.rb
@@ -12,9 +12,14 @@ def initialize(object_name, method_name, template_object, collection, value_meth
end
def render
- selected_value = @options.has_key?(:selected) ? @options[:selected] : value(@object)
+ option_tags_options = {
+ :selected => @options.fetch(:selected) { value(@object) },
+ :disabled => @options[:disabled]
+ }
+
select_content_tag(
- options_from_collection_for_select(@collection, @value_method, @text_method, :selected => selected_value, :disabled => @options[:disabled]), @options, @html_options
+ options_from_collection_for_select(@collection, @value_method, @text_method, option_tags_options),
+ @options, @html_options
)
end
end
View
27 actionpack/lib/action_view/helpers/tags/select.rb
@@ -11,21 +11,30 @@ def initialize(object_name, method_name, template_object, choices, options, html
end
def render
- selected_value = @options.has_key?(:selected) ? @options[:selected] : value(@object)
+ option_tags_options = {
+ :selected => @options.fetch(:selected) { value(@object) },
+ :disabled => @options[:disabled]
+ }
- # Grouped choices look like this:
- #
- # [nil, []]
- # { nil => [] }
- #
- if !@choices.empty? && @choices.first.respond_to?(:last) && Array === @choices.first.last
- option_tags = grouped_options_for_select(@choices, :selected => selected_value, :disabled => @options[:disabled])
+ option_tags = if grouped_choices?
+ grouped_options_for_select(@choices, option_tags_options)
else
- option_tags = options_for_select(@choices, :selected => selected_value, :disabled => @options[:disabled])
+ options_for_select(@choices, option_tags_options)
end
select_content_tag(option_tags, @options, @html_options)
end
+
+ private
+
+ # Grouped choices look like this:
+ #
+ # [nil, []]
+ # { nil => [] }
+ #
+ def grouped_choices?
+ !@choices.empty? && @choices.first.respond_to?(:last) && Array === @choices.first.last
+ end
end
end
end
Something went wrong with that request. Please try again.