Closes #1882 Fix ambiguous selected items in "select_tag"

padrino · Oct 11, 2015 · 49f4e90 · 49f4e90
1 parent 8ab29c0
commit 49f4e90
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 4 deletions.
diff --git a/CHANGES.rdoc b/CHANGES.rdoc
@@ -1,9 +1,10 @@
 = CHANGES
 
-== 0.13.0 (October 11th 2015)
+== 0.13.0 (Unreleased)
 
 - FIX #1614 Do not search for caller in bundler lib (@ujifgc)
 - FIX #1796 Save and restore layout setting (@ujifgc)
+- FIX #1882 Fix ambiguous selected items in "select_tag" (@nesquena)
 - FIX #1965 Fail properly on wrong mailer name or message (@ujifgc)
 - FIX #1959 simplify the mounter class (@dnesteryuk)
 - FIX #1916 Check key existence on caching (@namusyaka)

diff --git a/padrino-helpers/lib/padrino-helpers/form_helpers/options.rb b/padrino-helpers/lib/padrino-helpers/form_helpers/options.rb
@@ -45,7 +45,9 @@ def blank_option(prompt)
         #
         def option_is_selected?(value, caption, selected_values)
           Array(selected_values).any? do |selected|
-            [value.to_s, caption.to_s].include?(selected.to_s)
+            value.to_s ?
+              value.to_s == selected.to_s :
+              caption.to_s == selected.to_s
           end
         end
 
@@ -90,7 +92,7 @@ def extract_option_items!(options)
             collection.map{ |item| [ item.send(fields.first), item.send(fields.last) ] }
           else
             options.delete(:options) || []
-          end        
+          end
         end
       end
     end

diff --git a/padrino-helpers/test/test_form_helpers.rb b/padrino-helpers/test/test_form_helpers.rb
@@ -773,6 +773,20 @@ def protect_from_csrf; false; end
       assert_has_tag('select option', :content => 'Black', :value => 'black1') { actual_html }
     end
 
+    it 'should display selected options first based on values not content' do
+      options = [['First', 'one'], ['one', 'two'], ['three', 'three']]
+      actual_html = select_tag(:number, :options => options, :selected => 'one')
+      assert_has_tag('select option', :selected => 'selected', :count => 1) { actual_html }
+      assert_has_tag('select option', :content => 'First', :value => 'one', :selected => 'selected') { actual_html }
+    end
+
+    it 'should display selected options falling back to checking content' do
+      options = [['one'], ['two'], ['three', 'three']]
+      actual_html = select_tag(:number, :options => options, :selected => 'one')
+      assert_has_tag('select option', :selected => 'selected', :count => 1) { actual_html }
+      assert_has_tag('select option', :content => 'one', :value => 'one', :selected => 'selected') { actual_html }
+    end
+
     it 'should display options with values and accept disabled options' do
       options = [['Green', 'green1', {:disabled => true}], ['Blue', 'blue1'], ['Black', "black1"]]
       actual_html = select_tag(:favorite_color, :options => options)
@@ -785,7 +799,7 @@ def protect_from_csrf; false; end
 
     it 'should display option with values and multiple selected' do
       options = [['Green', 'green1'], ['Blue', 'blue1'], ['Black', "black1"]]
-      actual_html = select_tag(:favorite_color, :options => options, :selected => ['green1', 'Black'])
+      actual_html = select_tag(:favorite_color, :options => options, :selected => ['green1', 'black1'])
       assert_has_tag(:select, :name => 'favorite_color') { actual_html }
       assert_has_tag('select option', :selected => 'selected', :count => 2) { actual_html }
       assert_has_tag('select option', :content => 'Green', :value => 'green1', :selected => 'selected') { actual_html }