Problem/Motivation

The D7-style component API has too many limitations and side-effects to write about.

Proposed resolution

Convert components to annotated, classes plugins that also implement PluginFormInterface to render the actual form elements. Use separate classes (as configured in the plugins' annotations) for rendering values or administrative configuration elements (don't put too many things in one class, basically).

All this makes for easier testing, and PluginFormInterface aides in component validation and submission. I can understand if you want to look at how Field API does this, but please don't. It isn't designed too well, mostly because we didn't get many useful features until after the API was redesigned for Drupal 8, even the maintainers agree to this.

Remaining tasks

T.B.D.

User interface changes

None.

API changes

Many. Yay!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

twistor’s picture

Point of clarification, shouldn't the PluginFormInterface be for plugin configuration? We would use a separate class, WidgetSomethingInterface, for rendering user-facing forms.

paulmckibben’s picture

Checking on the status of this task. Has any progress been made toward a plugin architecture for components? @fenstrat/@DanChadwicik, is this something @ttamniwdoog and I can help with? Hate to jump in if you're just about to release a solution. Thanks!

DanChadwick’s picture

I am only passively working on the D8 branch -- specifically applying D7 patches and managing issues so that D7 changes don't get lost.

fenstrat’s picture

No, nothings been done here @paulmckibben, if you and @ttamniwdoog want to jump in here that would be seriously appreciated! Even if it's just a WIP post a patch here, this is the exciting stuff of moving to D8!

DanChadwick’s picture

Is there an OOP roadmap for this? Do you imagine using inheritance for components (such as abstract class WebformComponent) or are you thinking of an interface (such as WebfromComponentable)?

Should there be some architecture discussion before the coding begins?

fenstrat’s picture

Assigned: Unassigned » fenstrat

No there's no roadmap here as of yet Dan, I've simply not had the time. Going to take a very rough first pass at it, working on that now.

fenstrat’s picture

Assigned: fenstrat » Unassigned
Status: Active » Needs work
FileSize
6.17 KB

Ok here's a very early WIP, but amazingly it actually works.

  1. Adds \Drupal\webform\Annotation\Component to define the expected properties. Only $id, $label, $description atm.
  2. Adds \Drupal\webform\ComponentInterface to define the basic methods available, again only getLabel() and getDescription() atm.
  3. Adds \Drupal\webform\ComponentBase to implement ComponentInterface
  4. Adds a ComponentManager, still not 100% on this. I've added an alterInfo('webform_component_info') call, but all of this needs work/testing.
  5. Adds Date.php and Email.php component "skeletons", again with just id, label and description.
  6. Uses the manager service in WebformSettingsForm to output the component type, again just the start, but at least it is working.

I've not got any more time for this now. Whoever wants to pick it up just make sure you assigning the issue to yourself before you start work so we don't step on each others toes.

ttamniwdoog’s picture

Assigned: Unassigned » ttamniwdoog

I've tested the patch and I have not seen an error related to this WIP. I'm going to spend some time over the next few days trying to add to what you've done here. Thanks for getting this started, which was the hard part :-)

fenstrat’s picture

Great, thanks @ttamniwdoog!

ttamniwdoog’s picture

Added the remaining shells for plugins following fenstrat's lead there.

I'd like to know what you envision as the next steps are here.
Since we cannot adequately test these components as plugins without having a route to view them.
I think DanChadwick inquired about a roadmap and I think that would be great but I think that might be a tall order.
I'm happy to continue to help but I'll need a little direction.

Thanks again!!

Michelle’s picture

Assigned: ttamniwdoog » Michelle

Just noting that I've picked this up from Matt. Not a lot of progress so far. Still trying to get the form working to add components. Will be working on it more tomorrow.

fenstrat’s picture

Any progress here @Michelle? Even a WIP. I'm spending next week on webform and this is one of the first patches I'd like to move forward.

Michelle’s picture

Oh! I'm so sorry. I had a lot of work in progress and then we realized that the client only needed a contact form and switched to doing what was needed for that. I had intended to take some of my own time to finish the in progress stuff and submit a patch and it just got lost in the mad rush of days.

When you say "next week" do you mean _next_ week as in starting on May 11th or this next week as in starting today? If it's the former, I'll spend some time this week getting it cleaned up and submitted. If you need it sooner, I'll just make a patch out of what I have as is.

fenstrat’s picture

No worries at all Michelle! Yeah by next week I mean the one starting May 11th. I'd like to get going this weekend 9th/10th so wherever you're at by the end of this week put it up as a patch and I'll jump in from there. Cheers.

Michelle’s picture

Ok, will do!

Michelle’s picture

Sorry this is late. I forgot about timezones and figured first thing in the morning would be ok but I suppose Saturday is half gone for you by now. :(

Anyway, it's been serious crunch time at work and all my brainpower has been sucked up there so I did not have time to clean this up. When I tried to do a diff against a fresh clone, I was getting changes that I don't think should be part of this so I ended up doing a diff against the last commit before I started making changes, which is this one:

commit 865baf2f1705cab7cd3c48b9cccab39bc0ad5356
Author: lkmorlan
Date: Sun Apr 12 08:41:25 2015 -0400

Issue #2465291 by Liam Morland: Batch download of submissions postgres compatibility.

This patch includes the prior patch on this issue.

It's been a while since I've looked at the code but I think I was following the image module example of imagestyle and effect with the webform being the imagestyle and the component being the effect. I believe I was in the process of making a webform config entity based on the imagestyle one when I got pulled off it. I know that's out of scope for the components to plugins on this issue but it seemed like the next logical step to put it all together and I was trying to get this working for the client at the time, not just focused on this issue.

Sorry it's not a clean and clear patch. :( Hopefully you can still make use of it.

fenstrat’s picture

No worries, thanks Michelle. As you'd not posted anything I started work on converting /node/{node}/webform, i.e. the component add/edit page for an individual webform. But it's kind of a chicken and egg situation as without the conversion to plugins quite a deal of that needs reworking based on this. So I'll work on merging what I'd started there with what you've got here. Tricky as there's so many broken things! Anyway, thanks for your work here.

Michelle’s picture

Yeah, that's the hard thing about porting modules is there is so much broken to start with that you keep having to fix one thing to get another thing working and so on.

I am pretty sure I had /node/{node}/webform working as far as being able to actually build a webform. You couldn't do anything with it but you could put the components on it. One of the next steps was to move the components' form code from the .inc files into the buildForm() of each plugin. I started with TextField and I _think_ I had that working but I'm not positive. I switched away from this on April 17 so it's all a little fuzzy. These are my last notes before the switch:

Got the mechanics of the main component form as well as adding components working with dummy data.

To do to get this working for [client]'s needs:
* Creating webforms
- Store and retrieve the webform configuration to use in the form.
- Make all component plugins deliver their own forms.
- Add edit/delete component forms and link them to the actions on the main form
- General cleanup, commenting, removal of hardcoded test data.

* Using webforms
- Not started yet

* Eloqua integration
- Not started yet

fenstrat’s picture

Assigned: Michelle » Unassigned

Pretty sure @Michelle isn't working on this, and I'm not currently either. I think there's just not enough functionality actually working in webform 8.x-4.x to attempt this conversion of components to plugins. Therefore I'm working towards trying to get some of the basic functionality ported and working in D8, and ideally some or all of the tests ported as well. Once that is done this will be my top priority. Having said that if someone is interested in continuing this now by all means go for it.

Michelle’s picture

No, I'm not anymore. My contrib time is all over the place based on what clients need, unfortunately. It's wonderful that the company gives back so I don't want to sound like a complaint but it does make it hard to stay focused on a project for the long haul. Hopefully as we get more D8 clients someone else will need this. :)

safetypin’s picture

Here's a clean patch from the current 8.x-4.x branch. I didn't make any changes to anything, hopefully I didn't make any mistakes either.

safetypin’s picture

Of course, I did actually make a mistake: I put the Annotations folder in the wrong place. I'm hiding my previous patch.

safetypin’s picture

Here's a corrected reroll of patch #16 against the main 8.x-4.x branch.

ianthomas_uk’s picture

This reroll has conflicted with #2488934: Convert webform_components_form to D8 which has already implemented WebformComponentsForm. You should not be replacing code from that issue unless you have a reason to.

safetypin’s picture

@ianthomas_uk Ah, I suppose I should have realized that since file exists that it was committed from another issue. I'll remove that part of the patch.

safetypin’s picture

FileSize
20.42 KB

I've gone through the commit, http://cgit.drupalcode.org/webform/commit/?id=db6601c, from #2488934: Convert webform_components_form to D8, and as far as I can tell, the only conflicting changes are the src/Form/WebformComponentAddForm.php file. So, I've removed that section of the patch. I'll hide the old patch, since it contained the error. @ianthomas_uk, do you see any other problems with this reroll?

safetypin’s picture

So, I've made a rough first-pass at a webform component that can actually be saved to the database. I really just copied over the old insert code, and the only thing I adjusted was the code to lock the database before incrementing a component's cid.

tedbow’s picture

I this is an important step I have added some stuff to the last patch.

This patch allows you to add and edit components on a webform. I have only been testing in on Text Field. I would suggest getting it working on 1 or two component types to figure out the ComponentInterface will need to be and what can live in ComponentBase before filling out the rest of the components.

I am providing and interdiff but here is what this patch adds

  1. Separates Component forms in WebformComponentAddForm and WebformComponentEditForm which both extend a new abstract class WebformComponentFormBase. These forms will have prepareComponent method that will get component ready whether new or existing.
  2. Adds ability to edit existing forms.
  3. Adds $supports_* properties like $supports_unique to ComponentBase so that components don't have to duplicate form elements that are used in many components. This can be overridden by subclasses and the parent will add the element.
  4. Moved a bunch of form elements out of TextField.php because it is $supports_ properties from above
  5. Moved WebformComponentAddForm->insertCompent to ComponentBase->save which will work for add and edit.
  6. Added $configuration property to ComponentBase with getter and setter methods
  7. Move Component plugins from Plugins\Components to Plugins\WebformComponents. My thought here is that in this module it is very obvious what Components are but for other contrib modules that add components to Webform it might not be obvious what the folder Plugins\Components is especially if the module does other things that aren't Webform related.

  • podarok committed 0eabc59 on 8.x-4.x authored by tedbow
    Issue #2293945 by safetypin, tedbow, fenstrat, Michelle, ttamniwdoog:...
podarok’s picture

@tedbow I've merged your work. As for me it looks good. Still needs work, but let's plan out progress to make small changes to be able to test them easily. Ping me directly if you'll have new additions

podarok’s picture

I'm suggesting to work on most popupal components at first
- textfield
- email
- checkbox
- select
- phone

podarok’s picture

I can be a bit crazy, but let's start to talk about making webform as part of Contact module
It already has all needed components, except of submissions storage

Michelle’s picture

If you want to go that route, here's the storage: https://www.drupal.org/project/contact_storage

podarok’s picture

@Michelle - wow
Thanks you tons
The only thing is to write upgrade path, I guess

Michelle’s picture

No problem. That's the route we ended up going for a project. I don't know if it will meet everyone's needs that uses webform but it works very well for a general contact form.

safetypin’s picture

@podarok, have you considered the peformance implications when scaled? I'm not an expert, but this has been discussed before, and I don't think it was resolved. It may well be a very unusual case, but some d7 webforms have hundreds of fields, and hundreds of thousands of submissions. I don't have any that are that big, but others have voiced this concern.

Michelle’s picture

Just noticed... Looks like Acquia went this route as well: https://www.drupal.org/node/2603724#comment-10601556

naveenvalecha’s picture

my two cents
https://www.ostraining.com/blog/drupal/drupal-8-contact-forms/

TL; DR

By larowlan

As a maintainer of contact in core and contact_storage in contrib I want to point out that there are some things that webform is better suited to - that contact module cannot do.

These are
* Forms with lots of fields. Each field is a config object and this can increase the size of your fieldmap, as well as the number of tables. Webform stores its data in a single table, Field API stores fields in one-table per field. There is obviously a storage and performance implication there (This is the same issue with entityform in D7).
* Multipage forms (although fieldgroup in contrib might power this for contact module)
* Lots and lots of forms. Each contact form is a new entity-type bundle so adding lots is equivalent to adding lots of content types. (This is the same issue with entityform in D7).

That said, we have plans to bring elements of contact_storage to core in an 8.x point release (e.g. 8.1, 8.2) - please join the conversation on https://www.drupal.org/node/2582955

I believe that the storage mechanism of webform module is different and would provides performance boost.

larowlan’s picture

Note what I said about field map, number of content-types etc might not be so big a problem on D8 because of work done in #2482295: Rebuilding field map with many bundles/fields is very slow

andypost’s picture

  1. +++ b/src/ComponentBase.php
    @@ -0,0 +1,305 @@
    +  protected $supports_disabled = FALSE;
    
    +++ b/src/Form/WebformSettingsForm.php
    @@ -47,14 +47,16 @@ class WebformSettingsForm extends ConfigFormBase {
    -        '#default_value' => $component['enabled'],
    +        '#default_value' => 1,//$component['enabled'],
    

    enabled should have default value in base plugin

  2. +++ b/src/Plugin/WebformComponent/TextField.php
    @@ -0,0 +1,79 @@
    +  protected $supports_unique = TRUE;
    ...
    +  protected $supports_disabled = TRUE;
    ...
    +  protected $supports_placeholder = TRUE;
    ...
    +  protected $supports_width = TRUE;
    

    Better to have getCapabilities()/defaultValues() method to return available options and their defaults

fenstrat’s picture

Status: Needs work » Closed (outdated)

Closing to clear out the old Webform 8.x-4.x branch. See #2827845: [roadmap] YAML Form 8.x-1.x to Webform 8.x-5.x.