Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
from sun's issue here: #849862: Bartik code problems
+++ themes/bartik/templates/node.tpl.php 6 Jul 2010 03:49:00 -0000
@@ -0,0 +1,126 @@
+ // Remove the "Add new comment" link on the teaser page or if the comment
+ // form is being displayed on the same page.
+ if ($teaser || !empty($content['comments']['comment_form'])) {
+ unset($content['links']['comment']['#links']['comment-add']);
+ }
Such adjustments should really be done in a preprocess function, not arbitrarily stuffed within a template. All core theme templates should be lean and mean.
Comment | File | Size | Author |
---|---|---|---|
#17 | drupal.bartik-comment-links.16.patch | 984 bytes | sun |
#8 | unset_comment_link_850740.patch | 1.83 KB | Jeff Burnz |
#1 | bartik-process-node.patch | 2 KB | bleen |
Comments
Comment #1
bleen CreditAttribution: bleen commentedComment #2
Jeff Burnz CreditAttribution: Jeff Burnz commented#1: bartik-process-node.patch queued for re-testing.
Comment #3
aspilicious CreditAttribution: aspilicious commentedis bartik_process_node a PREprocess function?
Comment #4
Jeff Burnz CreditAttribution: Jeff Burnz commentedno, its a process function, it runs after the preprocess phase.
Comment #5
aspilicious CreditAttribution: aspilicious commented"Such adjustments should really be done in a preprocess function,"
So this needs work as it has to be done in a preprocess function..
Correct?
Comment #6
Jeff Burnz CreditAttribution: Jeff Burnz commentednot necessarily.
Comment #7
Jeff Burnz CreditAttribution: Jeff Burnz commentedThe code in this is wrong:
Should be...
I'm not sure why we need to set a new variable here, I have rerolled this to fix the issues.
Comment #8
Jeff Burnz CreditAttribution: Jeff Burnz commentedWeird, my patch didnt get attached, lets try that again...
Comment #9
bleen CreditAttribution: bleen commentedComment #10
bleen CreditAttribution: bleen commentedstatus
Comment #11
sunThanks for taking the time to report this issue.
However, marking as duplicate of #754760: "Add new comment" appears directly above comment form / "post comments" does not work without "access comments" permission. You can follow up on that issue to track its status instead. If any information from this issue is missing in the other issue, please make sure you provide it over there.
Comment #14
jensimmons CreditAttribution: jensimmons commentedLoL, sun — YOU are the one who reported this issue, and didn't realize it's a dup. :P
Comment #15
sunYes. And Bartik should not contain any logic like this at all. (as copypasted in the OP of this issue from the original issue)
Therefore, Comment module needs to be fixed instead, and the existing code in Bartik needs to be removed.
Thus, duplicate of aforementioned issue.
Comment #17
sunRe-opening, since we forgot to fix this in #754760: "Add new comment" appears directly above comment form / "post comments" does not work without "access comments" permission
Comment #18
EvanDonovan CreditAttribution: EvanDonovan commentedLooks like a nice simplification. Will try to apply to a 7.x install soon, unless someone beats me to it :)
Comment #19
sunActually, I think we need to keep these old/current lines, since the renderable array contains additional #properties...
Powered by Dreditor.
Comment #20
geerlingguy CreditAttribution: geerlingguy commentedCan we update the title and re-summarize the issue? It seems the patch doesn't do what the title says, and the progression of this issue is a little confusing to me.
Comment #23
Jeff Burnz CreditAttribution: Jeff Burnz commentedWhat sun is saying is that its fixed in the other patch, but they forgot to remove the cruft from Bartik as part of that patch, and that we need to keep the conditional the way it was because $links can contain additional properties, so needs to be like this (sorry I can't roll a patch right at the moment and am breezing through all bartiks issues):
Add a condition for teaser if we still want to do that (hide it on teasers), personally I don't like arbitrarily hiding this on teasers, not in our default theme at least.