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

Accept multiple attributes #30 #33

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kirs
Copy link

@kirs kirs commented Dec 27, 2014

No description provided.

@kirs kirs mentioned this pull request Dec 27, 2014
@m-Peter
Copy link

m-Peter commented Dec 30, 2014

That's great @kirs !!!

@kirs
Copy link
Author

kirs commented Dec 30, 2014

I'm glad you like it! AFAIK it's frozen now for Rails core team review as @kaspth suggested.

@m-Peter
Copy link

m-Peter commented Dec 30, 2014

I'm currently reworking the ActiveForm::Base class. As you probably noticed, you have to provide the same methods for the ActiveForm::Base and ActiveForm::Form classes. Ideally the ActiveForm::Base should delegate such methods to the root model (which should be wrapped by a ActiveForm::Form class).

@kaspth
Copy link

kaspth commented Jan 2, 2015

@m-Peter You might want to check out the last commits in #18 more closely then 😁

My main realization was that main_model was really just another association and that there was no reason for it to be treated differently. I took to naming it ModelAssociation partly because a number of things had Form in their name and partly because the model being wrapped isn't really a form but an associated model 😉

Looking forward to what you come up with, 👍

@carlosantoniodasilva
Copy link
Member

Unfortunately I don't think we should copy over Rails code to handle multiparameters, we should think of a way to extract that on the Rails side, but I'm not sure that's going to be done. I've proposed this extraction idea, lets see what others think first, if it's accepted we can start by that.

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.

4 participants