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

Enhancement: put Appplication Form in its own class #30

Open
wernerkrauss opened this issue Jul 6, 2023 · 2 comments · May be fixed by #31
Open

Enhancement: put Appplication Form in its own class #30

wernerkrauss opened this issue Jul 6, 2023 · 2 comments · May be fixed by #31

Comments

@wernerkrauss
Copy link

In my current project I heavily adjusted ApplicationForm in JobController, now I need the same form with a dropdown of available jobs in the JobCollectionController.

It'd be cleaner, if ApplicationForm was encapsulated in a subclass of Form, then all the global modifications could be done in one place and I could adjust it slightly per controller.

@muskie9
Copy link
Member

muskie9 commented Jul 6, 2023

@wernerkrauss this is a very timely ticket as we have been discussing this and other similar adjustments to make the module more extensible. I like the idea of the form being a subclass and encapsulated for the very reason you mentioned.

We have a project coming up which we were planning on putting quite a bit of work into the module but if you're open to taking a shot at making this type of change to speed things up we're happy to review so you're not waiting on this feature.

@muskie9
Copy link
Member

muskie9 commented Jul 7, 2023

@wernerkrauss I've started #31 as a super rough PR. There are a few things that may be out of scope for this that I noticed as broken and haven't been able to test a submission quite yet.

This is also in a Silverstripe 4.13 installation. Some of these changes could require a new major version of the module, but if nothing is breaking between 4 and 5 we could possibly support both in the new major version. Open to feedback/suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants