For some reason, the "Items per block" setting in Views is never set when adding/updating a Block using the IPE. Debugging shows that the value is passed to the normal Block validation/submit handlers, but we must be missing something else. Looking at what Page Manager has done with Block forms may help us come up with a solution.

CommentFileSizeAuthor
#4 panels-ipe-block-config-fix-2636530-4.patch4.16 KBdsnopek
#2 ipe-block-config-fix-2636530-2.patch4.11 KBsamuel.mortenson

Comments

samuel.mortenson created an issue. See original summary.

samuel.mortenson’s picture

samuel.mortenson
he/him
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new ipe-block-config-fix-2636530-2.patch4.11 KB

We were passing the entire FormState to the Block Plugin's validate/submit handlers, when we should have been using #tree and passing only the relevant settings. This patch copies the same patterns Page Manager uses in its Block Plugin forms, and seems to fix this (and other) form bugs in the IPE.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Status: Needs review » Needs work

Patch works in manual testing! However, I have some minor code review:

  1. +++ b/panels_ipe/src/Form/PanelsIPEBlockPluginForm.php
@@ -213,7 +215,11 @@ class PanelsIPEBlockPluginForm extends FormBase {
+    $settings = (new FormState())->setValues($form_state->getValue('settings'));
+    $block_instance->validateConfigurationForm($form, $settings);
+    // Update the original form values.
+    $form_state->setValue('settings', $settings->getValues());

    Could we call $settings something with '_form_state' in it, like $block_form_state, $plugin_form_state or $settings_form_state? That'll make it clearer that it's a FormState object, and not a settings array. 

  2. +++ b/panels_ipe/src/Form/PanelsIPEBlockPluginForm.php
@@ -222,8 +228,11 @@ class PanelsIPEBlockPluginForm extends FormBase {
+    // Submit the block configuration form.
+    $settings = (new FormState())->setValues($form_state->getValue('settings'));
+    $block_instance->submitConfigurationForm($form, $settings);
+    // Update the original form values.
+    $form_state->setValue('settings', $settings->getValues());

    Same here. 

  3. +++ b/panels_ipe/src/Form/PanelsIPEBlockPluginForm.php
@@ -288,7 +297,10 @@ class PanelsIPEBlockPluginForm extends FormBase {
+    $settings = (new FormState())->setValues($form_state->getValue('settings'));
+    $block_instance->submitConfigurationForm($form, $settings);

    And one last time. :-)

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Status: Needs work » Needs review
StatusFileSize
new panels-ipe-block-config-fix-2636530-4.patch4.16 KB

Here's a new patch that switches to using $block_form_state per my own review. :-)

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Reviewed & tested by the community

Looks okay to me. RTBC pending Drupal CI.

samuel.mortenson’s picture

samuel.mortenson
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed, thanks all!

Status: Fixed » Closed (fixed)

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