This is a great module and almost exactly what I need for my site. However, I'm finding that I need more flexibility in my layouts than the module currently provides. Rather than ask bengtan to create layouts for every use case (as I'm sure he has better things to do), here's a patch that allows site administrators to add their own custom layouts without modifying composite.module.

The method is borrowed from Panels. Basically, I've replaced the hard-coded array that defined the zones for the layouts with a pair of functions that scans the theme directory for inc files. Then I added an inc file for each layout that itself contains a function that builds the array item for that layout.

To add a new custom layout, all the site administrator needs to do is place their custom tpl.php, css, png, and inc files in the theme directory -- the functions will find the files and add the new layout option.

I've tested it on my site, and it seems to be working well. Let me know what you think.

Comments

todddevice’s picture

subscribing

bonvga’s picture

subscribing !

bengtan’s picture

Subscribing :)

Sorry, I'm overseas at the moment, but I'll look into this when I get back to home base. Thanks.

bengtan’s picture

Status: Needs review » Postponed (maintainer needs more info)

Okay, I've cleared out my backlog and now I'm looking at this.

As far as I can tell, your patch adds a hook_composite_include() through which other modules can define additional layouts ...

(Plus, you've moved a lot of the declarations into separate .inc files - I don't have any problem with this aspect).

... Which is nice and all, but ...

There is already an existing hook hook_composite_layouts() through which other modules can already do this. (And in fact, Composite Layout itself uses hook_composite_layouts())

So, I can't quite see what additional functionality your patch adds.

Can you help me understand the reasoning behind your patch better? Have I misunderstood it?

jesss’s picture

For my site, I need a number of custom layouts, and in looking through the issue queue, I saw requests for additional layouts from others (#325056: Two Column Layouts with 75/25 and 25/75 Split and #448308: four columns?). So, this seems to be a desired feature.

I looked through the original module code, and I didn't see any obvious way of adding layouts. The layouts that come with the module were in a hard-coded array, and I didn't see the hook_composite_layouts() function you mention. There was, however, a comment on line 125 about how you would like to make layouts pluggable.

    // Would like to make layouts pluggable, but doesn't quite work
    //   because of inability to specify template files from other modules. 

That's what this patch does. By removing the layouts from the module code and into separate .inc, .tpl.php, and .css files, site maintainers can now plug in their own layouts (using the existing layouts as models and documentation). There's no need for them to write their own separate modules, hack the composite.module code, or request for you to add additional layouts for them.

bengtan’s picture

In composite.module in the function composite_get_layouts(), around line 127, there is a

$layouts = module_invoke_all('composite_layouts');

That calls hook_composite_layouts() for all modules which define it, including composite.module's own composite_composite_layouts(). If you have a module which also defines hook_composite_layouts(), your module can insert it's own layouts this way.

The comment ...

    // Would like to make layouts pluggable, but doesn't quite work
    //   because of inability to specify template files from other modules.

is a reference to the fact that...

1. To define a layout, a Drupal template (ie. something declared via hook_theme()) needs to be defined, and to properly do that, a default ___.tpl.php file should be specified.

2. However, if the layout is defined by another module, there is no way for composite.module to tell Drupal's theme registry (via composite_theme()) that the default ___.tpl.php for the corresponding template lives in that other module's directory.

BUT

Even if the template is not properly defined, I believe it is still possible to define a custom layout if a module and a theme are written to work together ie. the module defines a layout via hook_composite_layouts(), and the theme supplies an overridding ___.tpl.php file. (I haven't actually tested this though).

BUT

Your patch also has this limitation, so this is a red herring, as far as I can see.

---------------------------------

My opinion is that your patch only duplicates what was already there. There is an existing hook_composite_layouts(), but you've gone ahead and defined another hook_composite_include() which accomplishes the same functionality.

Have you successfully defined a custom layout with your patch? If you have, I'd like to see an example.

An example would demonstrate to me why your patch is functionally different, or I could demonstrate how to do the example using the existing hook_composite_layouts()

nicholas.alipaz’s picture

I see what jesss is describing. Basically he wants to enable some sort of theming system similar to that of views. Add a composite_layout() call and the appropriate tpl.php file and etc to your module then it allows the module to load them up. Additionally, he describes being able to simply drop in some template files and etc into the themes directory of the module and that would automatically load them as well.

IMO, I think the functionality should follow the current one of views.

// initialize the views support
function mymodule_views_api() {
  return array(
    'api' => 2,
    'path' => drupal_get_path('module', 'mymodule').'/views', // set the path
  );
}

// theme function
function mymodule_theme($existing, $type, $theme, $path) {
  $path .= "/views";
  return array(
    'views_view__mymodule__page_1' => array (
      'arguments' => array('view' => NULL),
      'template' => 'views-view--mymodule--page-1',
      'original hook' => 'views_view',
      'path' => $path,
    ),
  );
}

This would allow for the use of a new tpl.php file for overriding a specific view layout. So why could the composite module do something similar?

Please provide feedback on all the ideas mentioned here guys.

BTW, a description/documentation of how the current function (hook_composite_layouts()) is supposed to work would really help those who might need it. It should really be added to the module's documentation. I came here looking for some information on that function ttytt.

bengtan’s picture

I agree that implementing custom layouts is a good feature to have.

However, I am not convinced that the patch adds this feature. From what I can see, the patch is incomplete and I see no further evidence otherwise. If the original poster were to provide more information or preferably, a custom layout, so I can test that the patch works, then I'd gladly merge it in. Otherwise, I merely have someone's word that it works, but I haven't been able to test it successfully. Hence, no, I can't accept the patch as it is.

Now, if someone were to amend that patch and/or submit a new patch, then I'd certainly look at this issue again.

jesss’s picture

FileSize
2.18 KB

The patch is complete, and does add the feature. I didn't include any of my custom layouts with the original patch because the whole point is that anyone can add their own using the files I included in the patch for the existing layouts as a model.

However, if you need to see an additional layout as well, here you go. With the patch applied, drop the four files included in this zip archive into the composite theme directory. Voila, new layout.

bengtan’s picture

Status: Postponed (maintainer needs more info) » Active

Hi,

Thanks for posting up that example. It has cleared up some misunderstandings.

Your patch allows for the creation of custom layouts by adding files into Composite Layout's theme directory ie. .../modules/composite/theme. In other words, you actually modify the module Composite Layout - albeit by adding new files but not changing existing ones.

I initially thought your patch allowed for other modules to add custom layouts without changing Composite Layout ie. all the files would be contained in that other module. This would be the ideal (but trickier) solution.

There's a big difference between the two approaches, hence we were actually talking about different things, hence the misunderstandings.

So, now with that cleared up, let me rethink this issue and let me get back to you.

bengtan’s picture

After thinking and working on this further ...

I have a prospective implementation of custom layouts for Composite Layout available for testing.

However, this is a totally different implementation, and the changes are unrelated to the original patch in the original post.

The original post required adding files to the .../modules/composite/theme directory. My prospective patch allows any external module to define additional layouts. These two approaches are different, hence the implementation is also completely different.

I will attach the patch to this issue thread in two forms:

A patch created against 6.x-1.0-beta7. This patch is cumulative of the new feature and of all previous CVS commits since 6.x-1.0.beta7.

and as

two source files composite.module and composite.pages.inc which are to be copied into an existing 6.x-1.0-beta7 installation over the existing files.

Please use whichever is more convenient.

I will also be posting up a sample custom layout as an example.

Please have a look at this, test, try it out etc. and let me know how you go. I'm hoping to check this into cvs in the next week or two or three.

@jesss: With this new patch, the method of defining new layouts changes, so you'll have to change your layouts to suit. Please see the sample custom layout. Apologies for the inconvenience.

bengtan’s picture

And the sample module defining a new layout.

bengtan’s picture

Oops, sorry, the attached file in #12 is wrong. Here is the correct one.

nicholas.alipaz’s picture

An additional note, would it be better to put custom layouts into say, "/sites/all/library/composite/*"? That would keep from wiping out all the templates on upgrading. This is more in sync with the way D7 will likely be working, it has a "library" directory by default I believe.

of course on a drupal multisite the directory would be "/sites/sitename/all/library/composite/*"

ideas? The current implementation is nice, but I really think that not having the ability to let another module declare a template is a real downfall.

bengtan’s picture

@comment 14: The patch in comment #11 assumes (and requires) that other modules define a layout (including the ___.tpl.php template file). See the sample layout attached to comment #13. It's actually implemented as a separate module.

So... I think your questions don't really make sense in light of this.

However, if I have misunderstood anything, please ask.

nicholas.alipaz’s picture

Well, let me clarify what the patch does?

  • Allows another module to declare a ___.tpl.php file for composite layout to use.

Does this patch also allow placing ___.tpl.php files into the /sites/all/modules/composite/theme folder, to be automatically detected?
Or must all custom layouts be placed in a new module?

If the answer is yes to allow placing ___.tpl.php files into the /sites/all/modules/composite/theme folder

would[n't] it be better to put custom layouts into say, "/sites/all/library/composite/*"? That would keep from wiping out all the templates on upgrading. This is more in sync with the way D7 will likely be working, it has a "library" directory by default I believe.

If not, then it might be worth adding. Let me know what you think. Thanks again, and I am testing the new feature now...

bengtan’s picture

For the patch of comment #11:

Allows another module to define a new layout for Composite Layout to use. This includes a ___.tpl.php file, a css file, any template_preprocess___ functions, and an icon for the layout.

> Does this patch also allow placing ___.tpl.php files into the /sites/all/modules/composite/theme folder, to be automatically detected?

No

> Or must all custom layouts be placed in a new module?

Yes

> would[n't] it be better to put custom layouts into say, "/sites/all/library/composite/*"?

*shrugs*

I'm going to follow Drupal convention on this one. Since Drupal 6 doesn't have a .../library directory, then Composite Layout 6.x won't have one either.

If Drupal 7 (when it is released) uses a library directory, then the 7.x version of Composite Layout will use the library directory (if it is the convention to do so).

nicholas.alipaz’s picture

Cool, thanks for answering all my questions. That gives a real good summary of what the patch does as well. The patch seems to be working well here.

nicholas.alipaz’s picture

Status: Active » Needs review
bengtan’s picture

Status: Needs review » Fixed

Seeing as no one has objected to anything or reported bugs, I have committed the functionality of comment #11 into cvs. Hence, this feature request is complete. Thank you all for participating.

bengtan’s picture

Added and released in Composite Layout 6.x-1.0-beta8.

yejezkel’s picture

Title: custom layouts » Different templates for referenced nodes
Version: 6.x-1.0-beta7 » 6.x-1.0-beta8
Component: Code » Miscellaneous
Category: feature » task
Status: Fixed » Needs review

First of all the module is very very good, great work!!!!!.
I need the possibility to use a different node.tpl.php (template) for the nodes according to the layout and zone they are showing in, i was playing with the module and checking his code and the first way that i found was:
in the file composite.node.inc, method: composite_composite_node_api where the referenced nodes are been loading, add to the node object the information about the layout and zone ($reference function parameter) later another module or theme implementing preprocess_node function can check that information and change the template_files variable in order to apply the desired template file.

What i don't like about these solution is that i need to modify the module (just one line: $node->composite_location_info = $reference in the right please).
If someone knows another approach i thinks it would help not just to me.

Thank you
Great Module

bengtan’s picture

Title: Different templates for referenced nodes » custom layouts
Version: 6.x-1.0-beta8 » 6.x-1.0-beta7
Component: Miscellaneous » Code
Category: task » feature
Status: Needs review » Fixed

Your issue is a separate topic from the original post. Please do not hijack this thread. Instead you should file a new issue. Thank you.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.