Display layout twig not overridable.

Some maintainer feedback would be good. I am unable to override the ds-1col template for a specific node or entity type.

Thanks, I have it working now. What confused me is that in D8 you can select a DS layout for the default view mode, but for an overridden twig template to be applied, you need to be using a non-default view mode (and get the template name right, ignoring the suggestions in the UI).

Can you reupload the patch. For some reason the testbot isn't kicking in.

Tests are passing but I'm still afraid about this change...
You're now changing the suggestions but that means they conflict wit the suggestions at "_ds_field_ui_table_layouts_preview"
Could you also try to fix that for you?

It is related, if there is a bug we need to fix it at once.
The suggestions in the UI should match the suggestions in code which should match actual working suggestions.

I'm running into this problem for DS 3 and this approach enables to have suggestions again. I did not had this problem in DS 2.

I don't know if it is the policy of this module to fix first for the more recent version and then backport but I made a patch for DS 3.

I kept the layout id as I think is relevant for the template since the same template won't work for different layouts.

I'm extremely confused about all these theme suggestions stuff...
Latest patch is different than the one in #13.

There has been some changes concerning theme suggestions so I asked for more input.
Hopefully I'll get it soon.

For me I tested it manually very easily by installing a standard profile, enabling twig debugging and trying to use one of the overrides that are suggested.
I feel the reason is somehow the suggestions have to start with the base theme hook or they are ignored.

The latest patch is different because for 8.3.x the base theme hook is different from what it used to be.

If your policy is to change it first on DS 2 please change the branch of the issue.

I'm attaching the a new patch hoping to fix the tests.

I'm reasonably sure this change would be needed to work with Layout Plugin in 8.2.x, and I'm 100% sure it's needed for 8.3.x

The 'layout__' is what allows the 'base hook' portion to work, which is needed for preprocess and suggestions to work as expected.

See #2559825: Preprocess functions from base hooks not triggered for theme hooks not using the __ pattern

We should add a test for this.

So this is becoming confusing, I would have expected my test to fail to 8.x-3.x, but it doesn't.

However, the same test with #2860490: Invalid argument supplied for foreach() in ds_preprocess_ds_layout() line 366 of ds.module reverted fails. So I think we don't have problem anymore now in 8.x-3.x branch ?

Ok, I'm stupid, let me test something different :)

Ok, so when using a theme suggestion and then using layout--template.html.twig everything is fine .. hmm .. tricky upgrade path.
Patch using code from #17 + a test.

Actually, the test is lying, digging

ok, again, combination of #17 and a test.
But it will fail as the wrapper is not passed in .. very weird.

even with the latest dev (as today), after updating all my templates overwritten me appear as <>

Is there anything that I can verify to better understand what's the problem?

The layout__ prefix should NOT be needed, the 'base hook' => 'layout' added by LayoutPluginManager should be sufficient.
If there are more bugs around that, it's once again a problem in the Theme System itself :(

But it's worth using that as a workaround now, seems like there are still other problems here to resolve in the meantime.

Some debugging:

• without layout prefix/suggestion: both template_preprocess_layout() and ds_preprocess_ds_layout() are called, but the template is not switched
• with layout prefix/suggestion: only template_preprocess_layout() is called, but not ds_preprocess_ds_layout(), template is switched.

So if I do this in core, it works fine for say ds-1col--node.html.twig with current DS (so no prefix of layout or renames in template) ...

diff --git a/core/lib/Drupal/Core/Layout/LayoutPluginManager.php b/core/lib/Drupal/Core/Layout/LayoutPluginManager.php
index e11f105..d694d3c 100644
--- a/core/lib/Drupal/Core/Layout/LayoutPluginManager.php
+++ b/core/lib/Drupal/Core/Layout/LayoutPluginManager.php
@@ -150,7 +150,7 @@ public function getThemeImplementations() {
       if ($template = $definition->getTemplate()) {
         $hooks[$definition->getThemeHook()] = [
           'render element' => 'content',
-          'base hook' => 'layout',
+          //'base hook' => 'layout',
           'template' => $template,
           'path' => $definition->getTemplatePath(),
         ];

So this seems to be core bug. For testing, can anyone just comment out the 'base hook' line in LayoutPluginManager::getThemeImplementations()

I've tested with module and theme provided layouts, and some variations (e.g. just copy over ds-1col, or add ds-1col--node suggestion in theme) and everything works fine .. at least for DS layouts ..

Added patch over at https://www.drupal.org/node/1970534#comment-11995931 just for fun to see what breaks in core when that line is gone ..

Ok, I'll be honest, I'm completely stuck on this one. the suggestion works with layout plugin and 8.x-2.x so something is either different in layout_discover or in theme registry that breaks it.

Woops, still needs work.

For reference, the patch I'm using locally to try and figure out to get this working with core (again, move out 'base hook' from layout plugin manager and the suggestion is found ..)

+++ b/ds.module
@@ -94,6 +94,11 @@ function ds_theme_registry_alter(&$theme_registry) {
+      if ($theme_registry[$theme_hook]['type'] == 'theme_engine') {
+        unset($theme_registry[$theme_hook]['base hook']);
+      }

You can ignore that however

We might have a core patch available over at #2862683: 'base hook' key prevents template suggestions from working

With that patch, it means we can also remove following code in ds:

diff --git a/ds.module b/ds.module
index eab805f..fede342 100644
--- a/ds.module
+++ b/ds.module
@@ -87,12 +87,6 @@ function ds_theme_registry_alter(&$theme_registry) {
   // layout is using the Display Suite layout class.
   foreach ($theme_registry as $theme_hook => $info) {
     if (in_array($theme_hook, $layout_theme_hooks) || (!empty($info['base hook']) && in_array($info['base hook'], $layout_theme_hooks))) {
-
-      // @todo Remove once https://www.drupal.org/node/2861840 is resolved.
-      if (!in_array('template_preprocess_layout', $theme_registry[$theme_hook]['preprocess functions'])) {
-        $theme_registry[$theme_hook]['preprocess functions'][] = 'template_preprocess_layout';
-      }
-
       $theme_registry[$theme_hook]['preprocess functions'][] = 'ds_preprocess_ds_layout';
     }
   }

Let's see what this gives, insane workaround which made the suggestion work on my machine.

some cleanups in the docs

Added a check to be sure the hook is registered, although it would be weird, but I want to be sure at this point.

Ok, so interestingly enough, it seems that there's still a problem with themes that provide templates.

This was the latest change which made the patch fail:

+      // Ignored if not registered (which would be weird).
+      if (!isset($theme_registry[$hook])) {
+        continue;
+      }

Actually, I forgot to include a file in the latest patch, trying again.

Committed and pushed, beta1 will be up soon

As issue #2862683: 'base hook' key prevents template suggestions from working mentioned on #38 has been postponed, does this commit still require it?

I'm assuming yes as in my scenario I'm still affected by the problem (layout suggestion template file is not loaded, loading always the base layout template file).

Can we add a bit of context or try to understand why #2862683 has been postponed?
This is a quite annoying bug as website working on 8.2 are forced to update to 8.3 to get security patches and when they do any module layout-related stops working.

The reason why my suggestion wasn't loaded is because the workaround introduced by #39 process all LayoutDefinition[], but adds to the theme registry only suggestion of layouts implementing class: '\Drupal\ds\Plugin\DsLayout'.
Just adding the property to the layouts yaml definition and the suggestion will be found.

But I'm assuming this invalidate all the idea of creating a layout and being able to reuse it i.e. in DS and Panels, right?

And from the DS point of view having a generalised (on ds_theme_registry_alter() on ds.module):

is_a($info->getClass(), \Drupal\Core\Layout\LayoutDefault::class, TRUE)

instead of

is_a($info->getClass(), DsLayout::class, TRUE)

Is may be too much.

thanx @gambry in #49.

Adding class: '\Drupal\ds\Plugin\DsLayout' to my layouts.yml indeed did the trick.
And since I'm only using it for ds (and not with panels) everything is fine to me.

The proposed change to
is_a($info->getClass(), \Drupal\Core\Layout\LayoutDefault::class, TRUE)
also worked, but I prefer to change my modules yml instead of patching ds...

Here's a follow up and a patch based on gambry previous comment: #2887778: Layout template suggestions does not always work