-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2 customisation options for has_many form #1945
Conversation
I'm confused, what are you trying to achieve with this? Could you provide before & after screenshots? |
inputs options, &form_block | ||
|
||
js = js_for_has_many(association, form_block, template) | ||
js = options[:new_record] == false ? "" : js_for_has_many(association, form_block, template) | ||
form_buffers.last << js.html_safe | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this is a better way to write the changes:
# at the top of the method definition:
options = { :for => association, :title => true, :new_record => true }.merge(options)
# then the actual code
if options[:title]
title = object.class.reflect_on_association(association).class_name.human(:count => 1.1)
form_buffers.last << template.content_tag(:h3, title)
end
inputs options, &form_block
form_buffers.last << js_for_has_many(association, form_block, template).html_safe if options[:new_record]
Note that I changed klass.model_name
to the more succinct class_name
... @gregbell, is it possible to use Arbre here? I hate to force the title
part to live on four lines instead of one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With your changes i got tons of errors in specs:
with class_name and i 've got errors for spec if we do not add anything to form_buffers.last
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really? class_name works fine for me.
The form_buffers.last problem is likely that with my changes, the function will return nil instead of the form buffer if new_record is false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
with class_name i've got ' undefined method `human' for "Post":String' in specs.
-
with form_buffers changes you advise - because of form_buffers.pop (why?) it is not rendering first element inside has_many block:
spec failed: Failure/Error: body.should have_tag("label", "Title 1")
By the way, you need to rebase on master. git pull --rebase upstream master Specifically, the |
@daxter i've updated my forked repo. |
#2068 was just merged in that added the title functionality. Could you rebase this to just include your new option to hide the "add" button? |
Sure, but... please, tell me how :) |
First, start by rebasing your feature branch off of the master branch from upstream (gregbell/active_admin).
If you don't have
Then resolve conflicts as they arise when your commits are re-applied as new ones on master, like you had just done the work today. You'll need to remove the code that is no longer needed, since master now has the functionality for specifying a title. When you're done, force push your feature branch and we'll see the changes in this PR.
In general, you should create a branch in your fork & submit that branch as a PR to master over here. That way you can pull new changes from our master back into your forked repo's master at any time in the future. |
Closing this due to inactivity. @developer88 if you still want to add in the option to disable the "add" button, please create a new pull request. |
Added :title => false and :new_record => false to prevent adding h3 block and 'New object' button for has_many form