141 7 true 2008-08-15T00:07:29+01:00 1741 9903 837 patch-specify-the-disabled-attribute-for-option-tags 294 8994 committed actionpack enhancement helper patch tested 2009-02-28T15:27:39+00:00 1741 Tekin Tekin Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x Adds the ability to specify the disabled attribute when generating option tags with the form helpers. Adds the ability to specify the disabled attribute when generating option tags with the form helpers. <div><p>Adds the ability to specify the disabled attribute when generating option tags with the form helpers.</p></div> 1 Adds the ability to specify the disabled attribute when generating option tags with the form helpers. <div><p>Adds the ability to specify the disabled attribute when generating option tags with the form helpers.</p></div> false 2008-08-15T00:07:29+01:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2008-08-15T00:07:29+01:00 1741 Tekin Tekin http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 2 Here's an improved patch with added documentation. <div><p>Here's an improved patch with added documentation.</p></div> false 2008-08-16T11:38:52+01:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2008-08-16T11:38:52+01:00 1741 Tekin Tekin http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 2 false 2008-08-21T16:43:48+01:00 1741 --- :title: "PATCH: specify the disabled attribute for option tags" 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2008-08-21T16:43:48+01:00 1366 Pratik Tekin http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 0 Up to date patch with support added to select and collection_select helpers. <div><p>Up to date patch with support added to select and collection_select helpers.</p></div> false 2008-11-08T17:06:24+00:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2008-11-08T17:06:29+00:00 1741 Tekin Tekin http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 1 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. <div><p>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.</p></div> false 2008-11-10T20:05:40+00:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2008-11-10T20:05:43+00:00 141 Michael Koziarski Tekin http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 1 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. <div><p>Thanks Michael.</p> <p>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.</p></div> false 2008-11-10T22:50:51+00:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2008-11-10T22:50:57+00:00 1741 Tekin Tekin http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 1 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. <div><p>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).</p> <p>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.</p> <p>Thank you.</p></div> false 2008-12-28T12:27:17+00:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2008-12-28T12:27:20+00:00 41991 patrick collins Tekin http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 2 Glad you found it useful. I've also turned the patch into a plugin: http://github.com/tekin/option_tags_will_disable/tree/master The test coverage is patchy but it's working for my needs until something can get worked into core. <div><p>Glad you found it useful. I've also turned the patch into a plugin:</p> <p><a href="http://github.com/tekin/option_tags_will_disable/tree/master">http://github.com/tekin/option_t...</a></p> <p>The test coverage is patchy but it's working for my needs until something can get worked into core.</p></div> false 2008-12-28T12:40:36+00:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2008-12-28T12:40:41+00:00 1741 Tekin Tekin http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 2 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 @@@ ruby 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 <div><p>How about if instead of passing :disable =&gt; [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)</p> <p>it might look a little like</p> <pre><code class="ruby"> f.select :foo_id, :foos, :disabled =&gt; lambda {|foo| some_condition(foo)} </code></pre> <p>Might be over the top but feels less cumbersome than having to explicitly write out those ids that are generated</p></div> false 2008-12-28T15:00:32+00:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2008-12-28T15:00:34+00:00 17477 Frederick Cheung Tekin http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 2 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: <div><p>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.</p> <p>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.</p> <p>Here is the improved code:</p></div> false 2008-12-28T22:30:07+00:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2008-12-28T22:30:09+00:00 41991 patrick collins Tekin http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 3 @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":http://api.rubyonrails.com/classes/ActionView/Helpers/FormOptionsHelper.html#M001408 <div><p>@Frederick - That's not a bad idea! Will work something up and make it work for :selected too.</p> <p>@Patrick - the :first/:last bit is necessary to support different types of container when calling options_for_select. see the <a href="http://api.rubyonrails.com/classes/ActionView/Helpers/FormOptionsHelper.html#M001408"> rdoc</a></p></div> false 2008-12-30T20:04:45+00:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2008-12-30T20:04:50+00:00 1741 Tekin Tekin http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 3 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. <div><p>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.</p></div> false 2009-01-02T16:25:21+00:00 1741 --- :title: Specify the disabled attribute for option tags 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2009-01-02T16:25:22+00:00 1741 Tekin Tekin http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 4 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: @@@ ruby 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. =) <div><p>It looks very nice!! But maybe instead of having two parameters (selected and disabled), we could pass a hash.</p> <p>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.</p> <p>And another tip is to refactor this little piece of code into a private method:</p> <pre><code class="ruby"> 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 </code></pre> <p>You wrote it twice (for selected and disabled) and they exactly the same. =)</p></div> false 2009-01-07T15:57:57+00:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2009-01-07T15:58:00+00:00 19965 José Valim Tekin http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 4 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? <div><p>Yeah, I'm ready to extract those two blocks of code but was waiting for some more feedback on the general principle first.</p> <p>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?</p> <p>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.</p> <p>Anyone else think this would be a better approach?</p></div> false 2009-01-07T21:42:19+00:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2009-01-07T21:42:21+00:00 1741 Tekin Tekin http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 2 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. <div><p>OK, I've reworked this patch, rebasing with edge and removing some of the duplication.</p> <p>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.</p> <p>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.</p></div> false 2009-01-29T11:54:49+00:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2009-01-29T11:54:51+00:00 1741 Tekin Tekin http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 3 Here's the second duck-typing method patch. Would be good to get this into 2.3, any suggestions on moving forward with this? <div><p>Here's the second duck-typing method patch.</p> <p>Would be good to get this into 2.3, any suggestions on moving forward with this?</p></div> false 2009-01-29T11:56:06+00:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2009-01-29T11:56:11+00:00 1741 Tekin Tekin http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 141 4 I like the look of this, but we'll need changes to the documentation too so people know how to use it :) <div><p>I like the look of this, but we'll need changes to the documentation too so people know how to use it :)</p></div> false 2009-02-01T01:54:36+00:00 1741 --- :assigned_user: 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2009-02-01T01:54:41+00:00 141 Michael Koziarski Tekin Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 141 4 OK, here's an updated patch with some no-frills documentation. Hopefully someone with better writing skills and docrails access will spruce them up! <div><p>OK, here's an updated patch with some no-frills documentation. Hopefully someone with better writing skills and docrails access will spruce them up!</p></div> false 2009-02-01T12:49:09+00:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2009-02-01T12:49:13+00:00 1741 Tekin Tekin Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 141 5 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. <div><p>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.</p></div> false 2009-02-09T14:36:19+00:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2009-02-09T14:36:20+00:00 1741 Tekin Tekin Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 141 5 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. <div><p>Tekin,</p> <p>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.</p> <p>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.</p></div> false 2009-02-13T09:25:51+00:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2009-02-13T09:25:52+00:00 141 Michael Koziarski Tekin Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 141 5 OK, new patch, rebased with edge, with separate commits to make things clearer. Much nicer! <div><p>OK, new patch, rebased with edge, with separate commits to make things clearer. Much nicer!</p></div> false 2009-02-14T01:02:48+00:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 new actionpack enhancement helper patch tested 2009-02-14T01:02:53+00:00 1741 Tekin Tekin Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 141 6 (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/commit/d676a7f18a5df86096f708052eb0c62ce4063310 <div><p>(from [d676a7f18a5df86096f708052eb0c62ce4063310]) Updated rdoc to reflect changes to form option helpers</p> <p>Signed-off-by: Michael Koziarski <a href="mailto:michael@koziarski.com">michael@koziarski.com</a> [<a href="/projects/8994/tickets/837" title="Ticket #837">#837</a> state:committed] <a href="http://github.com/rails/rails/commit/d676a7f18a5df86096f708052eb0c62ce4063310"> http://github.com/rails/rails/co...</a></p></div> true 2009-02-14T01:57:10+00:00 1741 --- :state: new 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 committed actionpack enhancement helper patch tested 2009-02-14T01:57:12+00:00 17393 Repository Tekin Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 141 6 Here's a patch for the changelog so it's not overlooked. <div><p>Here's a patch for the changelog so it's not overlooked.</p></div> true 2009-02-28T15:27:35+00:00 1741 --- {} 9903 837 patch-specify-the-disabled-attribute-for-option-tags 0 8994 committed actionpack enhancement helper patch tested 2009-02-28T15:27:39+00:00 1741 Tekin Tekin Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/837 2.x 2e88c7f25147a9c97492e40d415d2be016a163ee text/plain 2008-12-28T12:27:17+00:00 options_for_select_change.txt 74107 1701 41991 http://rails.lighthouseapp.com/attachments/74107/options_for_select_change.txt 160bee931122ab5a1a075e92f47c0117d3f85360 text/plain 2008-12-28T22:30:08+00:00 options_for_select_change.txt 74130 1098 41991 http://rails.lighthouseapp.com/attachments/74130/options_for_select_change.txt 55fc070cf8cba9166fa60b2660f61fed2e6e9656 text/plain 2009-01-29T11:54:49+00:00 enhance-option-tag-helpers-to-allow-disabled-with-extra-param.diff 82835 21289 1741 http://rails.lighthouseapp.com/attachments/82835/enhance-option-tag-helpers-to-allow-disabled-with-extra-param.diff 117b685445704f5e2a06ccaf9a05c1f305d3a732 text/plain 2009-01-29T11:56:06+00:00 enhance-option-tag-helpers-to-allow-disabled-with-duck-typing.diff 82836 20015 1741 http://rails.lighthouseapp.com/attachments/82836/enhance-option-tag-helpers-to-allow-disabled-with-duck-typing.diff a8fa827148ff233c560e684556e25f7b32f46a12 text/plain 2009-02-01T12:49:09+00:00 enhance-option-tag-helpers-to-allow-disabled-with-duck-typing.diff 83660 28415 1741 http://rails.lighthouseapp.com/attachments/83660/enhance-option-tag-helpers-to-allow-disabled-with-duck-typing.diff b01bfd9d620a07503dfbd0881488daaae994f377 text/plain 2009-02-14T01:02:48+00:00 enhance-option-tag-helpers.diff 88826 30017 1741 http://rails.lighthouseapp.com/attachments/88826/enhance-option-tag-helpers.diff a42a0c82fc1d65974e87e59f59a765bdd0c4585d text/plain 2009-02-28T15:27:35+00:00 changelog-update.diff 94290 936 1741 http://rails.lighthouseapp.com/attachments/94290/changelog-update.diff