Layouts currently don't render region wrappers around the various regions. This will be problematic as we move to migrating existing theme page templates to layout plugins. Layouts are also treating the output as strings, and I'm pretty sure they should be arrays.

Eclipse

CommentFileSizeAuthor
#3 1929682-3.patch7.26 KBEclipseGc
#1 1929682-1.patch3.12 KBEclipseGc
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

EclipseGc’s picture

Status: Active » Needs review
FileSize
3.12 KB

and a patch.

Status: Needs review » Needs work

The last submitted patch, 1929682-1.patch, failed testing.

EclipseGc’s picture

Status: Needs work » Needs review
FileSize
7.26 KB

ok, let's see if this passes now.

Gábor Hojtsy’s picture

<div class="layout-region .. is already the region wrapper, no? Adding one more wrapper does not sound like very efficient. What about making it possible to add classes on the existing region wrapper instead?

EclipseGc’s picture

Actually the problem here is that in the bartik conversion, none of the regions have a region wrapper because it's added on programmatically somewhere else. I'm open to ideas here, but we should either manually add region wrappers in the bartik tpl, or we should remove the regions wrappers from our existing layout plugins. I'm fine with either solution. Thoughts?

Eclipse

Gábor Hojtsy’s picture

We can remove the existing region wrappers from the layout templates. I don't think duplicate region wrappers is a good idea. I smell divitis there.

EclipseGc’s picture

Yeah, I think that's the right solution too, I worry about people wanting to get rid of the regions wrappers though. :-S

webchick’s picture

Project: Drupal core » Layout
Version: 8.x-dev » 8.x-1.x-dev
Component: layout.module » Code

Layout module got removed from core, so moving this over here instead.