Support grouped collection for collection_select
#41562
Conversation
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
🦄 |
jonathanhefner
force-pushed
the
collection_select-grouped-collection
branch
from
0787276
to
5467523
Compare
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
jonathanhefner
force-pushed
the
collection_select-grouped-collection
branch
from
5467523
to
483d5ed
Compare
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
This adds support for grouped collections to the `collection_select` helper, much like the `select` helper: ```ruby collection_select :country_id, @countries.group_by(&:continent_name), :id, :name ``` This differs from `grouped_collection_select` in that it does not require a secondary model to group options: ```ruby grouped_collection_select :country_id, @continents, :countries, :name, :id, :name ``` Of course, a secondary model can be used as well: ```ruby collection_select :country_id, @countries.group_by { |country| country.continent.name }, :id, :name ```
jonathanhefner
force-pushed
the
collection_select-grouped-collection
branch
from
483d5ed
to
400c1c3
Compare
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
jonathanhefner
changed the title
collection_select
| def collection_grouped? | ||
| entry = @collection.first if @collection.is_a?(Hash) || @collection.is_a?(Array) | ||
| entry.respond_to?(:last) && entry.last.is_a?(Array) | ||
| end |
There was a problem hiding this comment.
This is almost equivalent to the implementation for select:
rails/actionview/lib/action_view/helpers/tags/select.rb
Lines 36 to 38 in 436207a
What do you think of matching it exactly? My concern is consistent edge-case behaviour, e.g. passing a set of arrays of arrays will work for select, but won't work here.
There was a problem hiding this comment.
Sure! The reason I limited this behavior to Hash and Array was because I did not realize I could use blank? to ensure that an AR relation was loaded. I had assumed blank? would execute a limited query, just like empty?. It looks like this behavior was explicitly implemented in #5461 (and tests were added in #5469), so I will change this method to mirror Select. 👍
| option_tags_options = { | ||
| selected: @options.fetch(:selected) { value }, | ||
| disabled: @options[:disabled] | ||
| } |
There was a problem hiding this comment.
These lines are currently (almost) identical to their counterparts in the Select tag helper class:
rails/actionview/lib/action_view/helpers/tags/select.rb
Lines 17 to 20 in 436207a
I don't think we should refactor them, and hide the equivalence, without a good reason.
There was a problem hiding this comment.
Yes, I noticed the same pattern in GroupedCollectionSelect, too.
In #44335, I factored this out into a SelectRenderer module. (So Select would look like this, and CollectionSelect would match.) I was planning to merge that PR first, and then rebase this PR on top of it. Any objections?
This adds support for grouped collections to the
collection_selecthelper, much like theselecthelper:This differs from
grouped_collection_selectin that it does not require a secondary model to group options:Of course, a secondary model can be used as well:
This finishes #40338, which can't be reopened because
masterbranch has been deleted.