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

Work in progress: Add ability to link/unlink existing records #13

Merged
merged 5 commits into from
Sep 26, 2018

Conversation

mlewis-everley
Copy link
Contributor

This module works great, but you cannot link existing records (or unlink a record once linked).

This PR lays some of the groundwork, but I am am now kind of stuck...

Basically, the GridFieldAddExistingAutoCompleter and the new GridFieldHasOneUnlinkButton work, but they do not force the GridField to reload (or possibly, the GridField is not properly updated on reload).

I was wondering if you guys might have some insight? I am assuming you would like to add this functionality (to me it makes sense to be able to link/unlink as well as just add new)?

@hchokshi
Copy link
Contributor

@mlewis-everley This package hasn't had a commit in quite a while, so I've created a fork that has some extra functionality and also merged in your PR with some refactoring at hchokshi/silverstripe-hasonefield.

It's marked as a composer replacement for this package under 3.x-dev due to a couple of semver breaks.

@mlewis-everley
Copy link
Contributor Author

@hchokshi, Cool! I have been meaning to look into this in more detail, so I will try and check it out ASAP.

I think this repo is low priority, but it is still maintained by @bummzack and @wilr (at least as far as I am aware). If your changes have finished off what I was doing, maybe you could submit a PR back to this repo and one of those guys might be able to merge it in?

@wilr
Copy link
Contributor

wilr commented Sep 26, 2018

@hchokshi long term did you want to PR your changes back here or do you want us to redirect this repo to your version?

@hchokshi
Copy link
Contributor

hchokshi commented Sep 26, 2018

@wilr I'm happy to PR this back. There is a breaking change in this commit - hchokshi@b7033ed

If you guys are OK with issuing a new major version with this change, I'll revert my composer vendor/replaces changes and PR this in?

@wilr
Copy link
Contributor

wilr commented Sep 26, 2018

Yep I'll tag a release prior to merge and start a new release line with your version.

@mlewis-everley
Copy link
Contributor Author

Unfortunately I am still unable to get this PR request finished. @wilr (as you are about) any idea how I can make the parent EditForm reload after linking/unlinking an existing record?

@hchokshi
Copy link
Contributor

@mlewis-everley I merged your commits into my fork and fixed that reload issue (a962987).

@wilr #15 is up 😄

@wilr wilr merged commit a414a7e into silvershop:master Sep 26, 2018
@wilr
Copy link
Contributor

wilr commented Sep 26, 2018

Awesome work @hchokshi

@hchokshi
Copy link
Contributor

Thanks @wilr. Also nice work @mlewis-everley 😃

@mlewis-everley
Copy link
Contributor Author

@hchokshi OK, thanks, I think I can see your fix (although I tried this and couldn't seem to get it working).

You seem to have removed the link existing button though (which was also part of this PR)?

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.

None yet

3 participants