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

Convert active_admin_form_for to Arbre component - resolves #2542 #3486

Merged

Conversation

@varyonic
Copy link
Contributor

commented Oct 13, 2014

Resolves #2542. Uses FormBuilderProxy as suggested by Igor in #2689, removing and replacing form_buffers from ActiveAdmin::FormBuilder. As a bonus allows 'inputs', etc. instead of 'f.inputs' in DSL. Allowing Arbre within has_many block deferred as a second step. May break existing code that mixes ERB inside active_admin_form_for in a template. Can also break other code that makes assumptions about the context of the block, see fix to active_admin_comments.rb included.

@timoschilling

This comment has been minimized.

Copy link
Member

commented Oct 16, 2014

Can you add some examples into the readme and/or the docs

@zorab47

This comment has been minimized.

Copy link
Contributor

commented Oct 17, 2014

Wow, awesome work. 😸

@varyonic varyonic closed this Oct 19, 2014

@varyonic varyonic deleted the varyonic:2542-arbe-buffering-inside-aa-forms branch Oct 19, 2014

@varyonic varyonic restored the varyonic:2542-arbe-buffering-inside-aa-forms branch Oct 19, 2014

@varyonic varyonic reopened this Oct 19, 2014

@varyonic varyonic force-pushed the varyonic:2542-arbe-buffering-inside-aa-forms branch from 3a1e1bd to 259f2cd Oct 19, 2014

@timoschilling timoschilling added workaround and removed 4 - Review labels Oct 20, 2014

@timoschilling

This comment has been minimized.

Copy link
Member

commented Oct 20, 2014

Is the old way still supported?

@timoschilling

This comment has been minimized.

Copy link
Member

commented Oct 20, 2014

An the fist look it's great! 👍

Please apply my notes and make repush, to get more overview

@varyonic varyonic force-pushed the varyonic:2542-arbe-buffering-inside-aa-forms branch from 259f2cd to 774c5f0 Oct 21, 2014

@timoschilling timoschilling added 4 - Review and removed workaround labels Oct 21, 2014

@timoschilling

This comment has been minimized.

Copy link
Member

commented Oct 21, 2014

I'm not sure, but c10a6c4 can bring problems to the developer.
@seanlinsley what is your opinion to c10a6c4?

@timoschilling

This comment has been minimized.

Copy link
Member

commented Oct 23, 2014

I think many users use f.form_buffers in there code. @varyonic and @seanlinsley how should we deal with that?

def form_buffers
  raise "`form_buffers` has removed from ActiveAdmin:: FormBuilder, please read https://github.com/activeadmin/activeadmin/blob/master/docs/5-forms.md for details."
end
@Fivell

This comment has been minimized.

Copy link
Member

commented Oct 23, 2014

Agree about f.form_buffers , we have too many projects using AA from edge, this change is critical for us ...

@varyonic

This comment has been minimized.

Copy link
Contributor Author

commented Oct 24, 2014

@timoschilling I added a backwardly compatible form_buffers method with a deprecation warning.

@seanlinsley

This comment has been minimized.

Copy link
Member

commented Oct 24, 2014

I'd rather just raise an error. The next release is going to be 1.0.0, and we'll be following semantic versioning. I really don't want that cruft in the code base until 2.0.0

@timoschilling

This comment has been minimized.

Copy link
Member

commented Oct 24, 2014

@varyonic nice work, but I have the same meaning as @seanlinsley

@varyonic varyonic force-pushed the varyonic:2542-arbe-buffering-inside-aa-forms branch from 474cfbd to 1a8fcf0 Oct 25, 2014

@varyonic

This comment has been minimized.

Copy link
Contributor Author

commented Oct 25, 2014

Moved backwardly compatible version of form_buffers to https://gist.github.com/varyonic/542c37648de91a02402a If anyone decides to use it please star and comment on the gist.

@varyonic varyonic force-pushed the varyonic:2542-arbe-buffering-inside-aa-forms branch from 1a8fcf0 to d7e8d73 Oct 25, 2014

@varyonic varyonic force-pushed the varyonic:2542-arbe-buffering-inside-aa-forms branch from d7e8d73 to c024ce3 Oct 25, 2014

@varyonic varyonic force-pushed the varyonic:2542-arbe-buffering-inside-aa-forms branch from c024ce3 to 3570bfa Oct 25, 2014

@timoschilling

This comment has been minimized.

Copy link
Member

commented Oct 25, 2014

@varyonic thanks for your great work on this!!! 👍 ❤️

timoschilling added a commit that referenced this pull request Oct 25, 2014
Merge pull request #3486 from varyonic/2542-arbe-buffering-inside-aa-…
…forms

Convert active_admin_form_for to Arbre component - resolves #2542

@timoschilling timoschilling merged commit d775bb9 into activeadmin:master Oct 25, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@varyonic varyonic deleted the varyonic:2542-arbe-buffering-inside-aa-forms branch Oct 27, 2014

brianswko pushed a commit to lumoslabs/ActiveAdmin-Globalize3-inputs that referenced this pull request Jun 1, 2015
@brianswko brianswko referenced this pull request Jun 1, 2015
@fabn fabn referenced this pull request Jul 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.