The code in plugins/display-edit.inc takes good care of integrating the 'settings form' callback form for custom layout plugins and handling its submission also with the 'settings submit' callback. However, these form functions are not used directly, they are used via panels_ui_layout.class.php, which does invoke the form builder from display-edit.inc to build in custom elements to the form from layout plugins but will never invoke the submission handler as part of its submission handling. It merely invokes the generic parent submission handler in ctools_export_ui.class.php and a tiny bit of modifications on its own.

The result is that the submission handler specified for the layout plugin (as well as other submission logic implemented for panels layouts) is never invoked.

Found while working on a pretty early version of http://drupal.org/project/layout. Without this patch, the layouts will not be saved properly there, because the submission process is not integrated form the layout (builder) plugin.

CommentFileSizeAuthor
layout-settings-submit.patch1.11 KBGábor Hojtsy
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sdboyer’s picture

sdboyer CreditAttribution: sdboyer commented

hey nifty, didn't see you guys had started up that project. glad to see it.

it does rather seem like that should be there. right now the flexible layout defines a callback for 'settings submit', but doesn't actually have the corresponding function. hmm. i guess it's adequate for flexible to simply have the layout settings array stored on the saved item - i guess you need more? that doesn't seem unreasonable, though it could result in some funky doubling of data, as the assumption is that the data in $display->layout_settings is enough.

in any case, this *seems* ok to me, without having tested. i'll poke at it later today, if merlin doesn't have more immediate thoughts to share.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Yeah, the flexible layout has the form function and the submit function provided in the plugin structure but it does not define those functions. What it does is that it updates the layout through AJAX callbacks, so it does not actually modify the form in any way. It probably used to do it, but it does not do it anymore. Not sure if there is any use case in panels itself where this is used, which explains the bug.

Currently as I found at least, the custom form submission callbacks are not invoked without the patch (as traced down from creating a custom layout builder and its form handling via the ctools export form handler that panels uses for builders).

sdboyer’s picture

sdboyer CreditAttribution: sdboyer commented

my, how i love cruft.

i'll give it a look over later today for a basic sanity check.

mattmo’s picture

mattmo CreditAttribution: mattmo commented

Interesting project you guys are working on here:).

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

Isn't there also a corresponding validate?

sdboyer’s picture

sdboyer CreditAttribution: sdboyer commented

@merlinofchaos - you'd think, right? one thing i intend to check on later.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
Status: Needs review » Fixed

Okay, this looks mostly fine. I added a corresponding validate hook, committed and pushed. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

  • Commit c1190e1 on 7.x-3.x, 7.x-3.x-i18n, 8.x-3.x by merlinofchaos: 
    Issue #1676890 by Gbor Hojtsy: Layout form needs to honor custom layout...
Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location Newburgh, NY
CreditAttribution: Jon Pugh commented
Issue summary: View changes
Status: Closed (fixed) » Needs work

I'm not sure this fixed the bug in Layout.module. I still cannot modify a layout once it has been created.

Adding reference issues for #2103473: Cannot save layout regions and #2103549: Resizing region does not work.

The issue author mentioned in #2103473: Cannot save layout regions that it does work in spark and panopoly, so I'll start investigating what might be missing that's in those distros.

Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location Newburgh, NY
CreditAttribution: Jon Pugh commented
Related issues: +#2103473: Cannot save layout regions, +#2103549: Resizing region does not work
iwyg’s picture

iwyg CreditAttribution: iwyg commented

Ran across this issue using panels 7.x-3.7 and the layout module (with ctools version 1.10). After a bit of digging I found that the 'settings submit‘ callback of the layout module’s 'Responsive‘ plugin doesn’t get invoked by `panels_edit_display_settings_form_submit()` in 'includes/display-edit.inc‘. Since panels basically uses `ctools_load_plugins()` to discover plugin callbacks, the callback will never be loaded because it passes 'layout‘ instead of 'layouts‘ as the plugin-type argument.

Can anyone confirm this as a possible solution?

Kind regards

  • merlinofchaos committed c1190e1 on 8.x-4.x 
    Issue #1676890 by Gbor Hojtsy: Layout form needs to honor custom layout...