I'm not keen on the way jstools does that theme-specific style thing.

Couldn't we get your theme template.php to respond to a hook_jstools_settings() sort of thing?

This question is out of scope for this particular issue, but I think it's be a lot tidier, and certainly easier to manage and distribute with custom themes.

When I saw

function dynamicload_theme_data() {
  $data = array();
  $data['content'] = 'div#center div.left-corner';
  return $data;
}

I expected to be able to go

function dynamicload_mytheme_data() {
  $data = array();
  $data['content'] = 'div#content';
  return $data;
}

in my template.php, not add stuff to the distribution. wildcarding {theme} with {themename} almost like usual.

Or something similar - mytheme_dynamicload_data() would of course be more normal.

Comments

nedjo’s picture

nedjo
he/him/his
Primary language English
commented

We need the .inc files in order to support e.g. core themes where there will not be native support.

But I'd welcome a patch that would add optional support for functions directly in theme files. This would be a patch on jstools_theme_data() in jstools.module. Yes, the function name format mytheme_dynamicload_data() makes sense.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

If it's theme specific, why not simply use a theme function?

// Default is Drupal core: Garland
function theme_dynamicload_data() {
  $data = array();
  $data['content'] = 'div#center div.left-corner';
  return $data;
}

We can add selectors for other core themes in the same file:

function bluemarine_dynamicload_data() {
  $data = array();
  $data['content'] = '#main';
  return $data;
}

And any theme is able to override the theme function as usual in template.php:

function mytheme_dynamicload_data() {
  $data = array();
  $data['content'] = 'div#page-content';
  return $data;
}

btw: I'd propose to rename that function. 'data' is somewhat misleading. Aren't we talking about CSS region selectors here? theme_dynamicload_regions() would be better, IMHO.

nedjo’s picture

nedjo
he/him/his
Primary language English
commented

If it's theme specific, why not simply use a theme function?

Sounds good, but I'd still like a way to load theme inc files for themes that don't include this function natively, like the module .inc files (modules can implement dynamicload hooks directly, the include files are for those that don't).

'data' is somewhat misleading

I see your point. My idea was (a) we might need further data later and (b) I wanted a method that could be called in all the jstools modules where theme-related data were needed. I'd prefer to leave this as _data for now. For 6.x I'm thinking of breaking jstools apart into separate d.org projects so potentially we could revisit the naming then.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

I'd still like a way to load theme inc files for themes that don't include this function natively

Themes that do not implement this theme function, will automatically use the default theme_dynamicload_data().

Anyway, it don't believe much of a default can be applied to a theme in this context. Each theme has different regions and containers. So I see no reason why theme-related data shouldn't be in a theme function. The complete sub-directory 'theme' could be removed by implementing this.

...like the module .inc files (modules can implement dynamicload hooks directly, the include files are for those that don't).

Sorry, I did not get the point in that - could you explain it a bit more?

nedjo’s picture

nedjo
he/him/his
Primary language English
commented

Sorry, I did not get the point in that - could you explain it a bit
more?

Ah, that would be because I haven't (yet) implemented a 'modules' directory for dynamicload (though I roughed one in locally).

In general, dynamicload contains a lot of unfinished ideas. That's why I removed it from the stable release and haven't yet added it back.

But see for reference the activeedit and activemenu modules subdirectories.

The point of module and theme .inc files is to add code (e.g., hook implementations) that *could* be in modules or themes, but are not. For example, core Drupal modules and themes will never have dynamicload hook implementations (or theme functions). So we put them in .inc files that are loaded as appropriate. If a .inc file's code is ever moved to the appropriate module (e.g. a contrib one), the .inc file can then be deleted.