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.

CommentFileSizeAuthor
#17 drupal.bartik-comment-links.16.patch984 bytessun
#8 unset_comment_link_850740.patch1.83 KBJeff Burnz
#1 bartik-process-node.patch2 KBbleen

Comments

bleen’s picture

bleen commented
Status: Active » Needs review
StatusFileSize
new bartik-process-node.patch2 KB
Jeff Burnz’s picture

Jeff Burnz commented

#1: bartik-process-node.patch queued for re-testing.

aspilicious’s picture

aspilicious commented

is bartik_process_node a PREprocess function?

Jeff Burnz’s picture

Jeff Burnz commented

no, its a process function, it runs after the preprocess phase.

aspilicious’s picture

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?

Jeff Burnz’s picture

Jeff Burnz commented

not necessarily.

Jeff Burnz’s picture

Jeff Burnz commented
Title: Simplify Bartik node.tpl » unset Add New Comment link is not unset
Category: task » bug
Priority: Minor » Normal

The code in this is wrong:

$content['links']['comment']['#links']['comment-add']

Should be...

$content['links']['#links']['comment-add']

I'm not sure why we need to set a new variable here, I have rerolled this to fix the issues.

Jeff Burnz’s picture

Jeff Burnz commented
StatusFileSize
new unset_comment_link_850740.patch1.83 KB

Weird, my patch didnt get attached, lets try that again...

bleen’s picture

bleen commented

Only local images are allowed.
Only local images are allowed.

bleen’s picture

bleen commented
Status: Needs review » Reviewed & tested by the community

status

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Reviewed & tested by the community » Closed (duplicate)

Thanks 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.

jensimmons’s picture

jensimmons commented

LoL, sun — YOU are the one who reported this issue, and didn't realize it's a dup. :P

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Yes. 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.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Closed (duplicate) » Needs review
StatusFileSize
new drupal.bartik-comment-links.16.patch984 bytes

Re-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

EvanDonovan’s picture

EvanDonovan commented

Looks like a nice simplification. Will try to apply to a 7.x install soon, unless someone beats me to it :)

sun’s picture

sun
Primary language German
Location Karlsruhe
commented 
+++ themes/bartik/templates/node.tpl.php	11 Jan 2011 14:56:51 -0000
@@ -106,17 +106,11 @@
     // Only display the wrapper div if there are links.
-    $links = render($content['links']);
-    if ($links):
+    if ($content['links']):

Actually, I think we need to keep these old/current lines, since the renderable array contains additional #properties...

Powered by Dreditor.

geerlingguy’s picture

geerlingguy commented

Can 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.

Jeff Burnz’s picture

Jeff Burnz commented
Status: Needs review » Needs work

What 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):

$links = render($content['links']);
if ($links):
?>
<div class="link-wrapper">
  <?php print $links; ?>
</div>
<?php endif; ?>

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.

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.