compatibility problem with drupal-8.3.0 and ds-8.x-3.x

We also just noticed this issue. This should use the base class of layout discovery to be compatible.

Possible fix attached.

Looks good and removes all initial errors.

I encountered the originally reported error yesterday after updating core to 8.3.0 and DS to version 3. The patch defined in #3 worked for me. Thanks much!

Yes this works on 8.3.0 with a recent zurb_foundation version. This is a pretty big deal.

I wish there was a way to set this up conditionally so that the theme code can work in both 8.2.x and 8.3.x correctly. It does work correctly so I think it should be committed.

I wish there was a way to set this up conditionally so that the theme code can work in both 8.2.x and 8.3.x correctly. It does work correctly so I think it should be committed.

While it should be possible to do by providing 2 base classes, that would be super awkward to do and maintain. It shouldn't matter however, since 8.2.x is no longer supported at the moment.

This is why I think this should be committed (and a new release should be tagged with this included. This makes the pain of upgrading a lot less.

@borisson_ fair enough but this will cause a problem for clients that are already on Drupal 8.2.x and if they try to update :(

You're right, let's make that explicit. Setting to NR, because I RTBC'd the issue.

The patch works for zurb foundation but when trying to install a subtheme the dependencies are null and the install fails at ThemeInstaller.php line 128

Removing system and layout_discovery dependencies in zurb_foundation.info.yml fixes the subtheme installation.

So reading #10 it looks like the solution provided by @Novitsh makes more sense. Hiding my patch.

Unfortunately this isn't as simple as changing the namespace, according to the documentation at https://www.drupal.org/node/2578731 :

Note: You can specify a custom 'class' with layouts registered in both modules and themes, BUT the class itself MUST be in a module. Themes can't contain autoloadable class files.

So while this patch may fix the problem for Display Suite users, we're not supposed to have the "FoundationLayout" class in the theme at all. I have existing Panels sites that use zurb_foundation layouts and they throw fatal errors like "Plugin (foundation_1col) instance class "Drupal\zurb_foundation\Plugin\Layout\FoundationLayout" does not exist", with or without this patch when running the 3.x branch of Panels.

To further validate the problem, it looks like having the layout class live in the theme has broken the Panels backend for users, even before layout_plugin was moved into core: #2846067: Panels error selecting layout.

The correct solution here would be to create a new contrib module containing Foundation layouts, and move the class and layouts there. While this is disruptive to existing users, which I don't like, it's what we should have done in the first place. I'll plan on doing this today and posting a patch to this issue.

Edit: It looks like a contrib module already exists for this, we just need to have an upgrade paths for our existing layouts to use https://www.drupal.org/project/foundation_layouts

I started on this but don't have an issue for the foundation_layout project yet. I'll put this issue in review when I have a patch removing the layouts from this theme.

I just filed #2871917: Adopt broken layouts from Zurb Foundation theme to move the theme's layouts to a module namespace, the upgrade path for existing users would be to:

1. Update to the latest release of Zurb Foundation (once this patch makes it into a release)
2. Install Foundation Layouts
3. Rebuild cache

As I mentioned in #12, this is a disruptive change but we should have never included a custom layout class in the first place, as theme classes are not autoloaded.

Missed the patch, as usual.

Another alternative to all this is to simply remove the "class" from each Layout Plugin, which the patch in #2871479: Upgrade from layout plugin to Layout API / Layout discovery does. I'd like to get feedback on both approaches if possible.

#2871917: Adopt broken layouts from Zurb Foundation theme to me sounds like the best approach since it offers some backwards compatibility and resolves the issue in the theme moving forward.

After working through this and a similar issue with a custom layout, my preference is the approach used in #2871479: Upgrade from layout plugin to Layout API / Layout discovery. In general, I see Layout as something that should be able to be defined in a theme. With Layout functionality in core (and hopefully increased use of layouts as a result), not shipping he theme with layouts seems like a missed opportunity.

Thanks for all the attention given to this issue. My question is, for right now, what is the recommended way of addressing this issue so I can update to Drupal 8.3.x? Should I apply #15 and #16? Is the patch in #3 not recommended?

Update: I attempted to apply the patch in #16 but it does not apply:

Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/zurb_foundation-movelayouts-2867688-14.patch

I did later apply the patch in https://www.drupal.org/node/2871479, which installs cleanly, but does not resolve the issue for me.

Error: Class 'Drupal\layout_plugin\Layout' not found in /var/www/staging/drupal/web/modules/contrib/ds/src/Ds.php on line 88 #0 /var/www/staging/drupal/web/modules/contrib/ds/ds.module(77): Drupal\ds\Ds::getLayouts()
#1 /var/www/staging/drupal/web/core/lib/Drupal/Core/Extension/ModuleHandler.php(501): ds_theme_registry_alter(Array, NULL, NULL)
#2 /var/www/staging/drupal/web/core/lib/Drupal/Core/Theme/Registry.php(365): Drupal\Core\Extension\ModuleHandler->alter('theme_registry', Array)
#3 /var/www/staging/drupal/web/core/lib/Drupal/Core/Theme/Registry.php(232): Drupal\Core\Theme\Registry->build()
#4 /var/www/staging/drupal/web/core/lib/Drupal/Core/Utility/ThemeRegistry.php(86): Drupal\Core\Theme\Registry->get()
#5 /var/www/staging/drupal/web/core/lib/Drupal/Core/Utility/ThemeRegistry.php(67): Drupal\Core\Utility\ThemeRegistry->initializeRegistry()
#6 /var/www/staging/drupal/web/core/lib/Drupal/Core/Theme/Registry.php(252): Drupal\Core\Utility\ThemeRegistry->__construct('theme_registry:...', O"

UPDATE 2: Stupid me, I hadn't updated to the latest Display Suite, I thought I had. It seems the problem is resolved.

I prefer the approach in #16 to the one in the other issue. However, #16 needs a reroll.

I popped the binary entries out of #16 patch and it applies cleanly now.

This patch removes the custom layouts that are packaged with the theme, and everything seems to be working cleanly. The following code in your composer.json will lock onto the file;

    "extra": {
        "installer-paths": {
            "web/core": ["type:drupal-core"],
            "web/libraries/{$name}": ["type:drupal-library"],
            "web/modules/contrib/{$name}": ["type:drupal-module"],
            "web/profiles/contrib/{$name}": ["type:drupal-profile"],
            "web/themes/contrib/{$name}": ["type:drupal-theme"],
            "drush/contrib/{$name}": ["type:drupal-drush"]
        },
	"patches": {
		"drupal/zurb_foundation": {
			"fix bad layout code removes preset layouts": "https://www.drupal.org/files/issues/zurb_foundation-movelayouts-2867688-24.patch"
		}
	}
    }

Follow this with

composer update zurb_foundation 

and you should see the patch get applied.

corrections to #24:

• composer update drupal/zurb_foundation (you missed the drupal/)
• before you can do that, you'll need to have done: "composer require cweagans/composer-patches" - this isn't obvious to those of us new to D8/composer :)
• at this time, the patch doesn't apply properly, either via composer or git apply

That said, manually deleting the files indicated in the patch does seem to resolve the issue.

Thanks!
Charles

@mausolos I am still getting the patch on 24 to apply against the current head, 78eea1f2ff58993075bb209e6fb737a02cd966f7 , is working correctly.

Thanks, the patch#24 worked for me too. Using D 8.4.0

Nice fix: removing all layouts instead of fixing compatibility issue.

#24 applies cleanly for me from within the theme directory, and fixed the Class 'Drupal\\layout_plugin\\Plugin\\Layout\\LayoutBase' not found error.

This bug completely breaks sites (WSOD), so fix needs to be applied. I'll vote for for this one b/c it's already RTBC.

I suspect the Display Suite changes also broke the fix for empty highlighted regions that was mentioned in #2418039-15: 8.x-5.x Roadmap.

In case anyone else encounters that, here's a new patch that also removes extra whitespace from an empty status messages block by way of twig whitespace control.

It would be great if the people at Drupalcon right now can come to a consensus about this issue. We can either keep our layout classes and follow the lead of Bootstrap theme with a 'heavier' theme, or we can remove our layouts ( #24 patch ) and put them in foundation_layouts. I am totally down to accept whatever a group decides at the con- there are good arguments to go either way. Thanks everyone!

Thanks @HongPong and @kentr

I successfully applied patch #31 to zurb foundation 8.x-6.0-alpha4 using Drupal 8.5.5 and white screen was averted.

This is a pretty big issue. Shouldn't this be rolled into a new release by now.

This is a big issue but we need to align this change with https://www.drupal.org/project/foundation_layouts/issues/2871917

A smooth transition to use the Foundation Layouts module should be possible so we should attempt it.

There is also the other issue that the Zurb library comes with various layout options across its releases (cells/rows/columns ) so we need to have this in mind when making changes here.

+1 for use module "foundation_layouts" , => This gives the sitebuilder the choice whether to use it or not ...

Here is the patch for last dev (dev-6.x 7b2c156)

HI SteveOriol I tried applying this against current head and it applies cleanly. Thank you, i would like to commit this, in alignment with issue here #2871917: Adopt broken layouts from Zurb Foundation theme...

Still having the issue after zurb_foundation-removelayouts-2867688-36.patch

Fatal error: Class 'Drupal\layout_plugin\Plugin\Layout\LayoutBase' not found in /mnt/www/html/myweb/docroot/themes/contrib/zurb_foundation/src/Plugin/Layout/FoundationLayout.php on line 16

$ curl -O https://www.drupal.org/files/issues/2018-08-01/zurb_foundation-removelay...
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 19481 100 19481 0 0 224k 0 --:--:-- --:--:-- --:--:-- 226k

$ patch -p1 < zurb_foundation-removelayouts-2867688-36.patch
The next patch would delete the file layouts/foundation_1col/foundation-1col.html.twig,
which does not exist! Assume -R? [n] y
patching file layouts/foundation_1col/foundation-1col.html.twig
The next patch would delete the file layouts/foundation_2col/foundation-2col.html.twig,
which does not exist! Assume -R? [n] y
patching file layouts/foundation_2col/foundation-2col.html.twig
The next patch would delete the file layouts/foundation_2col_bricks/foundation-2col-bricks.html.twig,
which does not exist! Assume -R? [n] y
patching file layouts/foundation_2col_bricks/foundation-2col-bricks.html.twig
The next patch would delete the file layouts/foundation_2col_stacked/foundation-2col-stacked.html.twig,
which does not exist! Assume -R? [n] y
patching file layouts/foundation_2col_stacked/foundation-2col-stacked.html.twig
The next patch would delete the file layouts/foundation_3col/foundation-3col.html.twig,
which does not exist! Assume -R? [n] y
patching file layouts/foundation_3col/foundation-3col.html.twig
The next patch would delete the file layouts/foundation_3col_stacked/foundation-3col-stacked.html.twig,
which does not exist! Assume -R? [n] y
patching file layouts/foundation_3col_stacked/foundation-3col-stacked.html.twig
The next patch would delete the file layouts/foundation_4col/foundation-4col.html.twig,
which does not exist! Assume -R? [n] y
patching file layouts/foundation_4col/foundation-4col.html.twig
The next patch would delete the file layouts/foundation_6row_combo/foundation-6row-combo.html.twig,
which does not exist! Assume -R? [n] y
patching file layouts/foundation_6row_combo/foundation-6row-combo.html.twig
The next patch would delete the file src/Plugin/Layout/FoundationLayout.php,
which does not exist! Assume -R? [n] y
patching file src/Plugin/Layout/FoundationLayout.php
The next patch would delete the file zurb_foundation.layouts.yml,
which does not exist! Assume -R? [n] y
patching file zurb_foundation.layouts.yml

Please let me know if i did anything wrong.

Thank you Mario

@ginhoelias I may be reading your patch output wrong, but I don't think your patch is being applied.

When I see a bunch of messages like:

patch -p1 < zurb_foundation-removelayouts-2867688-36.patch
...
patching file src/Plugin/Layout/FoundationLayout.php
The next patch would delete the file zurb_foundation.layouts.yml,
which does not exist! Assume -R? [n] y
patching file zurb_foundation.layouts.yml

My first thought is that the patch can't find the file, layouts/foundation_1col/foundation-1col.html.twig.

What I would do is:

1. Make sure you are in the correct directory: '/mnt/www/html/myweb/docroot/themes/contrib/zurb_foundation'
2. Make sure the files actually exist: 'src/Plugin/Layout/FoundationLayout.php'

ginhoelias i would try using the command git apply -v from within the root level of the repository.

Also, what would people like to do at this point about this issue? It is always hard to say since we don't like to disrupt what is working for people, but this is a troublesome problem! I would accept the consensus around what most maintainers and community around the theme would want.

Maybe this is related or unrelated?

I applied patch #36 and get the following when enabling a layout from the module. Of course I couldn't check if the same error comes without the patch because only a white screen without the patch.

(EDIT: Funny behaviour, if i save again, same error. If I fill the regions with fields and save, error is gone. I can even remove the fields from the regions and the error stays away.)

(Line 59 is

'#default_value' => $configuration['wrapper_id'],

)

Notice: Undefined index: wrapper_id in Drupal\foundation_layouts\Plugin\Layout\FoundationLayouts->buildConfigurationForm() (line 59 of modules/contrib/foundation_layouts/src/Plugin/Layout/FoundationLayouts.php).
Drupal\foundation_layouts\Plugin\Layout\FoundationLayouts->buildConfigurationForm(Array, Object) (Line: 776)
_ds_field_ui_table_layouts('commerce_product', 'clothing', 'default', Array, Object) (Line: 42)
ds_field_ui_fields_layouts(Array, Object) (Line: 247)
ds_form_entity_view_display_edit_form_alter(Array, Object, 'entity_view_display_edit_form') (Line: 539)
Drupal\Core\Extension\ModuleHandler->alter('form', Array, Object, 'entity_view_display_edit_form') (Line: 834)
Drupal\Core\Form\FormBuilder->prepareForm('entity_view_display_edit_form', Array, Object) (Line: 276)
Drupal\Core\Form\FormBuilder->buildForm('entity_view_display_edit_form', Object) (Line: 93)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 582)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 151)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 99)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 78)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 665)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

It looks like we will have to start a new branch for the next version of Foundation anyway which would be a good spot to make the breaking change on layouts proposed here, so I am marking this postponed.

I have committed the minimal patch offered by Philosurfer in #3032594: Drupal 8.7.x-dev Layout Builder breaks due to changed namespace that should minimize any breaking changes.

Bumping this thread - per #2989534: Consensus On Layouts it seems like we generally have consensus to move the layouts out of the theme into the foundation_layouts module. If anyone is not supportive of this let's try to sort it out soon (this month) - i would suggest discussing on that issue.

Then we can move on to issues such as updating the build process using gulp or other build tools, drush 9 etc.

I'm closing this issue because I think it was fixed by removing layouts. If this is not the case, please reopen it and this would need to be addressed before a beta release.