Currently comment_wrapper doesn't have contextual links. Was this purposefully done?
Skinr module, for instance requires it (and shouldn't contextual allow any module to modify any hook?)

Also, due to the placement of the contextual links, when adding additional contextual links, the alignment is sometimes off (centered for the "powered by drupal" block, for instance).

Attached patch fixes this.

CommentFileSizeAuthor
#14 674546-14.patch1.73 KBJacine
#10 contextual_comment_wrapper_2.patch.patch786 bytesJacine
#7 contextual_comment_wrapper_2.patch710 bytesmoonray
#4 contextual_comment_wrapper_1.patch754 bytesmoonray
#1 contextual_comment_wrapper.patch1.25 KBmoonray
contextual_comment_wrapper.patch1.19 KBmoonray
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

moonray’s picture

moonray CreditAttribution: moonray commented
FileSize
contextual_comment_wrapper.patch1.25 KB

Updated patch for updated Drupal version.

Jacine’s picture

Jacine
she/her
Location New York
CreditAttribution: Jacine commented

+1 for this patch.

I would like some clarification on this as well.

It really sucks not being able to use this in more places, namely right after <body> and and in comment wrapper.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented 
+++ modules/contextual/contextual.css	5 Jan 2010 08:17:25 -0000
@@ -63,6 +63,7 @@
+  text-align: left;

why that?

Powered by Dreditor.

moonray’s picture

moonray CreditAttribution: moonray commented
FileSize
contextual_comment_wrapper_1.patch754 bytes

Sorry separate issue. Breaking it out, and rerolling.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Reviewed & tested by the community

Thanks!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, contextual_comment_wrapper_1.patch, failed testing.

moonray’s picture

moonray CreditAttribution: moonray commented
FileSize
contextual_comment_wrapper_2.patch710 bytes

OK, let's try with an up to date version.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs work » Reviewed & tested by the community

Odd. Still good.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, contextual_comment_wrapper_2.patch, failed testing.

Jacine’s picture

Jacine
she/her
Location New York
CreditAttribution: Jacine commented
Status: Needs work » Needs review
FileSize
contextual_comment_wrapper_2.patch.patch786 bytes

Just rerolling @moonray's patch to see if the testbot's problem was with the way it was generated. No changes from his patch.

Jacine’s picture

Jacine
she/her
Location New York
CreditAttribution: Jacine commented
Status: Needs review » Reviewed & tested by the community

yay, green.

realityloop’s picture

realityloop
he/him
CreditAttribution: realityloop commented

#10: contextual_comment_wrapper_2.patch.patch queued for re-testing.

Dries’s picture

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

Looks like we need to document these variables in the phpDoc.

Also, reading the phpdoc, what does it mean to 'wrap comments'. Weird name, but probably separate issue.

Jacine’s picture

Jacine
she/her
Location New York
CreditAttribution: Jacine commented
FileSize
674546-14.patch1.73 KB

Yes, "wrap comments" is a little odd, but then again so is this whole template file. ;) I changed it to: "Default theme implementation to provide an HTML container for comments." I also added docs for the title_suffix/prefix. Let me know if it needs anything else.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Reviewed & tested by the community
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

This doc looks consistent with what's in node.tpl.php.

Committed to HEAD. Thanks!

Status: Fixed » Closed (fixed)

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