For the integration of Variable into Configuration builder, I want to have a 'Variable' Form Builder element that allows me to drag and drop a Variable registered variable form into any configuration page. See video for example of this working: http://www.youtube.com/watch?v=aMRTqb5_jDo

To do this, I needed to make a hook_element_info() entry to 'variable_form_element' as well as a theme item.

Patch attached in first comment, hope you consider this, otherwise I will be forced to do it in my own module which makes little sense and it wouldn't allow others to potentially leverage the element.

Configuration builder issue can be found at: #1686800: Ability to add form items for variables defined by the variables module

CommentFileSizeAuthor
#8 variable_form_element_element-1688586-8.patch1.06 KBDeciphered
#1 variable_form_element_element-1688586-1.patch1.6 KBDeciphered
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Deciphered’s picture

Deciphered CreditAttribution: Deciphered commented
FileSize
variable_form_element_element-1688586-1.patch1.6 KB

Patch

bforchhammer’s picture

bforchhammer CreditAttribution: bforchhammer commented
Version: 7.x-2.1 » 7.x-2.x-dev

Wow, this looks really cool :)

Is there a companion patch for the configuration builder module? I'm wondering how patch #1 is used...

Also note that the sub-module variable_realm makes it possible to store different values for different realms (e.g. languages, domains, ...). Here's a screenshot how this is currently handled on forms supported by the variable module. Ideally this should also be supported by the "configuration builder" module somehow... :)

Deciphered’s picture

Deciphered CreditAttribution: Deciphered commented

There's still a few issues to iron out in for the Configuration builder side of things, but I will update the referenced issue with a patch or notice of the commit when it's ready.

The patch here is simply to allow for a 'variable_form_element' to be used, which is the element seen in the video. To use it you would simply add something like the following to a FAPI form:

$form['variable'] = array(
  '#type' => 'variable_form_element',
  '#variable' => 'site_name',
);

And Variable will take care of making sure the correct FAPI form is rendered based on the chosen variable.

Configuration builder just uses system_settings_form() for the submissing of all configuration pages, so if Variable works with that it should work, if not, then I'm definitely open to suggestions.

bforchhammer’s picture

bforchhammer CreditAttribution: bforchhammer commented
Status: Active » Needs review

Thanks for the quick explanation, makes sense. Patch looks fine to me, let's see what Jose says...

Configuration builder just uses system_settings_form() for the submission of all configuration pages, so if Variable works with that it should work, if not, then I'm definitely open to suggestions.

Yes, that's what variable works with so it looks like this should just work :)

Deciphered’s picture

Deciphered CreditAttribution: Deciphered commented
Status: Needs review » Needs work

Marking this patch as needs work, it's not quite right yet. Primarily, the field ID doesn't get set.

Jose Reyero’s picture

Jose Reyero CreditAttribution: Jose Reyero commented

I am all for providing any API Config builder needs to work together.

However about this issue I don't really understand why we need a variable_form_element that renders a different variable widget (is that it?). I mean this is 'form widgets sourcery' that IMO belongs to config_builder module. (Unless it provides a feature that is reusable for other modules that config_builder, that I don't see.)

I think Variable should focus on providing variable lists / definitions / form elements that Config builder may need

Anyway though I don't understand this belongs to Variable if you guys insist on it, then ok, it can be here...

Btw, please take a look at my first 'integration try' (you both have commit access to this sandbox), see
http://drupal.org/sandbox/reyero/1689632

Deciphered’s picture

Deciphered CreditAttribution: Deciphered commented

I would have to say that it makes sense to be in Variable as it on it's own could be very useful to other modules, the ability to render a Site Name widget exactly as defined with just the code in #3 alone should be enough reason. All the logic as already in place, it's just a case of reregistering and rendering the element.

However, as I need the code to make Configuration builder work with Variable I would be happy to put it in Configuration builder, that would just mean that if anything wants to use the Variable based element they will first need to install my module and use an element named 'confiig_builder_variable_form_element'.

Deciphered’s picture

Deciphered CreditAttribution: Deciphered commented
Status: Needs work » Needs review
FileSize
variable_form_element_element-1688586-8.patch1.06 KB

Updated patch, much better this time, I'm not sure why I couldn't get the #process approach working the first time around (and I did try) but it's working perfectly now.

mstrelan’s picture

mstrelan CreditAttribution: mstrelan commented

Now it works like the video when using config_builder from GitHub. I am getting these PHP notices though, not sure that they were there before.

Notice: Undefined index: name in variable_include() (line 476 of /home/abc/public_html/sites/all/modules/variable/variable.module).
Notice: Undefined index: name in variable_include() (line 480 of /home/abc/public_html/sites/all/modules/variable/variable.module).
Notice: Undefined index: name in variable_children() (line 81 of /home/abc/public_html/sites/all/modules/variable/variable.module).

Also when I first drag the variable element across it just shows an empty rectangle but IIRC the video shows something like "No variable selected".

Deciphered’s picture

Deciphered CreditAttribution: Deciphered commented
Status: Needs review » Needs work

Those issues are fixed in an uncommitted version of Config Builder (variable branch), not related to this issue.

The empty variable bit is part of this patch though, it's something that I forgot, I'll update the patch shortly.

Deciphered’s picture

Deciphered CreditAttribution: Deciphered commented
Status: Needs work » Needs review

Marking this back to needs review, opted to go with an alternative approach where a default #variable is chosen when placing the Form Builder element, which means that the message is no longer needed.

Jose Reyero’s picture

Jose Reyero CreditAttribution: Jose Reyero commented

Sorry but I still don't get it. I mean how reusable is a widget that lets you select a variable and then changes the widget itself.

Really this should be in config_builder. You don't really need a long name, 'variable_form_element' should be fine, if you look at other modules defining form element types I don't think the use the module name prefix all the time.

Deciphered’s picture

Deciphered CreditAttribution: Deciphered commented
Status: Needs review » Closed (won't fix)

It's your call to make, but I absolutely think it would be a useful addition to your module. If I want to make a standard FAPI form and have any variable in it's defined form without having to define that form myself, this would be the solution.

But as it's a requirement of the Configuration builder module I have little other choice but to define it myself.

Jose Reyero’s picture

Jose Reyero CreditAttribution: Jose Reyero commented

If you want to add a variable element to your form, you just need to.

$form += variable_edit_subform('my_variable_name');

Really this 'magical form element' is a bad idea for building regular forms, though I can see it may be useful for config_builder or other modules that create form widgets. I don't think it will work for regular forms since building it like that you are missing the form element 'type' the variable should have.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented