Theming optimizations

Note: we're already using this in production and it turned out to be the best and most powerfull solution, so if you have remarks, please comment. Also, this ain't an API change, merely a CSS change :) We'll probably commit this any time this week.

With only looking at this issue and the code for a minute or two, my initial thoughts would be to move more of the build in theme code into templates and preprocess functions so that it's easier to override.

Cheers,
Deciphered.

@Deciphered
That's the idea too, the complete logic/code of

foreach ($object_display->themed_regions as $region_name => $region_data) // ds.module around line 335

will be moved into a separate theming function. We'll look if we can move any other stuff too!

Some things that I noticed.

ds_render_content() has some hardcoded html and css class names. This should rather be a theme function! (EDIT: ok, yes, you said that)

The classes "one-sidebar" and "sidebar-left" etc are likely to cause nameclashes. Unfortunately, IE does not support chained classnames like ".ds-region-middle.one-sidebar", so we have to pray. At least the ".node .sidebar-left" can avoid a clash with the body classes.

Nested ds displays (think of nodereference or embedded views) cause clear: problems in the middle region. The solution can be to float the middle region and work with negative margins, like zen does it.

Nested ds displays are harder to target, because IE does not understand ".buildmode-full > .ds-region-left". Trying to throw more classes at the problem will bloat the html, but at least themes should have the possibility to add their own class names.

An even nicer solution would be to make the fields configuration a tree structure, as explained here:
#570978-7: Possible new features for D7 port

Has been committed to D6, marking as to be ported to include this in D7

The ".node .sidebar-left" selector solves the problem just for nodes, but what about comments and other modules?

I think that if the sidebars are supposed to behave the same for all modules (25% | variable | 25%) at least as a default style, then we should use a ".ds .sidebar-left" selector instead.

.ds .no-sidebars {
  clear: left;
}

.ds .sidebar-left {
  margin-left: 25%;
}

.ds .sidebar-right {
  margin-right: 25%;
  clear: left;
}

.ds .two-sidebars {
  margin-left: 25%;
  margin-right: 25%;
}

What do you think guys?

And it all doesn't help for nested stuff. Views in node fields, etc.

EDIT:
It does help for nested stuff, if the different levels come from different modules.
If two node displays are nested in each other, then we need custom classes, which can be done with theming (looking at dev snapshot). So I guess it's kind of ok now.
Next step should be custom html configured from the Drupal backend.

Hi again..
Another note about the zen-like solution.
In zen, the middle column has an explicit width setting. In our case this would be 50%.
Anyone who wants to change the width of the left and right columns has to adjust the width of the middle column.
With px widths, this requires to know the total width of the container.
With % widths, we can get rounding errors.

I would like to propose an alternative way, that I usually use for float columns, and that does not require an explicit width for the middle column.

Here is the html:

<div class="middle"><div class="middle-inner">..</div></div>
<div class="left">..</div>
<div class="right">..</div>

And here the css:

.middle {
  display:inline;  /* avoid IE duplicate margin bug, just in case */
  float:left;
  width:100%;
  margin-right:-100%;  /* old trick from zen theme */
}
.middle-inner {
  margin-left:25%;  /* width of the left column */
  margin-right:25%;  /* width of the right column */
}
.left {
  display:inline;
  float:left;
  width:25%;
  /* zen would give this a negative margin-right, but we don't need that here */
}
.right {
  display:inline;
  float:right;  /* zen would float this left, but right is simpler */
  width:25%;
}

Why is this good?
Because now someone can override the column widths with absolute px values, without knowing the total width of the container.

.special .left {
  width:160px;
}
.special .middle-inner {
  margin-left:160px;  /* width of left column */
}

What's missing?
We need to wrap everything into a container that clears its content, using one of the common tricks (hidden :after content, hasLayout, etc)
We could also add some equal height columns magic.

Since we want to keep the amount of divs as small as possible, I would keep this 'clearing' job for the node or block div. Just add a class='clear-block' there.

Not sure about this. We want to keep the header and footer region out of the floating game.

I am running the new ds, and the new columns solution works great! I didn't expect it could be that small number of divs!

Just, I had to update some of my CSS files, or I would see broken layout.
This could make you unpopular with some folks who didn't bother to make a backup before upgrading their production sites. I guess that was unavoidable, but it shouldn't happen too often.

donquixote: yeah, we are now going to stop with css updates and default colums for a while, we added some info on the release pages so people know what todo with the latest update. Glad it's working out fine! :)

Did you test this with IETester?
I have seen a site where the nd footer doesn't clear properly. Not sure if this is the fault of ds or the site theme.

EDIT:
This problem only occurs if both ds sidebars are empty. Apparently, IE6 thinks it doesn't need to clear a float that has -100% margin right.
Why is the middle column floated anyway, if there are no sidebars?

EDIT:
The following code helps:

.nd-no-sidebars {
  /* fix an ie bug, see http://drupal.org/node/722852#comment-2963186 */
  display:block;
  float:none;
  margin-right:0;
  width:auto;
}