Rails Ruby on Rails → 2.x

Comments and changes to this ticket

• You flagged this item as spam.
  

  Tekin August 16th, 2008 @ 11:38 AM

  Here's an improved patch with added documentation.

• 

  Pratik August 21st, 2008 @ 04:43 PM

  • Title changed from “PATCH: specify the disabled attribute for option tags” to “Specify the disabled attribute for option tags”

• You flagged this item as spam.
  

  Tekin November 8th, 2008 @ 05:06 PM

  Up to date patch with support added to select and collection_select helpers.

• 

  Michael Koziarski November 10th, 2008 @ 08:05 PM

  Not sure if I like the hash with disabled values, but I don't have a better suggestion either. Let's park this till post 2.2 and think a bit more about the API after that.

• You flagged this item as spam.
  

  Tekin November 10th, 2008 @ 10:50 PM

  Thanks Michael.

  Yeah, I'm not entirely happy with the API either but not sure what the alternative would be without drastic changes. In the mean time, I'll create a plugin and run it through it's paces.

• You flagged this item as spam.
  

  patrick collins December 28th, 2008 @ 12:27 PM

  I too have been wanting the ability to pass in disabled as an parameter for options_for_select. However, I needed more control than the disabled = {} offered, since I have helper methods that generate the label/value pairs for options_for_select. I needed the ability to have my helper method determine which options would be disabled, so that I wouldn't have to manually go in and specify various values as disabled (which could change and end up giving me more work to do).

  So... I modified the patch that was posted and tested it out, and it seems to work great for what I needed.. I thought I'd post it back in case anyone else finds it useful.

  Thank you.

  • options_for_select_change.txt 1.7 KB

• You flagged this item as spam.
  

  Tekin December 28th, 2008 @ 12:40 PM

  Glad you found it useful. I've also turned the patch into a plugin:

  http://github.com/tekin/option_t...

  The test coverage is patchy but it's working for my needs until something can get worked into core.

• 

  Frederick Cheung December 28th, 2008 @ 03:00 PM

  How about if instead of passing :disable => [1,2,3] you passed a proc which would return whether an item was disabled or not (so a bit closer in spirit to what options_from_collection_for_select does for id/value)

  it might look a little like

  
  f.select :foo_id, :foos, :disabled => lambda {|foo| some_condition(foo)}
  

  Might be over the top but feels less cumbersome than having to explicitly write out those ids that are generated

• You flagged this item as spam.
  

  patrick collins December 28th, 2008 @ 10:30 PM

  Well, I realized something still wasn't right with the behavior, and it got me to examine the original code for option_text_and_value. It originally had this respond_to?(:first) and respont_to?(:last) --- which I don't quite understand what the relevance of that was, as that seems to return true in most all cases.

  Anyway, I found that I was getting an error when passing in option, values without a disabled attribute, and it was because the option_text_and_value's passed in option param was a number 5.. Which I still don't get why that was happening. Anyway, I changed things and tested it all out, and it works great (for me atleast).. Like I said, this is ideal because you don't have to manually manage the disabled in the options_for_select, but rather you can have the disabled options be part of the array passed into it.

  Here is the improved code:

  • options_for_select_change.txt 1.1 KB

• You flagged this item as spam.
  

  Tekin December 30th, 2008 @ 08:04 PM

  @Frederick - That's not a bad idea! Will work something up and make it work for :selected too.

  @Patrick - the :first/:last bit is necessary to support different types of container when calling options_for_select. see the rdoc

• You flagged this item as spam.
  

  Tekin January 2nd, 2009 @ 04:25 PM

  • Title changed from “Specify the disabled attribute for option tags” to “disabled attribute and lambdas for option tags”

  OK, here's an improved patch with more tests and the added bonus of being able to use a Proc to identify selected and disabled option tags.

• 

  José Valim January 7th, 2009 @ 03:57 PM

  It looks very nice!! But maybe instead of having two parameters (selected and disabled), we could pass a hash.

  Rails 2.2 (or it's 2.3?) changed quite some ActionView helpers to work with hashes instead of multiple arguments because the first is much more readable.

  And another tip is to refactor this little piece of code into a private method:

  
    if selected.is_a?(Proc)
      selected_values = collection.map do |element|
        element.send(value_method) if selected.call(element) 
      end.compact
    else
      selected_values = selected
    end
  

  You wrote it twice (for selected and disabled) and they exactly the same. =)

• You flagged this item as spam.
  

  Tekin January 7th, 2009 @ 09:42 PM

  Yeah, I'm ready to extract those two blocks of code but was waiting for some more feedback on the general principle first.

  With regard to passing a hash instead of the extra parameter, do you mean repurpose the selected parameter so that it can be a selected value, an array of selected values, or a hash for both selected and disabled values?

  I don't know if that feels right... Then again, it doesn't feel quite right adding the extra optional parameter to those methods either.

  Anyone else think this would be a better approach?

• You flagged this item as spam.
  

  Tekin January 29th, 2009 @ 11:54 AM

  OK, I've reworked this patch, rebasing with edge and removing some of the duplication.

  I've also worked up an alternative which doesn't change the API and instead duck-types the selected parameter so that you can pass in a hash if you want to specify disabled option tags.

  Personally, I'm not convinced by the duck-typing, but it does have the advantage of not adding any more parameters (optional or otherwise) to the methods.

  • enhance-option-tag-helpers-to-allow-disabled-with-extra-param.diff 20.8 KB

• You flagged this item as spam.
  

  Tekin January 29th, 2009 @ 11:56 AM

  Here's the second duck-typing method patch.

  Would be good to get this into 2.3, any suggestions on moving forward with this?

  • enhance-option-tag-helpers-to-allow-disabled-with-duck-typing.diff 19.5 KB
• 

  Michael Koziarski February 1st, 2009 @ 01:54 AM

  • Assigned user set to “Michael Koziarski”

  I like the look of this, but we'll need changes to the documentation too so people know how to use it :)

• You flagged this item as spam.
  

  Tekin February 1st, 2009 @ 12:49 PM

  OK, here's an updated patch with some no-frills documentation. Hopefully someone with better writing skills and docrails access will spruce them up!

  • enhance-option-tag-helpers-to-allow-disabled-with-duck-typing.diff 27.7 KB

• You flagged this item as spam.
  

  Tekin February 9th, 2009 @ 02:36 PM

  With 2.3 RC1 out, is this going to slip to the next release? Otherwise, let me know if there is anything else needed from my end to get this patch into core before 2.3 goes official.

• 

  Michael Koziarski February 13th, 2009 @ 09:25 AM

  Tekin,

  Sorry for the delay, this one fell of my radar. It doesn't apply cleanly at present, but if you can rebase it it looks good to me.

  Also, perhaps split the dummy_posts change into another changeset, took me a while to separate out the changes in the patch from the (nice) refactoring.

• You flagged this item as spam.
  

  Tekin February 14th, 2009 @ 01:02 AM

  OK, new patch, rebased with edge, with separate commits to make things clearer. Much nicer!

  • enhance-option-tag-helpers.diff 29.3 KB
• 

  Repository February 14th, 2009 @ 01:57 AM

  • State changed from “new” to “committed”

  (from [d676a7f18a5df86096f708052eb0c62ce4063310]) Updated rdoc to reflect changes to form option helpers

  Signed-off-by: Michael Koziarski michael@koziarski.com [#837 state:committed] http://github.com/rails/rails/co...

• You flagged this item as spam.
  

  Tekin February 28th, 2009 @ 03:27 PM

  Here's a patch for the changelog so it's not overlooked.

  • changelog-update.diff 936 Bytes