Support from Acquia helps fund testing for Drupal Acquia logo

Comments

DamienMcKenna created an issue. See original summary.

DamienMcKenna’s picture

Status: Active » Needs review
FileSize
2.25 KB

This mirrors hook_panels_ipe_blocks_alter().

tim.plunkett’s picture

+++ b/panels_ipe/src/Controller/PanelsIPEPageController.php
@@ -166,12 +166,12 @@ public function getLayouts($panels_storage_type, $panels_storage_id) {
+    $definitions = $this->layoutPluginManager->getDefinitions();

@@ -180,8 +180,12 @@ public function getLayouts($panels_storage_type, $panels_storage_id) {
+    \Drupal::moduleHandler()->alter('panels_ipe_layouts', $layouts);

Hm, so I envision that every different implementation of layouts will want a hook like this. But that would only make sense if you alter the definitions directly, not this meta version.

Also I could see adding a method to LayoutPluginManager that would be like ->getDefintionsForUseCase($use_case) (but a better name) where you would pass in "panels_ipe" and it would be passed along to an alter hook or event. (That should not block this IMO)

DamienMcKenna’s picture

japerry’s picture

DamienMcKenna’s picture

FileSize
2.28 KB
724 bytes

This reindexes the blocks after they're altered, per #2767087.

dsnopek’s picture

It'd be really great if some of the context in which the IPE was being used was passed to this alter hook. Using even just the PanelsDisplayVariant it's possible to learn a lot! For example, you can include some Panelizer specific code in your alter hook to see what content type is currently being edited.

hideaway’s picture

Patch has a small error most likely due to copy-paste from similar issue :)
this:

$blocks = array_values($blocks);

should be:

$layouts = array_values($layouts);
DamienMcKenna’s picture

*cleans his glasses again*

Thanks @hideaway.

DamienMcKenna’s picture

DamienMcKenna’s picture

Talking through this some more with tim.plunkett, is this still really needed? The layouts provided by core could be intentionally kept somewhat limited to make them usable universally, while people building custom layouts for their own sites or to bundle with their themes could just use a category name that mentions its intended use, e.g. "Full page; Columns: 2" for layouts intended for a full page which has two columns.

Chris Burge’s picture

@DamienMcKenna - Would this cover a use case where a site builder writes 2 or 3 custom layouts and wants to restrict users to only those layouts?

DamienMcKenna’s picture

@Chris Burge: Yes, that's the exact scenario I was working from.

hideaway’s picture

Status: Needs review » Needs work

The last submitted patch, 14: panels-n2849219-9-reroll-4.1.patch, failed testing.

Chris Burge’s picture

Version: 8.x-3.x-dev » 8.x-4.x-dev
Status: Needs work » Needs review

Updating version to 8.x-4.x-dev

dsnopek’s picture

Per my comment in #7, here's a new patch that also passes the PanelsDisplayVariant. This could allow you to, for example, change the layouts differently for two particular Panelizer content types.

These are the same changes as in #2886230: hook_panels_ipe_blocks_alter() should receive the PanelsDisplayVariant being editted for hook_panels_ipe_blocks_alter()

DamienMcKenna’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for the improvement @dsnopek.

Lets do this thing and go home.

samuel.mortenson’s picture

Status: Reviewed & tested by the community » Fixed

Committed! The review in #3 was never really addressed but I feel like that could be a core issue about plugins in general.

Status: Fixed » Closed (fixed)

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