Problem/Motivation

For the sake of simplicity of altering the add/edit block forms, the form ids should be unified and set in the base class instead of the edit/add form class. Besides that it would be helpful to have page and display variant as whole objects in the form state available.

An example of where this will be useful can be found here: #2509142: Add support for page manager

Proposed resolution

Move getFormId() from sub classes to parent class DisplayVariantConfigureBlockFormBase and add page and display variant to form state in the buildForm() method of that class.

CommentFileSizeAuthor
#6 unify-block-form-ids-2574871-6.interdiff.txt2.69 KBs_leu
#6 unify-block-form-ids-2574871-6.patch4.11 KBs_leu
#2 unify-block-form-ids-2574871-1.patch2.95 KBs_leu

Comments

s_leu created an issue. See original summary.

s_leu’s picture

s_leu commented
Status: Active » Needs review
StatusFileSize
new unify-block-form-ids-2574871-1.patch2.95 KB

Here's a first patch.

Status: Needs review » Needs work

The last submitted patch, 2: unify-block-form-ids-2574871-1.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented 
  1. +++ b/src/Form/DisplayVariantConfigureBlockFormBase.php
@@ -64,11 +64,19 @@ abstract class DisplayVariantConfigureBlockFormBase extends FormBase {
+    return 'page_manager_display_variant_block_form';

    Why not just use BaseFormIdInterface instead? 

  2. +++ b/src/Form/DisplayVariantConfigureBlockFormBase.php
@@ -64,11 +64,19 @@ abstract class DisplayVariantConfigureBlockFormBase extends FormBase {
+    $form_state->set('page', $page);
...
+    $form_state->set('display_variant', $page->getVariant($display_variant_id));

    These are worrisome to me, we're just asking for trouble with serialization.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

I think we already end up caching those objects as they are passed in to the form? At least the page...

s_leu’s picture

s_leu commented
Status: Needs work » Needs review
StatusFileSize
new unify-block-form-ids-2574871-6.patch4.11 KB
new unify-block-form-ids-2574871-6.interdiff.txt2.69 KB

Applied change as mentioned in 1. of #4. Didn't apply change suggested in 2. of #4 due to comment in #5

japerry’s picture

japerry
Location KOMK
commented
Status: Needs review » Closed (outdated)

Page Manager 8.x-1.x is no longer supported, closing.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.