Hello,

I'm still using intensively your module. I noticed you override the $node->body variable in the hook_nodeapi() with 'alter' op. In some cases, this cause weird displays, because the $node->content still contains some remaining stuff such as content fields.

In fact, I use composite as a "node page layout manager", but I display the node itself in full or teaser mode in a block in this layout. The odd behavior here is that I have a lot of information which is being displayed twice (in the block itself, and in the normal node content).

To avoid this, I would like to change the way your module displays the node body.

I'm going to implement the hook_menu_alter() in order to change the node view page callback, and replace it with one of your module which display directly the built layout, and only the built layout, not the node view.

Ideally, it would be great that it become an optional feature of you module, with configuration on a per node type basis. I would like to add an option such as "override the node page view with layout display only" in the composite enabled content types.

The menu callback for the node page view would then be proxy function that either display the normal node page (by storing the old callback in a variable or such, if this last one has been modified by another module, such as panels), or the layout only callback in case the option has been checked for the current content type.

This would need, I think, to ensure the hook_menu_alter() to run after the other to be carefull about module weights, but I think in most site configurations it would play nicely without having to care about this (not that much modules alters this menu entry, and in case some does, it's quite easy to spot and warn the user about it).

I'm going to do the patch anyway, but tell me if you're interrested by such feature.

CommentFileSizeAuthor
#7 composite-6.x-1.x-dev-node_view_alter-fixes_and_tests-2.patch11.43 KBpounard
#3 composite-6.x-1.x-dev-node_view_alter-fixes_and_tests.patch10.75 KBpounard
#2 composite-6.x-1.x-dev-node_view_alter.patch4.02 KBpounard

Comments

pounard’s picture

pounard
Primary language French
commented
Version: 6.x-1.0-beta8 » 6.x-1.x-dev

I'm going to work and patch the 6.x-1.x-dev version from now, so the current patches (like recursion check and theming refactor) won't bother me. Are you actually planning to release a new stable version soon? I could help by doing some simpletest unit tests if it can help doing non regression testing for stable release.

pounard’s picture

pounard
Primary language French
commented
StatusFileSize
new composite-6.x-1.x-dev-node_view_alter.patch4.02 KB

And did the patch, seems to be seamlessly working on my env.

pounard’s picture

pounard
Primary language French
commented
StatusFileSize
new composite-6.x-1.x-dev-node_view_alter-fixes_and_tests.patch10.75 KB

Here is a patch, fixed after I did some unit testing (really basic though).

This patch, beside doing some fixes and adding some tests, moves *.test files into a tests subdir.

bengtan’s picture

bengtan commented

Hi,

I had to override $node->body in the nodeapi->alter hook. I don't think there was any other way to do it. If you have duplicate portions of content appearing, you might be able to get the 'other module' not to display their content and let Composite Layout take care of it.

As for your idea about overriding node_page_view ... I don't ... really ... want to go there at the moment. Sorry.

If enough people make noise about wanting a new beta release, I can release one. Just let me know.

pounard’s picture

pounard
Primary language French
commented

Ok, heard what you said. You have to notice this is an optional feature, non activated per default, it does not hurt. It does not modify any existing behavior, it only provide a menu entry alteration if enabled in order to display a brand new page (without altering which already exists).

If you feel the need to test, please do it, at least you will see it doesn't hurt :)

Let me know if you change your mind, I'll keep using the patch on my side anyway.

jide’s picture

jide commented

subscribing. (FYI, we are working together with pounard on a project involving composite).

pounard’s picture

pounard
Primary language French
commented
StatusFileSize
new composite-6.x-1.x-dev-node_view_alter-fixes_and_tests-2.patch11.43 KB

Just some minor fixes, node will never show the composite view if they are configured with node page alteration, this will ensure some recursions and weird behaviors when displaying the node else where than on its own page.

bengtan’s picture

bengtan commented

Hi,

I'd still rather not incorporate this into Composite Layout. Anything that needs to do a menu_alter() to override Drupal core ... just feels a bit too specialised and potentially intrusive for my liking.

Plus, it means extra code and functionality that I have to support moving forward, and if only one or two people use it ... then it's debatable whether the benefits are worth the cost over the lifetime of Composite Layout.

Here's a possible compromise ...

Do you realise you can probably write a custom module to do this? You can do a menu_alter(), and anything else you require, in this separate module, whilst leaving Composite Layout unchanged. Then I don't have to incorporate this feature, but you also get the functionality you want, but only for your site(s).

pounard’s picture

pounard
Primary language French
commented

Bengtan, this is not a bad idea, writing a custom module for this, but for this I'd need a way (API or options) to disable composite rendering in node body in the composite module.

bengtan’s picture

bengtan commented

Hi,

I've had another look at your patch.

It does seem like something that you could accomplish by writing your own add-on module. I don't understand when you say:

> I'd need a way (API or options) to disable composite rendering in
> node body in the composite module.

Why do you need to disable this? Your composite_page_view() function doesn't use $node->body, and there is no mention of disabling this in your patch.

pounard’s picture

pounard
Primary language French
commented

On the site I'm working on, I sometime display nodes, as full nodes, at various places in the site. I want those nodes not to display with their composite content, but using the normal Drupal display instead. I want to use the composite layout only for the node page.

bengtan’s picture

bengtan commented

Fair enough, but it doesn't show that in your patch and now I'm confused as to what you want.

Your patch indicates you want one thing, and now you say you want something else. I'm a bit lost.

pounard’s picture

pounard
Primary language French
commented

Ok, maybe my english is not that good.

I use the node composite layout as something I can display on a page, or elsewhere, but not in the node body itself. I would like to keep the full node view, in all most cases, as if the composite module was not enabled (such as in views, in core 'node' page, in taxonomy pages, etc).

The only place where I display the composite layout is in the 'node/%/view' page itself, but I don't show up the node, only the composite layout itself as a full page dedicated to it (no node title, no taxonomy terms, etc).

If I had to rewrite a part of your module, I would rewrite it to make the composite layout something that is not a part of the node itself, and that won't show up in the node body except if it has been configured for. I would make the composite module an API module not altering the node body, and I'd put the node body alteration in a submodule, so I could then display it anywhere (a bit like panels, but keeping it more simple, and I still need it to be attached to the node).

bengtan’s picture

bengtan commented
Status: Active » Closed (won't fix)

Expiring this issue as it is old and uncertain whether it is still applicable.