Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Added :wrapper_html option functionality to commit_button().

Deprecated the more succinct, but ultimately more suprising functionality that allowed the :class option to be passed down to the button wrapper.
  • Loading branch information...
commit eed0029b03e47a47762c5bc7bfda0641c768876d 1 parent 4b1ba5f
Justin French authored
Showing with 79 additions and 2 deletions.
  1. +8 −2 lib/formtastic.rb
  2. +71 −0 spec/commit_button_spec.rb
10 lib/formtastic.rb
View
@@ -340,7 +340,9 @@ def button_field_set(*args, &block)
#
def commit_button(*args)
options = args.extract_options!
+ ::ActiveSupport::Deprecation.warn(":class => 'whatever' is deprecated on commit button, use :wrapper_html => { :class => 'whatever' } instead.", caller) if options.key?(:class)
text = options.delete(:label) || args.shift
+
if @object && (@object.respond_to?(:persisted?) || @object.respond_to?(:new_record?))
if @object.respond_to?(:persisted?) # ActiveModel
@@ -371,10 +373,14 @@ def commit_button(*args)
button_html = options.delete(:button_html) || {}
button_html.merge!(:class => [button_html[:class], key].compact.join(' '))
- element_class = ['commit', options.delete(:class)].compact.join(' ') # TODO: Add class reflecting on form action.
+
+ wrapper_html_class = ['commit', options.delete(:class)].compact # TODO: Add class reflecting on form action.
+ wrapper_html = options.delete(:wrapper_html) || {}
+ wrapper_html[:class] = (wrapper_html_class << wrapper_html[:class] << button_html[:class]).flatten.compact.join(' ')

Why is the button_html[:class] added to the wrapper_html[:class]? If I pass a class to the button, I don't want it to end up in the wrapper...

Justin French Owner

Good question, I'll have to look at the output and make sure I'm not missing something, but this definitely sounds like an error. Can you please create an issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
accesskey = (options.delete(:accesskey) || self.class.default_commit_button_accesskey) unless button_html.has_key?(:accesskey)
button_html = button_html.merge(:accesskey => accesskey) if accesskey
- template.content_tag(:li, Formtastic::Util.html_safe(self.submit(text, button_html)), :class => element_class)
+ template.content_tag(:li, Formtastic::Util.html_safe(self.submit(text, button_html)), wrapper_html)
end
# A thin wrapper around #fields_for to set :builder => Formtastic::SemanticFormBuilder
71 spec/commit_button_spec.rb
View
@@ -434,5 +434,76 @@ def self.human_name
end
end
+
+ describe ':wrapper_html option' do
+ describe 'when provided' do
+ it 'should be passed down to the li tag' do
+ form = semantic_form_for(@new_post) do |builder|
+ concat(builder.commit_button('text', :wrapper_html => {:id => :another_id}))
+ end
+ output_buffer.concat(form) if Formtastic::Util.rails3?
+ output_buffer.should have_tag("form li#another_id")
+ end
+
+ it 'should append given classes to li default classes' do
+ form = semantic_form_for(@new_post) do |builder|
+ concat(builder.commit_button('text', :wrapper_html => {:class => :another_class}))
+ end
+ output_buffer.concat(form) if Formtastic::Util.rails3?
+ output_buffer.should have_tag("form li.commit")
+ output_buffer.should have_tag("form li.another_class")
+ end
+
+ it 'should allow classes to be an array' do
+ form = semantic_form_for(@new_post) do |builder|
+ concat(builder.commit_button('text', :wrapper_html => {:class => [ :my_class, :another_class ]}))
+ end
+ output_buffer.concat(form) if Formtastic::Util.rails3?
+ output_buffer.should have_tag("form li.commit")
+ output_buffer.should have_tag("form li.my_class")
+ output_buffer.should have_tag("form li.another_class")
+ end
+
+ it 'should merge in classes applied using the :class option' do
+ with_deprecation_silenced do
+ form = semantic_form_for(@new_post) do |builder|
+ concat(builder.commit_button('text', :class => 'from_button_html', :wrapper_html => {:class => 'from_wrapper_html'}))
+ end
+ output_buffer.concat(form) if Formtastic::Util.rails3?
+ output_buffer.should have_tag("form li.commit")
+ output_buffer.should have_tag("form li.from_button_html")
+ output_buffer.should have_tag("form li.from_wrapper_html")
+ end
+ end
+
+ it 'should warn that :class is a deprecated option' do
+ with_deprecation_silenced do
+ ::ActiveSupport::Deprecation.should_receive(:warn).any_number_of_times
+ @form = semantic_form_for(@new_post) do |builder|
+ concat(builder.commit_button('text', :class => 'from_button_html', :wrapper_html => {:class => 'from_wrapper_html'}))
+ end
+ end
+ output_buffer.concat(@form) if Formtastic::Util.rails3?
+ output_buffer.should have_tag("form li.commit")
+ output_buffer.should have_tag("form li.from_button_html")
+ output_buffer.should have_tag("form li.from_wrapper_html")
+ end
+ end
+
+ describe 'when not provided' do
+ it 'should use default id and class' do
+ form = semantic_form_for(@new_post) do |builder|
+ concat(builder.commit_button('text'))
+ end
+ output_buffer.concat(form) if Formtastic::Util.rails3?
+ output_buffer.should have_tag("form li.commit")
+ end
+ end
+
+ end
+
+
+
+
end
Manuel Meurer

Why is the button_html[:class] added to the wrapper_html[:class]? If I pass a class to the button, I don't want it to end up in the wrapper...

Justin French

Good question, I'll have to look at the output and make sure I'm not missing something, but this definitely sounds like an error. Can you please create an issue?

Please sign in to comment.
Something went wrong with that request. Please try again.