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 NestedForm integration #202

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

add NestedForm integration #202

wants to merge 1 commit into from

Conversation

smskin
Copy link
Collaborator

@smskin smskin commented Jan 30, 2022

Resolved conflicts of PR #146

Due to the long response, I had to recreate the PR 200 (#200) to release the master branch. the master branch will be used to create a package in the packagist.

New package with fixes: https://packagist.org/packages/smskin/nova-dependency-container


Nova Nested Form is a useful package to create related resource inline using a cool UX.
This package has some problem if it contains a DependencyContainer field.

This PR allow these 2 packages to cooperate.

NestedForm uses a custom field syntax for nested purpose (based on PHP square bracket array syntax).
This PR change standard behavior of DependencyContainer to use this syntax if it used inside of a NestedForm.
It is obviously completely retro-compatible.

It fixes these PRs
yassilah/laravel-nova-nested-form#26
yassilah/laravel-nova-nested-form#114

@smskin
Copy link
Collaborator Author

smskin commented Apr 30, 2022

@NoahNxT, @wize-wize I think that is not correct if I accept the pull request that I created. Can you review it?

@NoahNxT
Copy link
Contributor

NoahNxT commented Apr 30, 2022

I'm not at my laptop today, but I already tested it previously and it looked good, haven't found anything breaking. You can merge it or wait until tomorrow when I'm home to test it 1 last time.

@wize-wiz
Copy link
Contributor

@wize-wize I think that is not correct if I accept the pull request that I created. Can you review it?

@NoahNxT @smskin

Currently I have no setup to test it. I haven't even looked at Nova 4, sadly I can't be of any help.

I guess it's just going to be merging a bunch of PRs that look good after some testing and cleanup/close many of the current issues/PRs and just see what the response will be. I fear some of the PR's became obsolete or the author doesn't respond/care to update the code base anymore.

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.

3 participants