The goal of this issue is to improve the Panels API so that developers can easily:

  • Create a new Panels display
  • Export a Panels display to an array for storage somewhere (ie. allowing Panelizer to store in a field)
  • Import a Panels display from an array (validating against a schema)
  • Change configuration on the Panels display, ie. set layout, place blocks, etc
  • Render the Panels display

This is an alternative approach to solving the same problem as #2626944: Create Panels value object which can be serialized/unserialized with a schema to validate it

Basically, the difference is that #2626944 makes a new value object, whereas the goal of this patch is to continue working directly with PanelsDisplayVariant objects but improving the API in an incremental way, and adding a PanelsDisplayManager service for creating, exporting and importing

However, the goal of having an easy to use Panels API to support modules like Panelizer, Mini Panels, etc is the same.

Here's some example code using this patch:

$panels_display_manager = \Drupal::service('panels.display_manager');

// Create and configure a new Panels display.
$panels_display = $panels_display_manager->createDisplay();
$panels_display->setLayout('twocol');
$panels_display->addSelectionCondition([
  'id' => 'node_type',
  'bundles' => [
    'page' => 'page',
  ],
  'context_mapping' => [
    'node' => 'node',
  ],
]);
$panels_display->addBlock([
  'id' => 'entity_view:node',
  'label' => 'View the node',
  'provider' => 'page_manager',
  'label_display' => 'visible',
  'view_mode' => 'default',
]);

// Export the display for storage somewhere.
$config = $panels_display_manager->exportDisplay($panels_display);

// Import the display from storage (validating against the schema first).
$panels_display = $panels_display_manager->importDisplay($config, TRUE)
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dsnopek created an issue. See original summary.

dsnopek’s picture

dsnopek’s picture

Status: Active » Needs work
FileSize
5.71 KB

Here's my first pass at this. It really should add all the API improvements with block placements from #2626944: Create Panels value object which can be serialized/unserialized with a schema to validate it, but that'll have to wait until we kill (hopefully) the BlockVariantInterface and can do our own thing. It also doesn't add the schema or validate against it yet, but there's a @todo for where the validation should go. This should be enough to start rendering in my Panelizer WIP, though!

dsnopek’s picture

FileSize
6.38 KB
1.77 KB

Here's some stuff that should have been in the last patch, but got lost somehow!

dsnopek’s picture

Status: Needs work » Needs review
FileSize
13.67 KB

Here is a much overdue updated patch! It includes the schema for Panels display variants, and validates against it on import. There is an issue with the layout settings schema from layout_plugin, which depends on this issue to fix: #2646156: Define layout_plugin.settings typed config so config validates!

Unfortunately, that means the tests will fail on Drupal.org until there is a layout_plugin release. :-( Running the tests here anyway, because I want to make sure the failure is what I expect!

Status: Needs review » Needs work

The last submitted patch, 5: panels-display-manager-2631854-5.patch, failed testing.

dsnopek’s picture

FileSize
12.12 KB

The failure is what I expected! So, that's good at least. :-)

Also, some merge junk snuck into that patch -- here's a cleaner version.

dsnopek’s picture

Hrm. In practice, this also sort of depends on this core patch because otherwise some blocks will fail validation (including the CTools EntityField block): #2392057: Config schema fails to expand dynamic top-level types

However, the test case here passes - I think the temporary solution is just for Panelizer (or other modules that use this service) to just call importDisplay($config, FALSE) to prevent validation from running until it's fixed.

dsnopek’s picture

Issue summary: View changes

Updated the issue summary to make the goal, approach and outcome of this issue clearer.

dsnopek’s picture

Issue summary: View changes
dsnopek’s picture

Issue summary: View changes
EclipseGc’s picture

  1. +++ b/config/schema/panels.schema.yml
    @@ -0,0 +1,55 @@
    +panels.block_plugin.*:
    +  type: block.settings.[id]
    +  mapping:
    +    region:
    +      type: string
    +      label: 'Region'
    +    weight:
    +      type: integer
    +      label: 'Weight'
    +    uuid:
    +      type: string
    +      label: 'UUID'
    

    This should all get moved to ctools schema because we're using functionally the same thing there. #2646284: Move panels block plugin schema to CTools.

  2. +++ b/config/schema/panels.schema.yml
    @@ -0,0 +1,55 @@
    +    context_mapping:
    +      type: sequence
    +      label: 'Context assignments'
    +      sequence:
    +        - type: string
    

    Shouldn't need this. block.settings has it in core.

  3. +++ b/config/schema/panels.schema.yml
    @@ -0,0 +1,55 @@
    +    selection_logic:
    +      type: string
    +      label: 'Selection logic'
    +    selection_conditions:
    +      type: sequence
    +      label: 'Selection Conditions'
    +      sequence:
    +        - type: condition.plugin.[id]
    +          label: 'Selection Condition'
    

    This shouldn't have to exist. I realize that ctools BlockDisplayVariant has this in the default config, so we're going to want a ctools issue to fix this. #2646286: Remove selection logic/conditions from the BlockDisplayVariant

  4. +++ b/panels.services.yml
    @@ -2,6 +2,9 @@ services:
    +    class: Drupal\panels\PanelsDisplayManager
    

    I'd really like to not name things "Managers" which are not Plugin Managers.

  5. +++ b/src/PanelsDisplayManager.php
    @@ -0,0 +1,107 @@
    +  public function createDisplay() {
    

    I'd almost like to see this be "public function createDisplay($layout = 'onecol', $builder = 'standard') {" so that I can change this stuff during calling this method instead of having to call public methods on the returned object.

  6. +++ b/src/PanelsDisplayManager.php
    @@ -0,0 +1,107 @@
    +    // Currently, we don't do anything with version.
    +    unset($config['version']);
    ...
    +    $config['version'] = 1;
    

    Either we need to elaborate on this in the validate method, or we need to remove this.

  7. +++ b/src/Plugin/DisplayVariant/PanelsDisplayVariant.php
    @@ -148,6 +150,27 @@ class PanelsDisplayVariant extends BlockDisplayVariant {
    +    if ($builder instanceof DisplayBuilderInterface) {
    +      $this->builder = $builder;
    +      $this->configuration['builder'] = $builder->getPluginId();
    +    }
    +    else {
    +      $this->builder = NULL;
    +      $this->configuration['builder'] = $builder;
    

    since we're not typehinting the parameter here, this should probably be elseif (is_string($builder)) and throw an exception if the parameter is not a string or a DisplayBuilderInterface object.

  8. +++ b/src/Plugin/DisplayVariant/PanelsDisplayVariant.php
    @@ -161,6 +184,28 @@ class PanelsDisplayVariant extends BlockDisplayVariant {
    +    if ($layout instanceof LayoutInterface) {
    +      $this->layout = $layout;
    +      $this->configuration['layout'] = $layout->getPluginId();
    +      $this->configuration['layout_settings'] = $layout_settings;
    +    }
    +    else {
    +      $this->layout = NULL;
    +      $this->configuration['layout'] = $layout;
    +      $this->configuration['layout_settings'] = $layout_settings;
    +    }
    

    Same comment as the previous issue.

dsnopek’s picture

FileSize
12.94 KB
5.33 KB

Thanks!

#12.1/3: Per our discussion, @EclipseGC is going to open the issue for this!

#12.2: Removed it, and everything still validated. You were right!

#12.4: PanelsDisplayExporter? PanelsDisplayFactory? Both of those are inaccurate when looking at all the methods together - I'm leaving this as PanelsDisplayManager for the time being... But if someone can come up with a better name, I'd be fine to change it!

#12.5: Done! But I made the arguments use NULL as there default, so that later we can look up the actual default configured for the current site (like we do in D7).

#12.6: Ok! We don't actually need the version stuff for anything right now, and it'd be easy enough to put back in later when we do need it, so I've removed it. It was more there as an example of the type of things that we might do in importDisplay() and exportDisplay() to convert a Panels display to it's "canonical export format".

#12.7/8: Done!

dsnopek’s picture

I've posted a patch on #2646284: Move panels block plugin schema to CTools. that will allow us to do the following:

--- a/config/schema/panels.schema.yml
+++ b/config/schema/panels.schema.yml
@@ -1,34 +1,7 @@
-panels.block_plugin.*:
-  type: block.settings.[id]
-  mapping:
-    region:
-      type: string
-      label: 'Region'
-    weight:
-      type: integer
-      label: 'Weight'
-    uuid:
-      type: string
-      label: 'UUID'
-
 display_variant.plugin.panels_variant:
-  type: display_variant.plugin
+  type: ctools.block_display_variant
   label: 'Panels display variant'
   mapping:
-    selection_logic:
-      type: string
-      label: 'Selection logic'
-    selection_conditions:
-      type: sequence
-      label: 'Selection Conditions'
-      sequence:
-        - type: condition.plugin.[id]
-          label: 'Selection Condition'
-    blocks:
-      type: sequence
-      label: 'Blocks'
-      sequence:
-        - type: panels.block_plugin.[id]
     page_title:
       type: label
       label: 'Page title'

This should address #12.1 and make it so #12.3 isn't the responsibility of this patch. :-)

dsnopek’s picture

FileSize
12.36 KB
1 KB

Alright! Now that #2646284: Move panels block plugin schema to CTools. is in, I've updated this patch. That should address all the review from #12 - but we should hold off on merging this until after a CTools and Layout Plugin release, so that we don't break the tests. More review is definitely welcome, though!

dsnopek’s picture

Status: Needs work » Postponed

This will be affected by #2644024: Store builder and layout in plugin collections which I'm going to try and get a patch ready for in a little bit!

dsnopek’s picture

Status: Postponed » Needs work
FileSize
11.15 KB

Ok, I'm not going to postpone this on #2644024: Store builder and layout in plugin collections because that one isn't 100% essential for the -beta. Here's a straight re-roll for the most recent run of commits.

dsnopek’s picture

Removed some unnecessary stuff per discussion with EclipseGC on Hangouts.

dsnopek’s picture

Status: Needs work » Needs review

Let's throw this one at testbot and see what happnes! :-)

Status: Needs review » Needs work

The last submitted patch, 18: panels-display-manager-2631854-18.patch, failed testing.

dsnopek’s picture

Status: Needs work » Needs review
FileSize
10.3 KB
490 bytes

This was a simple conflict with #2558575: Provide simple UI for changing the layout which got committed last week. This patch should fix it!

japerry’s picture

Status: Needs review » Fixed

Looks good to me, added!

  • japerry committed 02e04af on 8.x-3.x authored by dsnopek
    Issue #2631854 by dsnopek, EclipseGc: Create PanelsDisplayManager and...

Status: Fixed » Closed (fixed)

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