Add 'mixed_options_for_select' helper to generate combined bare option and optgroup tags from nested array #7889

Closed
wants to merge 1 commit into
from

Projects

None yet

4 participants

@chatura-atapattu
mixed_options_for_select([["Pop"], ["Rock", [["Alternative Rock"], ["Hard Rock"]]], ["Country"]])
<option value="Pop">Pop</option>
<optgroup label="Rock">
  <option value="Alternative Rock">Alternative Rock</option>
  <option value="Hard Rock">Hard Rock</option>
</optgroup>
<option value="Country">Country</option>

Currently not possible.

@robin850
Member

Hi,

could you squash your commits into a single one (with git rebase) and use Ruby 1.9 hash syntax please ? :)

@chatura-atapattu

Alright, should be good now. Let me know if anything else needs changing.

@robin850
Member

I allow myself to ping @rafaelfranca, I don't who works on AP, sorry.

@al2o3cr
al2o3cr commented Oct 10, 2012

This has been suggested before, but the counterargument was that the API was already complicated enough without another select options helper:

https://rails.lighthouseapp.com/projects/8994/tickets/1986-create-optgroups-in-options_for_select

@chatura-atapattu

Thats fair, I agree that there is a lot going on with regards to options helpers. I saw that old ticket and have no problem incorporating my changes into grouped_options_for_select.

On the flip-side, the mixed_options_for_select method would take any array (designed with just options for options_for_select, or optgroups for grouped_options_for_select) and return the right output.

Either way, I support simplifying/consolidating the option helpers here and cleaning up/optimizing the code. Also, as a relatively new developer to RoR, I'll accept any help/advice/suggestions.

@rafaelfranca
Member

Thank you for the pull request but I'm not sure about this feature. You can archive the same doing:

options_for_select(["Pop"]), + grouped_options_from_select(["Rock", [["Alternative Rock"], ["Hard Rock"]]]) + options_for_select(["Country"])

I don't think this feature would be a good addition to the Rails project.

(@robin850 generally @carlosantoniodasilva and I are the guys who work in the Action View Form Helpers, you can ping us)

@chatura-atapattu

@rafaelfranca :

I see your point and like @al2o3cr actually agree that things should be uncomplicated. While

options_for_select(["Pop"]) + grouped_options_for_select([["Rock", [["Alternative Rock", {:class => "rock"}], ["Hard Rock"]]]]) + options_for_select(["Country"])

will generate,

<option value="Pop">Pop</option>
<optgroup label="Rock">
    <option value="Alternative Rock" class="rock">Alternative Rock</option>
    <option value="Hard Rock">Hard Rock</option>
</optgroup>
<option value="Country">Country</option>

grouped_options_for_select does not support adding any html attributes on the optgroup level such as,

grouped_options_for_select([["Rock", { :class => "rock" }, [["Alternative Rock"], ["Hard Rock"]]]])

For reference, that produces:

<optgroup label=\"Rock\">
    <option value=\"rock\">class</option>
</optgroup>

And attempting

grouped_options_for_select([["Rock", [["Alternative Rock"], ["Hard Rock"]], { :class => "rock" }]])

just ignores the attempt to add an attribute to the optgroup.

<optgroup label=\"Rock\">
    <option value=\"Alternative Rock\">Alternative Rock</option>
    <option value=\"Hard Rock\">Hard Rock</option>
</optgroup>

Furthermore, shouldn't you be able to feed in a structured array with hashes containing the html attributes and get out a structured list, regardless of whether there are bare options or optgroups?

I agree that another function is unnecessary and excessive. I'm starting to think generation of the options/optgroups should be consolidated into one function that does everything (bare options, optgroups, html attributes, arrays, hashes, etc.) and existing functions treated as wrappers (for backwards compatibility to be depreciated in the future).

Thoughts?

@robin850
Member

(@rafaelfranca thanks for the tip ;) )

@rafaelfranca
Member

I don't think is worth add this kind of logic in the core. I would create a custom helper that generates exactly what I want in my application.

Something like:

def music_style_options
  content_tag(:optgroup, label: "Rock", class: "rock") do
      content_tag(:option, "Alternative Rock", value: "Alternative Rock")
  end
end

You can make it generic too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment