Skip to content
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

Hide new record link for has_many form #2134

Merged
merged 1 commit into from
Apr 25, 2013

Conversation

developer88
Copy link
Contributor

This changes will do:

  1. add option 'new_record' for has_many form to hide 'new item link'
  2. add missed specs for 'heading' option that discussed in #1945
  3. add changes to documentation to docs/5-forms.md

Screenshots to illustrate changes:

Showing Add button
before2

not showing Add button
after2

@@ -80,7 +80,8 @@ def has_many(association, options = {}, &block)

inputs options, &form_block

form_buffers.last << 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to see the more implicit version:

js = options[:new_record] ? js_for_has_many(association, form_block, template) : ""

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

@seanlinsley
Copy link
Contributor

@developer88 could you squash these into a single commit with a useful commit message?

@seanlinsley
Copy link
Contributor

After that I'll pull this in

@developer88
Copy link
Contributor Author

@daxter it's done!

@seanlinsley
Copy link
Contributor

Okay, merging. Thanks for taking the time to contribute :)

seanlinsley added a commit that referenced this pull request Apr 25, 2013
Hide new record link for has_many form
@seanlinsley seanlinsley merged commit 4b58b84 into activeadmin:master Apr 25, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants