Some naming conventions are not up to date with Drupal 8.
build > buildForm
validate > validateForm
submit > submitForm
getId > getFormId
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | cool-drupal8-formapi-update-2401887-5-D7.patch | 7.02 KB | oliverpolden |
Some naming conventions are not up to date with Drupal 8.
build > buildForm
validate > validateForm
submit > submitForm
getId > getFormId
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | cool-drupal8-formapi-update-2401887-5-D7.patch | 7.02 KB | oliverpolden |
Comments
Comment #1
oliverpolden commentedComment #2
oliverpolden commentedCorrected patch
Comment #3
pedrorocha commentedHi oliverpolden, thanks for the interest!
Unfortunatelly, I don't see this module being ported to Drupal 8 and many modules are already depending on this method signature, so I don't see a huge benefit on making this change, while I do see many problems. It would be an option only for a 2.x branch.
I do agree that is a good thing to follow what become a standard in the Drupal community(despite the fact that I don't like too much to say "validateForm" inside a form, as it is kind of obvious that we are doing things on a form, in a form controller class), so i'll consider this patch if a new 2.x branch become a reality.
In fact, i'll probably create this 2.x branch in a near future, because I already want to change some decisions, but it would break a lot of modules that I made, so i'm waiting to have a more stable feature set and test the architectural decisions to see which path to go with the 2.x branch.
Thanks again for the help and if you have any other suggestions, please let me know!
Comment #4
oliverpolden commentedThis isn't to port to Drupal 8. This is to update the work for Drupal 7 to bring it up to date with the current state of the API in Drupal 8 making it easier to port dependent modules to Drupal 8 when that time comes.
I do agree that it should be a new version of the module so it doesn't break other dependencies.
Comment #5
oliverpolden commentedJust realised the Form controller has changed from FormController to just Form. This patch includes a fix for that in hook_forms().
Comment #6
pedrorocha commentedHi oliver, I understand that this is a patch for D7. My point was that i'm not exactly following D8 conventions cause i'm using D7 and don't have a plan to update the module to D8(I don't even know if does make sense to port it, as many things will need YAML files and I don't know if we'll be able to create some base classes like COOL does).
About the patches, when the time to create this new 2.x branch comes, i'll probably use them. For now, i'll keep the issue as "postponed", cause it'll not be implemented for some time, for the reasons explained above.
When the moment comes, let's check again, to see how the convention will be.
Comment #7
oliverpolden commentedOf course the COOL module would only be used in D7. With the changes I propose, any Forms defined in classes that depend on the COOL module could be pretty much dropped into a D8 module, no need for the COOL module. In fact, an entire src folder could potentially be dropped into D8.
Since the D8 APIs are frozen, we can confidently update COOL to reflect them.
The whole point is to allow any development in D7 to be forward compatible with D8. I wrote a blog post about this here http://www.oliverpolden.com/content/drupal-8-psr-4-form-compatibility-dr....
Comment #8
pedrorocha commentedHey! It's great to see an article extending my module! hehe
About the changes, I had to create a long time desired feature this week: dynamic blocks(1 class for multiple configurable blocks), and while working on it, some changes were needed and I decided to took a step toward the new 2.x branch right now. Locally, on my project, it does exist already. Do you have some time to take a look and give some feedback on what to improve, beyond this patch? I'll create and issue to discuss it.
Comment #9
pedrorocha commentedHere is the 2.x branch discussion: #2405169: New 2.x branch: conceptual changes to the API