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

Add support many nested forms #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

NikitaNaumenko
Copy link

In one of my pet project, I've found the issue. That component doesn't work if you need to have more than one nested model form. I've fixed and also save the compatibility with one model nested form.

@brodyhoskins
Copy link

I second this pull request. 😅 I'm currently using it on a few sites and it's working great.

@guillaumebriday
Copy link
Member

Thanks @NikitaNaumenko for the PR

Can you fix conflicts so I can review it?

@NikitaNaumenko
Copy link
Author

@guillaumebriday yep, I'll do it

@NikitaNaumenko
Copy link
Author

@guillaumebriday done!

@dp6ai
Copy link

dp6ai commented Aug 3, 2021

Hi.

Hi @guillaumebriday do you have an idea of when this may get merged?

Thanks

@ferrisoxide
Copy link

Hey folks. Maybe I have this wrong, but it looks like you can use multiple nested forms now. I should probably have commented on this ticket, but I've left notes here: #8 (comment)

Basically, just moving the controller attributes out of the form_with proper and into separate divs seems to do the trick. Just verified that it's working in an app I've working on, to make sure I wasn't going mad.

@aquilarafa
Copy link

I think the problem is this line
const content = this.templateTarget.innerHTML.replace(/NEW_RECORD/g, new Date().getTime().toString())
Since using /g will replace every ocurrence of NEW_RECORD then deep nested controller scopes will receive the parent timestamp id.

@aquilarafa
Copy link

aquilarafa commented Apr 19, 2022

I think the problem is this line const content = this.templateTarget.innerHTML.replace(/NEW_RECORD/g, new Date().getTime().toString()) Since using /g will replace every ocurrence of NEW_RECORD then deep nested controller scopes will receive the parent timestamp id.

Allowing to customize the NEW_RECORD placeholder for each nested form builder worked for me.


  static values = {
        wrapperSelector: {
            type: String,
            default: '.nested-form-wrapper'
        },
        newRecordPlaceholder: {
            type: String,
            default: 'NEW_RECORD'
        },
    }

    add(e) {
        e.preventDefault()
        const regex = new RegExp(`${this.newRecordPlaceholderValue}`, 'g')
        const content = this.templateTarget.innerHTML.replace(regex, new Date().getTime().toString())
        this.targetTarget.insertAdjacentHTML('beforebegin', content)
    }

In my case the first form receives a child_index placeholder of "NEW_RECORD_1" and the deep nested receives "NEW_RECORD_2" for example.

@aquilarafa
Copy link

I think the problem is this line const content = this.templateTarget.innerHTML.replace(/NEW_RECORD/g, new Date().getTime().toString()) Since using /g will replace every ocurrence of NEW_RECORD then deep nested controller scopes will receive the parent timestamp id.

Allowing to customize the NEW_RECORD placeholder for each nested form builder worked for me.


  static values = {
        wrapperSelector: {
            type: String,
            default: '.nested-form-wrapper'
        },
        newRecordPlaceholder: {
            type: String,
            default: 'NEW_RECORD'
        },
    }

    add(e) {
        e.preventDefault()
        const regex = new RegExp(`${this.newRecordPlaceholderValue}`, 'g')
        const content = this.templateTarget.innerHTML.replace(regex, new Date().getTime().toString())
        this.targetTarget.insertAdjacentHTML('beforebegin', content)
    }

In my case the first form receives a child_index placeholder of "NEW_RECORD_1" and the deep nested receives "NEW_RECORD_2" for example.

Using a custom stimulus value solves this issue but maybe a smarter way to set this id could bring a better experience. Any ideas?

@planetaska
Copy link

planetaska commented Aug 18, 2023

Can confirm @aquilarafa 's idea works for many layers of nested forms.

@ferrisoxide I think the issue is when you need to have a nested form that has 2 layers or more, the template in the second layer will be created with name value like this:

policy[agents_attributes][1692332505676][overrides_attributes][1692332505676][override_type]

Notice the same id for both parent and child records. This will make the form only submit the last item created in the 2nd layer, because all inputs in that layer will have the same name.

The correct template name value should be something like this:

policy[agents_attributes][1692332505676][overrides_attributes][NEW_OVERRIDE][override_type]

For a slightly "smarter" solution, I guess since we can get child_index value from the form builder:

# will return 'NEW_RECORD'
fields_for_form_builder.options[:child_index]

We can add a hidden field to the template, then figure out the given child index from the Stimulus controller?

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.

7 participants