Allow for more configurable regions

That would indeed be cool, but it not on the roadmap for now - we're also working hard on the port to D7 which will have this functionality by default. Not sure if this will be backported to drupal 6 afterwards since that's a lot of API changes and I really don't have time for that. So if anyone else is up for this and hands me patch and doesn't break anything between two releases, I'm happy to commit that :)

Subscribing

Cleaning up my queue and making sense of it.

This is acutally working in the Drupal 7 version. A back port is such a big API change and needs to be in different branch, something which I'm not up to anymore - unless someone else steps up and starts coding and patching for a few weeks :)

I think if someone wants new regions in a different layout, this should be done programmatically rather than clicketiclick.
I would be interested to see the D7 solution documented somewhere.

My own idea would be:
- either, programmatically add build modes, with custom regions. This could of course lead to name clashes with existing build modes.
- or, programmatically add build mode "patterns", each coming with its own regions and template. Each build mode can get a default pattern, and users can override the choice of pattern per node type.

I wonder if we could have a cheap solution for D6:
1. A new function ds_layout($build_mode, $type). The return value is an array containing region names and a theme hook name.
2. Introduce a new hook_ds_layout_alter(&$layout, $build_mode, $type), that can be used to have special layouts for specific build modes.
3. Change the ds_regions() function to use the ds_layout() function. This means, ds_regions() will need the build mode and type passed as arguments.
4. Change the way that regions are rendered, so it uses the theme hook provided by ds_layout().

The bad thing is, this will be extra work to upgrade to D7 for someone who used the hook_ds_layout_alter() approach in D6.

something which I'm not up to anymore

Completely understand, seeing how much time I already spend on my own modules..

I could work on a patch for that, but only if there is a chance of getting it in.

There's a 6.x-2.x branch since a week or two for new features (multigroup is in, but not yet really fully tested by me). I'd be happy to commit a patch to that branch for this functionality.

Hi!
Yes, I noticed the 2.x dev branch.
Are you happy with an approach as in #8, or should I rather study the D7 branch and try a full and upwards compatible backport?
What about the upgrade path from 6.x-1.x to 6.x-2.x to 7.x-*, for custom modules and configuration ?

Atm I don't have any D7 project with a budget (= motivation) or a client or a purpose. This is why I had only a very brief look at the D7 version so far.

ds_regions() is indeed the critical function, but haven't really thought about the other steps. The region/layout key could be stored into the "$module .'_display_settings_'. $object->type" variable (oh my, the more I see this, the more I hate it, D7 stores this much nicer, but anyway).

I personally don't care about the upgrade path from D6 to D7. Even just upgrading core is really hard as it is right now. Also, the DS D7 has been almost rewritten from scratch to be more flexible, has completely different (and better) storage etc. I wouldn't bother about that path :)

Need to go now, I'll try to think about this a bit more too the next couple of days.

Just had an idea. Maybe too simple / naive.
What about
- We backport the d7 branch as "ds7.module", which can be installed in parallel with ds on D6 sites.
- One can configure a type/buildmode with ds7, to override the existing setting from ds. Otherwise, the ds setting is used. And if none of these are configured, the usual node/cck is used.

I'm not sure if this really needs a new module named ds7, or if it could be included in ds. The key is that we want old and new in parallel, so we don't damage any existing setups. (the site i work on has 38 content types..)

The region/layout key could be stored into the "$module .'_display_settings_'. $object->type" variable

With the approach in #8, we would not store this key anywhere.
If you implement hook_ds_layout_alter(), you have to know what you are doing. The hook would be called every time that ds_regions() and thus ds_layout() is called. Maybe the result can be cached in a static var.

ds_layout() will always start with the basic default layout, and then ds_layout_alter() can decide what to do.
This means, you really need to be a module developer to take advantage of this. Unless someone codes a UI module to make this accessible to click-click site builders. This UI module would have to define its own storage.

Hrm, that's right. With that approach, you can keep the initial patch size fairly small too. A static variable will be needed for sure. Maybe we should try to avoid calling drupal_alter() or module_implements() and use a variable which just has the names of the module(s) implementing that hook - I can imagine not a lot will do this. Could be even just a function (like the infamous custom_url_inbound stuff) ? Just a thought of course.

Doing a backport in soms sort of ds7 module is going to be hard tomaintain I think.

And wow, 38 content types, that's a lot :)

use a variable which just has the names of the module(s) implementing that hook

module_implements() already has a static cache.
http://api.drupal.org/api/drupal/includes--module.inc/function/module_im....
There is also an rtbc D6 backport in the #557542: Cache module_implements() request for a persistent cache.

What I am a bit worried about is if we can safely change the signature of ds_regions().

Aha, I didn't know they were trying to backport that too. Great!

As for the signature, I don't think that's a big problem.

function ds_regions($regions = 'all', $render = FALSE, $type = NULL, $build_mode = NULL) {
  $options = array(
    'regions' => $regions,
    'render' => $render,
    'type' => $type,
    'build_mode' => $build_mode,
  );

  // Let other modules alter the region set.
  $regions = drupal_alter('ds_regions', $options);
  if (!empty($regions)) {
    return $regions;
  }

  // Fallback to default.
}

That way, existing installations won't break.

We need to let modules alter both the regions and the theme hook to be used.
The alter hook would be hook_ds_layout_alter(), not hook_ds_regions_alter()

Maybe like this,

<?php
function ds_regions($regions = 'all', $render = FALSE, $type = NULL, $build_mode = NULL) {
  if (!isset($type)) {
    return _ds_regions_legacy($regions, $render);
  }
  else {
    // we don't care about the $render option. The template / theme function will deal with that.
    $layout = ds_layout($type, $build_mode);
    return $layout['regions'];
  }
}

function ds_layout($type, $build_mode) {
  // copied from ds_display_overview_form()
  $regions_mode = variable_get('ds_build_mode_' . $build_mode, 'all');
  $layout = array(
    'regions' => _ds_regions_legacy($regions_mode, FALSE),
    'theme' => ($regions_mode == 'all') ? 'ds_layout_default' : 'ds_layout_plain',
  );

  // Let other modules alter the layout.
  drupal_alter('ds_layout', $layout, $type, $build_mode);

  return $layout;
}
?>

Not sure about this one,

<?php
  $regions_mode = variable_get('ds_build_mode_' . $build_mode, 'all');
?>

where is this variable configured?

Damn..
http://drupalcontrib.org/api/drupal/contributions--ds--ds.module/functio...
called by 12 functions. These are all within ds, so no problem.
But if any other (custom) module calls ds_regions() with the old 'all' parameter, then all we can do is return the default. So these modules will not get the correct (modified) region set.

On the other hand, this is only a problem if someone wants to implement the hook_ds_layout_alter(). So I guess it is ok.

Yeah, this will be drag+drop.

What you would do in your custom module:

<?php
function mymodule_theme() {
  return array('myregions' => array('regions' => array()));  // what exactly goes in the array? quite irrelevant if you don't want templates
}

function mymodule_ds_layout_alter(&$layout, $type, $buildmode) {
  if ($type = 'mytype' && $buildmode = 'mybuildmode') {
    $layout['regions'] = array(
      'myregion1' => 'My region 1',
      'myregion2' => 'My region 2',
    );
    $layout['theme'] = 'myregions';
  }
}

function theme_myregions($regions_html) {
  extract($regions_html);
  return <<<EOT
    <div class="myregion1">$myregion1</div>
    <div class="myregion2">$myregion2</div>
EOT;
}
?>

Then you get your two-region configuration form, and layout as in the theme function.

Did not really think about it, but yes, that's more or less how it would turn out.

I also considered if we should have a point in time where the entire thing is a big nested array for drupal_render(). Run it through an alter hook, then pass it to the theme function. Something like that.
But, to keep this a simple and small patch, I'd rather say no.

EDIT:
There is one difference.
The regions passed to theme_ds_regions are already sorted for print.
I think that's a bad idea: The theme function should decide itself, in which order it wants to print the regions.