Closed (fixed)
Project:
Drupal core
Component:
node system
Priority:
Normal
Category:
Feature request
Assigned:
Reporter:
Created:
2 Mar 2004 at 14:23 UTC
Updated:
5 Jan 2005 at 17:15 UTC
Jump to comment: Most recent file
When previewing a node, all links on that node are printed.
This is confusing, especially for an unexperienced user.
Actually this can be avoided by making addintional controls in all _link hooks.
A different solution could be to issue node_content instead of node_view when in preview.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | drnodelinks.patch | 7.04 KB | moshe weitzman |
| #3 | node_links_preview.patch | 639 bytes | moshe weitzman |
Comments
Comment #1
moshe weitzman commentedI just implemented a solution using the content hook instead of a whole node_view. It print the node body instead of a whole node view. But this equally insufficient for the user.
How about we add a param to the theme_node() function which disables outputting of node links. I think thats the best approach. Adding checks for the 'Preview' case in all modules which implement node links does not seem like a good time, though it certainly would work. Anyway, the only current way for a module to know we are previewing is to check if
$_POST['op'] == t('Preview')Comment #2
matteo commentedI absolutely agree with you, I think yours is the best approach. will you submit a patch to theme_node() ??
Comment #3
moshe weitzman commented3 line patch attached
these node links are often bad links since a nid has not yet been created
Comment #4
jonbob commented+1 for this; these links are very confusing for my users. In addition to sometimes being invalid, the links can encourage users to click away from the preview before completing the submission, because they look like a completed node.
Comment #5
matteo commentedVerified and it works perfectly. I hope it will be committed soon.
Matteo
Comment #6
dries commented+1 for fixing this. The patch is a hack though.
There is no easy fix because the theme system ask for the links (pull model). To fix this, we should eliminate node_links() and pass the links as part of the node objects (push model). At least, that is a possible path that need to be investigated.
Comment #7
Bèr Kessels commented+1 for the push method.
and if possible push it as an indexed array:
where by default the link will have the name of the module, but incase a module outputs more links, they should get an addtional id.
Themes can then attach unique calsses and id's similar to blocks. Themes can also easily do and if(key($link) == module1) to place thse links on different locations.
Also links should not pe passed as strings containing HTML, but rather as another array. That way theme functions can make icons (image links) out of the links if they wish)
Bèr
Comment #8
moshe weitzman commentedSo, does this imply that every module will have to figure out if we are previewing and then refrain from adding links in this case?
The proposed hack is harmless IMO. Other solutions will likely require quite some more code.
Comment #9
dries commentedI'm not going to commit this as is.
Comment #10
moshe weitzman commentedthis one pushes node links to the theme as suggested.
contrib themes should now print out the contents of $node->links instead of calling link_node(). that function no longer exists.
Comment #11
dries commentedCommitted to HEAD. Thanks Moshe.
TODO:
1. Update theme upgrade instructions in the handbook: node_link() is gone.
2. Remove page_link() just like we removed node_link().
Please don't close this report until (1) has been taken care of. Thanks.
Comment #12
moshe weitzman commentedComment #13
(not verified) commented