I've noticed that many of the display suite layouts in zurb-foundation leave empty header/footer/top/bottom divs (typically in the 'stacked' layouts) if those regions are left empty.

This makes those particular layouts less flexible than they could be, since e.g. the 2 column stacked layout could otherwise be used when the header or footer only sometimes have content, or for all of the following arrangements:

  • header, two columns, footer
  • header, two columns
  • two columns, footer
  • two columns

This could easily be fixed by moving the check for content from inside the relevant row to outside it:

Current:

<div class="row">
  <?php if (!empty($header)): ?>
     <<?php print $header_wrapper ?> class="group-header columns<?php print $header_classes; ?>">
     <?php print $header; ?>
     </<?php print $header_wrapper ?>>
  <?php endif; ?>
</div>

Proposed:

<?php if (!empty($header)): ?>
  <div class="row">
    <<?php print $header_wrapper ?> class="group-header columns<?php print $header_classes; ?>">
    <?php print $header; ?>
    </<?php print $header_wrapper ?>>
  </div>
<?php endif; ?>

I've marked this as 'Bug report', but I really just wanted to promote discussion: should this remain as-is, or should it be fixed? I can provide a patch if it's wanted.

--b

(edit: my colleague just pointed out to me that this could be intentional e.g. to preserve a design even when content is absent--if so, just change to "works as designed"...)

Comments

kevinquillen’s picture

kevinquillen CreditAttribution: kevinquillen commented
Status: Active » Needs work

For flexibility and fluidity, I would agree. Can someone roll a patch for both DS and Panel templates that address this?

Joran Lafleuriel’s picture

Joran Lafleuriel CreditAttribution: Joran Lafleuriel commented

subscribing !

Or may be an empty class could be injected :

<div class="row empty">

...to allow a design control using css...

chrisjlee’s picture

chrisjlee CreditAttribution: chrisjlee commented
Assigned: Unassigned » alexweber

Hey alex could you take a look at this at least please? If you can't feel free to unassign.

alexweber’s picture

alexweber CreditAttribution: alexweber commented
Assigned: alexweber » Unassigned

TL;DR Honestly, the more I think about this, the more convinced I am that these layouts are meant as "sensible default" starting points and we should maybe make this clearer in the documentation. Again, I don't see a problem with asking people to override these templates in their sub-theme if they want more control!

Hey Chris, thanks for bringing this to my attention! Turns out I'm already aware of it and, to be honest, I'm not sure what a good, generic solution would be moving forwards.

I've already hinted at this before in other related issues and the truth is that, in my experience, it's best to override these in your sub-theme and make it do exactly what you want...

At the end of the day, it's a bit of an awkward situation because we rely on people adding the classes themselves via DS UI in order to get the DS layout regions to behave properly. However, on a fresh install if we pick a layout it should just work out of the box, so there's those default classes added by altering the theme registry (that approach in itself is kinda flawed IMHO).

I guess what I'm trying to say is that the templates would become far too complex to be easily manageable if we were to add conditionals in order to account for every single use-case. I find it very annoying for example that with a given 2-column layout, even if the left/right region has no content, it's wrappers still get rendered and we're left with some big whitespace.

Or may be an empty class could be injected :

...to allow a design control using css...

This is actually a pretty decent idea too, but we'd have to check each region one by one in order to make sure it's empty and that would just really pollute the template files and make it more confusing to people when overriding them.

chrisjlee’s picture

chrisjlee CreditAttribution: chrisjlee commented

Thanks Alex or your insight. I know this is your area of specialty regarding our theme.

shauntyndall’s picture

shauntyndall CreditAttribution: shauntyndall commented
Version: 7.x-4.0-beta1 » 7.x-4.x-dev
Category: Bug report » Feature request

@chrisjlee @alexweber are there other themes we could reference to help guide this concept? It would seem that providing "smarter" default layouts reduces time investment for anyone implementing the theme.

alexweber’s picture

alexweber CreditAttribution: alexweber commented

@shauntyndall, the default Display Suite layouts from the module are a good starting point...

The problem with creating a "one-size-fits-all" approach with the DS layouts shipped with the this theme is that there's hardly such a thing as universal grid markup for content... it's really simple to override the provided layouts in your sub-theme, as explained here: https://www.drupal.org/node/2059481

I apologize if this isn't more helpful or what you're looking for exactly, it's been a while since I've worked on this and I unfortunately don't foresee being able to sink much if any time into it in the immediate future.

That said, I'm more than happy to review patches and help out in the issue queue... thanks! :)