Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Updated: Comment #N
Problem/Motivation
As we convert forms to use FormInterface, we come across cases where the form is used not as the route controller directly, and drupal_get_form() must be used.
We end up with code like this:
drupal_get_form(new SearchBlockForm());
drupal_get_form(ConfirmDeleteMultiple::create(\Drupal::getContainer()));
Proposed resolution
Make drupal_get_form() smart enough to take a class name, and instantiate it. Follow the same code flow as HtmlFormController (the class that process _form controllers):
drupal_get_form('Drupal\comment\Form\ConfirmDeleteMultiple');
drupal_get_form('Drupal\search\Form\SearchBlockForm');
Remaining tasks
N/A
User interface changes
N/A
API changes
API addition: Allow class names to be passed to drupal_get_form().
Related Issues
Comment | File | Size | Author |
---|---|---|---|
#3 | interdiff.txt | 579 bytes | tim.plunkett |
#3 | forms-2112711-3.patch | 13.5 KB | tim.plunkett |
#1 | forms-2112711-1.patch | 13.48 KB | tim.plunkett |
Comments
Comment #1
tim.plunkettThis will help #2110643: ControllerBase::container() and FormBase::container() needs to be private as well.
Comment #3
tim.plunkettDuh.
Also, we could decide to prevent passing an object to drupal_get_form(), which will reduce the number of ways to do one thing, and will force proper usage of ContainerInjectionInterface when dependencies are needed.
However, that would make this an API Change, not Addition.
Comment #4
Crell CreditAttribution: Crell commentedThis was actually a problem I ran into when creating a training demo module that had a form inside of a block. We had to pass several otherwise-irrelevant dependencies through multiple layers to get it to the form. Very +1 in concept. I've only skimmed the code so far; let's give the bot a chance.
Comment #5
ParisLiakos CreditAttribution: ParisLiakos commented+1000
this patch is a beauty
Comment #6
webchickThis is one of my favourite DX patches of all time, and I think it is worthy of "major" status:
a) You eliminate a bunch of stupid "use" statements that are not in fact "used" anywhere in the file.
b) You don't need to struggle with whether it should be
FooForm::create()
ornew FooForm
, depending on if DI is used.c) You empower copy/paste/modify developers to get stuff done without having to understand the entire inheritance tree of their dependencies.
So, in other words, plus freaking one!
Committed and pushed to 8.x.
Could we get a quick turnaround on this change notice? Alpha 4 is due out tomorrow. :)
Comment #7
andypostAlso this needs major follow-up to update doc-block of
drupal_get_form()
to current stateComment #8
andyposttag back
Comment #9
tim.plunkettComment #10
XanoDoes this need a dedicated change notice, or is the mention of this feature in https://drupal.org/node/1932058 enough?
Comment #11
Shyamala CreditAttribution: Shyamala commentedWriting Change notification.
Comment #12
Shyamala CreditAttribution: Shyamala commentedSummary
getFormID(), buildForm(), validateForm(), submitForm()
Before
drupal_get_form
was used as -<code>return drupal_get_form(new SearchBlockForm(), $this->request);
After
drupal_get_form
now used as -return drupal_get_form('Drupal\search\Form\SearchBlockForm');
Comment #13
tim.plunkettI would include the
use
statements for the "Before" section, otherwise this looks great! I think it can be published.Comment #14
Shyamala CreditAttribution: Shyamala commentedUpdated change notification at https://drupal.org/node/2173127
Comment #15
Shyamala CreditAttribution: Shyamala commentedRemoved Change notification tag.
Comment #17
quietone CreditAttribution: quietone at PreviousNext commentedpublish change record