Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

updated form_builder to adhere to the new formtastic spec (Issue #1085) #1140

Closed
wants to merge 1 commit into from

3 participants

@HectorMalot

This relates to issue #1085. Updates the code so that:

  1. New methods (#actions, #action, #commit_action_with_cancel_link
  2. All calls to #buttons, #commit_button, and #commit_button_with_cancel_link will receive a depreciation warning and then call their new corresponding method as an action. In the case of #commit_button, some parsing is done to translate the string that can be passed to #commit_button as part of the option has passed to #action(:submit).
  3. Small update to CSS that makes .action equivalent to .button

This does assume that active_admin eventually wants to change the syntax to f.actions etc together with formtastic, and depreciate the f.buttons syntax. Something that might need some more discussion?

@latortuga

I'm not sure we need additional deprecation warnings as formtastic already prints them to the console. Thoughts?

@HectorMalot

I'll be the first to say that I don't know for sure what the best path forward is for this issue. I'll try to explain my choices in a bit more depth:

  1. Forwarding calls from buttons to actions:
    If I understand the depreciation message from formtastic correctly, it means that when formtastic moves to 2.2 (or 3.0), it will break all active_admin setups that still use f.buttons. In my opinion there are 2 options to keep things working:

    • forwarding the calls in active_admin to the right method (as shown here, better backwards compatibility),
    • or leave the current methods as is, and urge everybody to upgrade. (As in pull request #1139, more code clarity)

    I really don't know which one to choose over another. Keeping old code compatible with newer versions of formtastic and active_admin seemed as a nice advantage to me at the time. It resembles the way Capistrano moved the deploy:symlink task to deploy:create_symlink task.

  2. Additional depreciation warnings

    This is a direct result of the forwarding of the button methods to the action methods. Because the f.buttons method of formtastic doesn't get called anymore, the formtastic depreciation warning will not show anymore. I added the depreciation warnings to make clear that f.actions is preferred over f.buttons.

@gregbell gregbell was assigned
@gregbell gregbell referenced this pull request from a commit
@gregbell gregbell Added back in the deprecated formtastic methods for easier transition
Thanks to @HectorMalot for most of the code from PR #1140
8f5418e
@gregbell
Owner

Merged into master

@gregbell gregbell closed this
@moorecp moorecp referenced this pull request from a commit in moorecp/active_admin
@gregbell gregbell Added back in the deprecated formtastic methods for easier transition
Thanks to @HectorMalot for most of the code from PR #1140
52bfa30
@makv makv referenced this pull request from a commit in makv/active_admin
@gregbell gregbell Added back in the deprecated formtastic methods for easier transition
Thanks to @HectorMalot for most of the code from PR #1140
be52a52
@parndt parndt referenced this pull request from a commit in resolve/active_admin
@gregbell gregbell Added back in the deprecated formtastic methods for easier transition
Thanks to @HectorMalot for most of the code from PR #1140
8821fe4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
4 app/assets/stylesheets/active_admin/_forms.css.scss
@@ -165,12 +165,12 @@ form {
}
- .buttons {
+ .buttons, .actions {
margin-top: 15px;
input[type=submit] { margin-right: 10px; }
}
- fieldset.buttons li {
+ fieldset.buttons li, fieldset.actions li {
float:left;
padding: 0;
View
37 lib/active_admin/form_builder.rb
@@ -26,15 +26,19 @@ def input(method, *args)
# The buttons method always needs to be wrapped in a new buffer
def buttons(*args, &block)
- content = with_new_form_buffer do
- block_given? ? super : super { commit_button_with_cancel_link }
- end
- form_buffers.last << content.html_safe
+ # Formtastic has depreciated #buttons in favor of #actions
+ ::ActiveSupport::Deprecation.warn("f.buttons is deprecated in favour of f.actions")
+ actions args, &block
end
def commit_button(*args)
- content = with_new_form_buffer{ super }
- form_buffers.last << content.html_safe
+ # Formtastic has depreciated #commit_button in favor of #action(:submit)
+ ::ActiveSupport::Deprecation.warn("f.commit_button is deprecated in favour of f.action(:submit)")
+ options = args.extract_options!
+ if String === args.first
+ options[:label] = args.first unless options.has_key?(:label)
+ end
+ action(:submit, options)
end
def cancel_link(url = nil, html_options = {}, li_attributes = {})
@@ -44,7 +48,26 @@ def cancel_link(url = nil, html_options = {}, li_attributes = {})
end
def commit_button_with_cancel_link
- content = commit_button
+ # Formtastic has depreciated #buttons in favor of #actions
+ ::ActiveSupport::Deprecation.warn("f.commit_button_with_cancel_link is deprecated in favour of f.commit_action_with_cancel_link")
+ commit_action_with_cancel_link
+ end
+
+ # The action methods always need to be wrapped in a new buffer
+ def actions(*args, &block)
+ content = with_new_form_buffer do
+ block_given? ? super : super { commit_action_with_cancel_link }
+ end
+ form_buffers.last << content.html_safe
+ end
+
+ def action(*args)
+ content = with_new_form_buffer { super }
+ form_buffers.last << content.html_safe
+ end
+
+ def commit_action_with_cancel_link
+ content = action(:submit)
content << cancel_link
end
View
58 spec/unit/form_builder_spec.rb
@@ -120,6 +120,64 @@ def build_form(options = {}, &block)
end
end
+
+ context "while transitioning from buttons to actions" do
+ it "should forward #buttons to #actions" do
+ body = build_form do |f|
+ f.should_receive(:actions).once
+ f.buttons
+ end
+ end
+ it "should forward #commit_button to #action(:submit)" do
+ body = build_form do |f|
+ f.should_receive(:action) do |arg1,arg2|
+ arg1.should == :submit
+ arg2[:label].should == "Create & Continue"
+ end
+ f.actions do
+ f.commit_button "Create & Continue"
+ end
+ end
+ end
+ it "should preserve a label over a string" do
+ body = build_form do |f|
+ f.actions do
+ f.commit_button "Not Valid", :label => "Valid"
+ end
+ end
+ body.should have_tag("input", :attributes => { :type => "submit",
+ :value => "Valid" })
+ end
+ end
+
+ context "with actions instead of buttons" do
+ it "should generate the form once" do
+ body = build_form do |f|
+ f.inputs do
+ f.input :title
+ end
+ f.actions
+ end
+ body.scan(/id=\"post_title\"/).size.should == 1
+ end
+ it "should generate one button and a cancel link" do
+ body = build_form do |f|
+ f.actions
+ end
+ body.scan(/type=\"submit\"/).size.should == 1
+ body.scan(/class=\"cancel\"/).size.should == 1
+ end
+ it "should generate multiple buttons" do
+ body = build_form do |f|
+ f.actions do
+ f.action :submit, :label => "Create & Continue"
+ f.action :submit, :label => "Create & Edit"
+ end
+ end
+ body.scan(/type=\"submit\"/).size.should == 2
+ body.scan(/class=\"cancel\"/).size.should == 0
+ end
+ end
context "without passing a block to inputs" do
let :body do
Something went wrong with that request. Please try again.