Comments

merlinofchaos’s picture

Status: Active » Postponed (maintainer needs more info)

I don't actually know why panels_breadcrumbs doesn't work with panelizer. Maybe it's because panels breadcrumbs assumes that the handler is the standard panel_context handler, which it is not when using panelizer. I'd think this would be an issue for the panels breadcrumbs module?

broeker’s picture

I tried setting this up using both Custom Breadcrumbs for Panels and just regular Custom Breadcrumbs and could not get it working. Here was my general process:

1) Create a new Panelizer Variant for each content type (e.g News and Blog)

2) Set up Custom Breadcrumbs (tried doing this via "enable breadcrumbs" in Panels and also by using the Panels option within Custom Breadrcrumbs)

3) In both cases, the breadcrumbs themselves work fine, e.g.

about/news/my-news-title
about/blog/my-blog-title

4. However, in practice, Drupal is only rendering whichever Panelizer variant is FIRST in my list, so for example if my News variant is first then the breadcrumbs will display as:

about/news/my-news-title

even if I am on a blog variant, in which case it displays:

about/news/my-blog-title

Any thoughts on how to overcome this one?

minneapolisdan’s picture

This sounds like a good start, hopefully someone can chime in with some help

andypost’s picture

Title: Breadcrumbs for nodes? » Panels Breadcrumbs for Panelizer
Project: Panelizer (obsolete) » Panels Breadcrumbs
Version: 7.x-3.x-dev » 7.x-1.6
Status: Postponed (maintainer needs more info) » Active

@devuo please comment how to add Breadcrumbs tab for panelizer. Is it possible at all?

devuo’s picture

Never used panelizer module. I'm planning a full rewrite of panels breadcrumbs for a 2.x, I might do that this weekend, or the during the next week. I will look into the panelizer issue.

populist’s picture

+1 on getting support for Panelizer in Panels Breadcrumbs! I am happy to help test patches to go into Panopoly (which uses both).

populist’s picture

Also check out this issue #1418598: Set page_manager_get_current_page() before the page render for a patch that can help make this a reality.

ygerasimov’s picture

Project: Panels Breadcrumbs » Panelizer (obsolete)
Version: 7.x-1.6 » 7.x-3.0-rc1
Category: support » task

I think the problem is in both Panelizer and Panes Breadcrumbs.

First of all Panels Breadcrumbs injects its form to panels via hook_page_manager_variant_operations_alter() and Panelizer never uses this hook.

Panelizer's logic is quite hardcoded in PanelizerEntityDefault::hook_menu() -- for menu items and PanelizerEntityDefault::make_fake_tabs() -- for tab links. We could of course in Panels Breadcrumbs alter node's panelizer plugin and have own class inherited from PanelizerEntityNode and overrite logic there, but I don't think it is right approach. We should be able to extend behavior of the Panelizer with some kind of hooks.

As this is really the case where Panelizer should be changed first -- moving issue to Panelizer module.

devuo’s picture

Indeed ygerasimov, I've looked deeply into panelizer's code, and for a module that uses so many hooks, it provides just two undocumented _alter functions, hook_panelizer_entity_plugin_process_alter() and hook_panelizer_default_types_alter(), the last useless, as it lies within the settings_form() method of PanelizerEntityDefault class which is then overriden by the PanelizerEntityNode.

While adding the breadcrumb configuration route to match the route for each panelizer entity, with a re-implementation of the code in PanelizerEntityDefault::hook_menu() with the assistance of panelizer_get_entity_plugins() in Panels Breadcrumbs is doable with an ugly implementation of hook_menu(), adding a "Breadcrumb" tab is impossible, given that the third-level tabs as you correctly pointed are hardcoded into PanelizerEntityDefault::make_fake_tabs() with no hook provided to make changes.

Right now, Panelizer is quite restricting in terms of modifications that can be done to its behavior. So yes, before Panels Breadcrumbs is able to integrate with Panelizer, further hooks must be provided by Panelizer.

merlinofchaos’s picture

Status: Active » Needs review
StatusFileSize
new1.03 KB

Is this sufficient?

If it is, documentation will need to be added to the readme, I would guess.

merlinofchaos’s picture

I think the overview one needs a bit more information in the alter. Try this:

merlinofchaos’s picture

Just to correct: The second alter in settings_form isn't overridden. The overriding settings_form() method calls to the parent and then just performs a warning. However it's not useful at all in this situation. But the alter isn't useless. :)

ygerasimov’s picture

@merlinofchaos thank you very much for the patch. I am working on the integration at the moment. I cannot tell before it is done what changes are needed. Lets postpone the patch to panelizer till I get full functionality to work.

devuo’s picture

drupal_alter() allows only up to two contexts, the attached patch fixes the patch in #12 by wrapping the extra contexts in an array and passing that array to drupal_alter().

andypost’s picture

+++ b/plugins/entity/PanelizerEntityDefault.class.phpundefined
@@ -1290,6 +1290,17 @@ abstract class PanelizerEntityDefault implements PanelizerEntityInterface {
+        'base_url' => $base_url,

@@ -1384,6 +1395,15 @@ abstract class PanelizerEntityDefault implements PanelizerEntityInterface {
+      'base_url' => $base_url

I think this useless

+++ b/plugins/entity/PanelizerEntityDefault.class.phpundefined
@@ -1384,6 +1395,15 @@ abstract class PanelizerEntityDefault implements PanelizerEntityInterface {
+    ¶

trailing whitespace

merlinofchaos’s picture

No I think the $base_url is needed since the alter will have to actually add the link which is formed using the base_url. andypost is right on that one, I think.

merlinofchaos’s picture

er, devuo is right!

devuo’s picture

$base_url is indeed needed, I've implemented a partially working solution on panels breadcrumbs and the $base_url is required to build the correct route for each entity type.

ygerasimov’s picture

StatusFileSize
new9.16 KB
new5.1 KB

I have managed to integrate panels_breadcrumbs with panelizer. Here is first try of patches. These patches do not use previous patches in this issue.

The idea is that panelizers PanelizerEntityDefault will have operations() method that provides information about all operations (settings) available. We also allow to alter that list so panels_breadcrumbs can add its settings form. Using magic __call() method we execute other module's operation callbacks. Another important change is that panels_breadcrumbs module changes the schema of panelizer_entity table to keep its settings. Personally I don't quite sure if its best way to go as it would be better to have some big text field where we can store all settings as serialized string for example.

ygerasimov’s picture

Added update procedure for panels_breadcrumbs module so functionality will be applied for currently installed systems.

Patch for panelizer is kept the same.

populist’s picture

Status: Needs review » Needs work

I did a review of this patch today and have some feedback:

1.) Apply to Default Panelizer Setups - It would be really helpful if this functionality could be applied to the default panelizer configurations through an operation callback like admin/config/content/panelizer/node/NODE_TYPE.page_manager/breadcrumbs. This might be just an extension of the existing logic in _menu() for adding the links and then some generalization of the configuration form.

2.) Restrict Available on Non "Full Page Overrides" - The option for breadcrumbs seems to show up on "Default", "Teaser", and other custom view modes. There might be use cases I can't think of now, but don't think setting the breadcrumb through a teaser is desirable! Probably should just be on Full Page Override. This would also solve an issue I saw where panels_breadcrumbs_entity_view() will fail when $entity->panelizer[$view_mode] isn't set properly.

3.) Handle Cases Where Rendered Entity is Never Fully Rendered - The breadcrumb setting seems to depend on hook_entity_view() which is only fired when Panelizer has a full node content pane present. However, if you are using Panelizer and only showing off various fields that hook is never run!

4.) Setting Storage - It also might be worth thinking through the way the settings are stored. Panels has a pattern that has a longtext variable storing information (i.e. layout_settings, panel_settings) that might be able to be extended to entity_settings.

4.) Expand

ygerasimov’s picture

@populist, thank you very much for the review. There is definitively place for improvement. I will work on the patch and will revert.

Meanwhile what does mean you last point "4.) Expand" ?

populist’s picture

4.) Expand is just my mistake. Sorry!

ygerasimov’s picture

Status: Needs work » Needs review

@populist, I have reviewed your comments once again and the code changes I proposed.

Regarding "Apply to Default Panelizer Setups" this is nice feature, but I think it should be implemented as separate issue.

For the point no 2 and 3, I think it is not good idea to restrict teaser or any other view modes. Generally we do not know what view_mode is full page override for custom entities. Sure we can assume that "teaser" should be restricted, but I don't think idea of making exceptions is good.

Regarding the storage. The thing is panelizer has its settings separately from panels. I think I miss this point and don't understand what do you mean by "entity_settings".

populist’s picture

1.) I think "Apply to Default Panelizer Setups" is a separate set of work, but I would be concerned that we introduce a new feature to Panelizer that doesn't work in the way that other Panelizer settings work which is to allow a default and allow it to be overridden. This might be confusing.

2.) If we allow the teaser to set the breadcrumb, doesn't that cause problems if *two* teasers are on the same page that fight over control? In my opinion, only "Full Page Overridden" can really be trusted to overriden the breadcrumb (since it is a page operation)

3.) I don't think this patch works in cases where you use "Full Page Overriden" in Panelizer and speciify individual fields (instead of a full render entity).

4.) By "entity_settings" I meant that we could add a new column to {panelizer_entity} that would have a serialized set of options. This is similar to how Panels stores settings with {display_settings} and {layout_settings}. This is not very important.

merlinofchaos’s picture

I agree with #1. We'll have to do it to the defaults as well as to specific entities, or you have this weird disconnect. That's just going to have to work that way.

2) Unfortunately I think this is going to need to be on a per entity basis. IMO there was a disconnect when view modes were introduced, that there isn't, in fact, a specific view mode for the full page view. It just uses the 'default' view mode, and the expectation was going to be that anything else would use the teaser or other view mode, but that's actually proven to be not entirely true.

4) Yes, panelizer can add a serialized blob field to both the panelizer_default and panelizer_entity tables for the use of modules that want to add data. Keeping data there would ensure that it exports properly and that's very important. I would definitely go with that approach.

merlinofchaos’s picture

Okay, after playing with this on and off for a few days, I've completely redone the Panelizer patch. Some notes:

1) The operations are moved to a generic function so that they can be used everywhere.
2) They are used for panelizer defaults as well as entity panelizers.
3) They are used for permissions -- adding an operation will automatically add permissions for that operation.
4) I didn't like the __call magic and removed it and replaced it with something simpler that still allows actual callbacks.
5) The original patch didn't translate and it also didn't respect capitalization.
6) I added docs.php to document the new hook.

I haven't redone the panels_breadcrumbs patch to compensate, other than to adjust the alter to satisfy myself that the pages actually show up. However, that patch needs to be compeltely redone, as panels_breadcrumbs should no longer be responsible for storage but instead that should be handled through panelizer. Both the admin (default) and entity versions have to be handled with slightly different chrome (they're saved differently) and that probably requires more documentation to get right.

To reiterate: This will NOT work with the breadcrumbs patch from above, and that will need to be redone before this can be fully tested. However, we CAN test to ensure that this does not break any existing functionality, since it redoes how the menu items are created.

ygerasimov’s picture

Status: Needs review » Needs work

Thank you for the patch. I will work on the panels breadcrumbs patch.

Found one bug:

+++ b/plugins/entity/PanelizerEntityDefault.class.phpundefined
@@ -304,37 +290,21 @@ abstract class PanelizerEntityDefault implements PanelizerEntityInterface {
+          if (isset($operation['file'])) {
+            $items[$this->plugin['entity path'] . '/panelizer/$view_mode/' . $path]['file'] = $operation['file'];
+          }
+          if (isset($operation['file path'])) {
+            $items[$this->plugin['entity path'] . '/panelizer/$view_mode/' . $path]['file path'] = $operation['file path'];

This should be '/panelizer/' . $view_mode . '/' instead of '/panelizer/$view_mode/'.

devuo’s picture

StatusFileSize
new5.81 KB

Wrote the new panels_breadcrumbs integration with panelizer using the new hook, hook_panelizer_operations_alter(), provided by merlin. It also requires the problem detected by ygerasimov in the patch to be fixed (see comment # 28, see # 30 for fix). As far as I can tell everything that is expected to work in panels_breadcrumbs, works.

This patch applies to the latest 7.x-2.x-dev version (b79f3a8cd8807a58561d2795cf1f2cfee518105c).

devuo’s picture

Here's the same patch to panelizer as provided by merlin in # 27 but with the fix to the problem reported by ygerasimov in # 28.

merlinofchaos’s picture

Status: Needs work » Needs review

I worry that hook_entity_view isn't the right place to act on panelizer data. For example, that may never be called using the 'page_manager' view mode.

Maybe we need to implement a hook in PanelizerEntityDefault::render_entity? I think that is the safest way to accomplish it so that panelizers can be acted on correctly, every time.

devuo’s picture

I concur merlin, so I added an hook_panelizer_pre_render_alter() to your patch and implemented it in panels_breadcrumbs.

populist’s picture

I did a review of these patches and see a few things on the Panels Breadcrumbs side:

1.) There seems to be some trouble (fatal errors) that occur in the new hook_panelizer_pre_render_alter() when $panelizer->entity_type is not being properly set. In my example site, I had ~3 different calls which didn't have that set. I rerolled the patch to only set the breadcrumbs when there actually is a known entity type, but assume this is a problem upstream.

2.) On new Panel Breadcrumbs form, I get a notice (Notice: Undefined property: stdClass::$extra in panels_breadcrumbs_panelizer_breadcrumbs_form()) because there is no $extra value set. I added a quick check here as well, but this could be improved.

3.) As part of the configuration, there isn't a check in panels_breadcrumbs_build_breadcrumb() to make sure the checkbox to "Enable Breadcrumbs" is actually checked. This usually isn't a problem since it exits if there isn't any breadcrumbs, but if there is and the box isn't checked then they show which is wierd. I fixed that by adding some logic to check for that.

Attached is a new patch that addresses those 3 things.

devuo’s picture

1.) That is weird, the variable PanelizerEntityDefault->entity_type should always be defined, at least from what I see in the code in PanelizerEntityDefault->init(). Can you describe the use case where this isn't true?

2.) Hum, the $extra should always be defined in the $panelizer, if anything its value should be NULL. I'm taken to believe this is somehow related to 1.)

3.) I had that check in 1.x. Somehow I removed it when I re-wrote panels_breadcrumbs in 2.x (fail)

devuo’s picture

Ok, 3.) wasn't a fail, I just forgot why I've done that. All the reported problems in 33 are fixed in the attached patch.

devuo’s picture

While I was at it I fixed the integration of panelizer with the render override of the breadcrumb content-type.

broeker’s picture

Both patches are applying cleanly (save some whitespace messages) and I've had success with everything I've tested so far -- thanks, it is awesome to see this working. I do have one logistical question:

On new nodes, breadcrumbs are now working out of the box. On existing nodes, I have to go into each individual node and do a "Reset/Save" within Panelizer settings. This works but I'm hoping to do this in a bulk operation for obvious reasons. Here's what I've tried so far:

a) I created a custom Views Bulk Operation view with some filters and the "Set panelizer status" -- this creates an Ajax 500 error and the screen is stuck on "Processing 0 of 7".

b) Then I tried creating my own "Advanced action" (Drupal core) again using "Set panelizer status" -- clicking "Create" causes a Server 500 error.

c) I then bit the bullet and un-panelized my content type completely -- first by trying "Reset to defaults" and then finally by unchecking the entire panelizer option on the main config screen. When I re-enabled and re-created my panelizer from scratch (ugh) everything else snapped back into place, but the breadcrumbs checkbox on the existing nodes remained unchecked.

So, is there a way to do this in bulk? Would option (a)/(b) above even work assuming no bugs? Is there some reason option (c) isn't working (I"m assuming the nodes are retaining settings even when the node type has been un-panelized)?

Thanks again . . .

merlinofchaos’s picture

Those 500 errors may well be bugs in the Views integration -- I'm not 100% sure how well that stuff got updated for view modes. Look in your logs and see what the 500 error is?

You can quickly and easily reset the panelizer status of a node by simply wiping its entry in the panelizer_entity table. Of course, that'll wipe all customizations, so you need to be careful.

Another option would be to take what's in the default extra field and apply it to the extra field on your existing panelizer_entity and that'll copy your breadcrumbs everywhere they need to go.

broeker’s picture

I'm just re-confirming that both of the most recent patches are still working flawlessly so far on a live site with numerous content types. I was able to solve my "Reset to default" problems by adjusting the "Number of entities to load at once" setting to "1" on my VBO bulk operations field settings (I don't think was was Panelizer related at all) -- this allows me to run "Set panelizer status" with no errors and it appears to reset all the nodes as intended.

populist’s picture

Here is a quick re-roll of the patch in #32 which works better with #1824566: Allow Panelizer Bundles To Use Panel Page's Content/Layouts as Default and #1772844: Allow More Granular Permissions in the IPE for drush make purposes.

populist’s picture

I have done some more testing here and there seems to be trouble when a user selects "No Extras" on their "Full Content" node. This, probably by design, doesn't execute the breadcrumbs code but that is a little wierd.

populist’s picture

I did some more digging and found an actual problem. The issue here is that when a Panelizer is coming from code, it uses the $panelizer->panelizer_type to desribe it's entity type instead of $panelizer->entity_type. While this sounds funky, this is how Panelizer already handles it in panelizer_panelizer_task_get_addressable().

The problem here is that Panels Breadcrumbs's patch in panels_breadcrumbs_panelizer_pre_render_alter() only checks for $panelizer->entity_type which *only* works once the entity has been overriden. This patch expands that logic to allow either $panelizer->entity_type or $panelizer->panelizer_type.

devuo’s picture

This is looking good.

broeker’s picture

Something in the latest DEV release of Panelizer is preventing the breadcrumbs UI from appearing on the Panelizer config page (admin/config/content/panelizer).

WORKING:

7.x-3.0-rc1+8-dev

A "breadcrumb" link appears (settings context layout content breadcrumbs) for each Full Page Override row, and I can correctly configure crumbs for each entity type.

NOT WORKING:

7.x-3.0-rc1+10-dev

The breadcrumb link is missing entirely (settings context layout content).

Both versions are using Panels Breadcrumbs 7.x-2.x-dev -- the working site has a slightly older copy, but it didn't seem to matter on the broken site which version was in play. The breadcrumbs admin link didn't appear for either version until Panelizer was downgraded to 7.x-3.0-rc1+8-dev. The working site is also currently running a slightly older version of Drupal core so there could be something else in play here but downgrading to an older version of Panelizer definitely fixes the problem.

UPDATE WITH MORE INFO:

Upon further review, downgraded to rc1+8 does cause the UI to re-appear, and I can enable/save breadcrumbs on entities, but they aren't actually rendering on the site. So, I'm not sure what is really in play here. The only other significant factor I can think of is that the broken site was originally (and mistakenly) running Panelizer 7.x-2.0. We upgraded to 7.x-3.0 with no apparent issues but perhaps that may be at the root of this?

devuo’s picture

I will try later today or tomorrow, the latest panelizer -dev and see if I can reproduce the problem.

merlinofchaos’s picture

Status: Needs review » Fixed

Hm. With the very latest -dev and the two most recent patches applied, I am seeing the breadcrumbs link.

Updating from Panelizer 2.x doesn't seem like it should be creating problems. I think the panelizer side of this is probably very close, and so I'm going to commit this; if we need to refine it, we can, but it will be a lot easier to work with this way. While I would not be surprised if we run into a small bug, I think it'll be a lot easier to cope with now.

Status: Fixed » Closed (fixed)

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

pyxio’s picture

Version: 7.x-3.0-rc1 » 7.x-3.x-dev
Category: task » bug

Greetings... Panels Breadcrumbs are not working for me. Is this patch supposed to be included in 7.x-3x-dev? I also tried 7.x-3.1 with no luck. Cheers Kevin

kclarkson’s picture

Title: Panels Breadcrumbs for Panelizer » Panels Breadcrumbs for Panelizer / Panopoly
Project: Panelizer (obsolete) » Panopoly
Version: 7.x-3.x-dev » 7.x-1.x-dev
Component: Code » Admin
Issue summary: View changes
Status: Closed (fixed) » Active

I am running the newest version of Panopoly and am not able to to get the custom breadcrumbs working.

While I am able to see the settings and create custom breadcrumbs but they are not actually showing on the Node Type that I enabled them one.

I tried in two areas:

Page Manager: Added another Panelizer context for the node type and then added the custom breadcrumbs their.

Panelizer: Changed the breadcrumbs on the Node Override in Panelizer.

kclarkson’s picture

Version: 7.x-1.x-dev » 7.x-1.1
dsnopek’s picture

Project: Panopoly » Panelizer (obsolete)
Version: 7.x-1.1 » 7.x-3.x-dev
Component: Admin » Code
Status: Active » Closed (fixed)

This is a closed Panelizerr issue. If you're still experiencing a problem under Panopoly, please create a new issue in our queue and link to this one if it's related. Thanks!