Fix for missing form fields in modal ajax call

[dividy]

This PR should be better than my previous one.
I'm trying to send the current form fields to the modal endpoint.
The code was already present in 2 other places in this file, so I integrated it in this ajax call too.

[request-info]

Hi there!

Could you please provide us with more info about this? Looks like you skipped the title/body.

Thank you!

--
Justin Case
The Backpack Robot

[scrutinizer-notifier]

The inspection completed: No new issues

[pxpm]

@tabacitu do you think this is something we can provide ?

It would allow for example to add/remove fields from inline create modal based on some form input.

I am just wondering, lot's of times (i think most of them) we don't want the full input of the form, rather 1/2/3 fields we are going to work on.

Should we implement something along the lines:

• 'include_form_fields' => true to include ALL form fields.
• 'include_form_fields' => 'field' to include only that field.
• 'include_form_fields' => ['field1','field2'] to include multiple fields.

By default 'include_form_fields' => false

[tabacitu]

Wow this looks like it would be useful indeed - thank you @dividy .

@pxpm I think you're right - an option to pick-and-choose which fields to send over would be even better. That way you don't bloat up the InlineCreate form with ALL form fields, which might end up conflicting anyway (input names from the main form and input names from the inlineCreate form). Excellent idea!

The one thing I'm pondering over though, from your suggestion @pxpm , is the position and naming of the attribute. Maybe:

[   // relationship
    'type'          => "relationship",
    'name'          => 'tags', // the method on your model that defines the relationship
    'ajax'          => true,
    'inline_create' => [ 
        'entity' => 'tag',
        'include_main_form_inputs' => true,
    ] // you need to specify the entity in singular
],

What I'm saying is:

• (1) Maybe a more accurate name would be include_main_form_inputs (difference being "main" and "inputs" instead of "fields" since that's what will actually be forwarded to the InlineCreate modal - right?); I'm not 100% sure about this though - maybe fields is easier to remember? Maybe not many people know the distinction between fields and inputs?
• (2) That the attribute itself should probably be inside the inline_create attribute, since it's only used there;

Other than that, I think this is an excellent feature already, great teamwork here! @dividy do you agree with the suggestions? Would you be interested in finishing up this feature, or should @pxpm or I schedule time to do it?

Cheers!

[pxpm]

Agree with @tabacitu

This should be pushed inside inline create definition array so we adhere to what it is really doing.

EDIT: Also I vouche for fields instead of inputs. They are inputs technically speaking, they are backpack fields in our panels. 😄

[dividy]

Other than that, I think this is an excellent feature already, great teamwork here! @dividy do you agree with the suggestions? Would you be interested in finishing up this feature, or should @pxpm or I schedule time to do it?

Ok, I'll do it.

[tabacitu]

Wow, awesome to hear that! Thanks a lot @dividy !

[pxpm]

Hello @dividy

Once again, thanks for the PR, I just submitted a complete version of it: #3013

All the options should be available now.

I am going to close this as I think #3013 would be a better general solution, 90%+ of time people does not need all the parent form fields.

Wish you the best,
Pedro

[dividy]

@tabacitu Sorry for the delay... was about to make it this week.
I'm late on all my projects currently.

Great job @pxpm !

[pxpm]

@tabacitu Sorry for the delay... was about to make it this week.
I'm late on all my projects currently.

Great job @pxpm !

Totally understandable. I just had this started in my local branch so I just polished it and submitted the PR.

If there is something more we can do to speed up your development process let us know, maybe it's a good backpack feature :)

Wish you the best @dividy

Pedro.

[tabacitu]

Update: just merged #3013 which fixes this. It'll be available later today with a composer update. Thanks again for this, you guys!

[tabacitu]

Update: sorry it looks like there might still be problems/inconsistencies with #3013 (comment) - at least the way I understand it. So this will NOT be available later today.