The "#" marking comment permalinks is just weird and untranslatable

This is the $permalink for the comment, not the number of the comment as we might expect - i.e. the number of the comment in one particular thread.

The "hash only" display is coded in core:

$variables['permalink'] = l('#', $uri['path'], $uri['options']);

This makes sense as the accurate alternative is...

$variables['permalink'] = l('#' . $comment->cid, $uri['path'], $uri['options']);

..which doesn't make sense because comments will show their cid, so if its the 588th comment for the whole site, but the first comment for a node it will show #588, which is potentially confusing to site users.

I would suggest we text-image replace this with an icon or override template_preprocess_comment and use the actual word "Permalink".

Let's fix this before Bartik goes into core.

What do you mean fix it? What is the issue here?

Critical?

The issue is that for each comment, there's a # next to them, and no number.

That just seems weird to me. I expect a number, and for the # to be hidden — be there, but not displayed visually. At least that's what I think I expect.

the # is not intended to indicate a number ... it is intended to indicate a permalink. I agree that its not very good at that, but that is the core output. I think we should

a) open an issue in core to make that output clearer in some way
b) use Jeff Burnz suggestion and come up with an icon of some kind that indicates permalink

In any case, I agree that this is not critical since all we are doing is displaying the core output as-is ... this shouldnt hold anything up

Heres a couple of things:

1 - the permalink is already being printed - its the comment title link
2 - neither of the links ($title or $permalink) have anything to indicate they are permalinks, its just assumed

So - thinking we can remove it, looks very much optional?
We could add rel="self bookmark" - the proposed hReview permalink microformat?

webchick also didn't like this:

for the "comment arrow", see #845904: Problem with comment arrows?

1. Not sure about that, it might confuse users, since the CID is the comment ID, so it will be something like #581 if this is comment number 581 for the whole site - that said if it was less prominent both in position and color/contrast and has the hover title "Permalink" it could work.

2. Yes, it is odd, I don't like it at all.

3. Links should be flush aligned left or right, they look a bit "floaty in space".

as for the links — that's in another issue. #846014: Check/fix alignment of links in Comments Ah, webchick and her screenshots that put multiple issues into one image. Didn't I read something about not bundling issues together on her blog somewhere. :D

As for the point of this issue, what should we do about that weird # floating there. There isn't a quick answer. We could make it light grey, and smaller... but really it's just weird and should be fixed in core somehow.

CSS styling is a Bartik thing. Changing the HTML markup is better done in core.

Ideas?

Or, yeah, we could just hide this. It's not a known design pattern to have a "#" to hold the permalink. Maybe really that's the solution — to visually hide the anchor.

1. There must be a visual element to link to the comment anchor. It's vital to be able to link directly to a point in a thread

2. Titles for individual comments in a thread are optional and cam be turned off by the admin so we can't count on using that for the link.

3. Numbered comments make no sense on threaded discussions
though datetime may work

I've seen the # convention around in heaps of discussion boards and it made lots of sense to me - but that's because I learned some HTML long ago and recognize #anchor as having a meaning on the web.
If you've never seen that in a web page or an URL then I guess it's not 100% intuative - but I've always thought if it as one of the more natural evolved conventions out there.
I've seen a small speech bubble icon used for this purpose also - which is ok - but I still prefer the #anchor like this
- it distinguishes between an outgoing text link and a link "to here"

Any alternative to the current convention would deserve a lot of investigation

FWIW, in my screenshot I didn't mean #2872871, #2387922, #23872823 (the comment ID). I meant #1, #2, #3... (the comment count). Like the issue queue.

I think that might be the best suggestion so far ... I'll play with this later tonight

EDIT: typo in this patch ... check out #17 instead

This patch takes webchick's suggestion from #14 ...

screenshot:

Nice, I like the way that looks. Good UX and good styling. I don't have time to review the patch right now; hopefully soon.

bump

Why do I see the #?

@aspilicious ... clear your cache?

I just did a fresh d7 install, applied patch and left a comment:

Nice! Even works in IE7-IE8!

But...

It doesn't work in rtl...
So back to needs work

this looks much better in RTL

The # part is rtbc.
But IE8 and IE8 compatibility mode (ie7) has still some issues with the comment box.

Attached a screenshot from the latter.

spoke with aspilicious on IRC ... the missing left borders seem to be caused by this patch. The messed up input formatting area is not. I'll work on this tonight.

A question/bikeshed - should this numbering be done in the bartik_process_comment in order to keep it bartik specific or should it be done elsewhere to make it the general case?

nbz: theoretically the numbering is actually the way that bartik decided to handle the "permalink" html that core spits out. There are certainly ways of using that markup that has nothing to do with the "count." Thats my argument for leaving it in Bartik, but I could certainly be swayed that the "count" is going to be such a common use case that we can move that into the comment module (or wherever) and let other themes use a theme function ...

My vote is 51/49 in favor of leaving it in Bartik's perprocess functions

Definitely Bartik. It's too late to make changes to the global HTML that Drupal spits out, unless it's a legitimate bug fix. It screws the people who are trying to upgrade their themes right now in preparation of Drupal 7's release.

Sold to the webchick in the first row!

this patch includes a little something extra for IE7 ... it's amazing how much I love IE7 (for not being IE6) and hate IE7 at the same time.

I <3 bleen.

Don't love bleen to much there is still a bunch of work to do to improve Bartik on the IE side of life.
you spoke about IE6, then I realised that I had windows xp mode...

And then I saw this needs work :)

But I admit you already did a great job even in IE6. (no errors on this picture just a screenshot to confirm how awsome it is)

Before and after on PPC OSX 10.5 using FF3.6.7 and Safari 5.0

EDITED: Before and after are different but this is not the actual result, you need to clear the cache.

The '#' permalink is above the name and outside the comment box in the before. It is above the title and inside the comment box in the after. I'm really green but ready to learn so please have patience :)

I have patience, I was just saying you need to clear your cache to see the actual result. You do this by going to configure, performance, clear all caches :)

Ah! Thank you aspilicious. After clearing caches I now have same results as you. New before and after:

Just a teeny-tiny tweak for IE6(RTL) and I think now we're good to go...

bump

Will look at this soon as possible :)

Needs a newline in the ie6 rtl version.
Except for that this looks rtbc. But look at the comment comment box in in IE6 rtl. The "name" and "subject" are on the same line. Related?

This takes care of both issues in #42

Looks great all browsers look how they suppose to look ;).

Needs sign-off from Jen, but #39 looks good to me!

spoke to webchick in IRC ... she meant #43 looks good to she

Mmm, does this work with threaded comments? Looking at the code, it looks like a comment's number could change when a comment is inserted in a higher thread. This could easily "break" conversations that reference those numbers like we do in the issues. We'll want to verify that, IMO.

This looks great to me. +1
Go ahead and commit it.

Yeah, Dries' point in #47 is right. There's a numbering problem with threaded comments.

:(

Actually, let's debate whether this is a blocker or not. #number is so much better than # by itself.

UX team, what do you think?

I've been talking with jensimmons on IRC about this ... its seems to me we have a few options:

1. leave this as is - comment #s can change
2. Use some sort of threded count: ex. #3, #4, #5, #5-1, #5-2, #5-2-1, #5-3, #6
3. Use the comment ID instead of a count: ex #313, #319, #394 (reply to #319), #325
4. Just use the comment number form the permalink url: #1, #2, #3, #6 (reply to #3), #5 (reply to #3), #4, #7
5. Dont show numbers at all - in this case what should we do with the permalink
6. ?

Thoughts?

#4.

I say, let's number them 1, 2, 3, 4, 5, 6, etc. Not #5-1, #5-2, etc ('cause what about nested comments that go 3 and 4 levels deep — is that #5-1-2-1?) And not with the comment ID ('cause that doesn't really solve any confusion).

Let's instead number them according to the order in which they were added to the discussion. That's how the URL's work now. The first comment is at 1#comment-1; the sixth comment is at 6#comment-6.... let's therefore number them 1 and 6.

The comment numbers will then be out of order on the page — but in chronological order. Which is actually kind of handy. You can see the order in which the discussion happened. I read deeply threaded discussions on groups.drupal.org (that nyc group) and find myself frequently confused because I *can't* tell which comment was added after which other one (or before). I'll hover over the URLs just to tell.

So visually, I'm talking about this:

It's worth noting a comment form chx (in IRC) that if we use a static counter (with any of these solutions) we need to bear in mind things like two nodes (with comments) showing up on teh same page using panels (or whatever)

If Jen's suggestion (#52) is the way to go (and I'm stil only 82% convinced it is...) I'm not sure how this could be accomplished. If anyone wants to point me in the right direction I'm happy to make a new patch... anyone? anyone?

Let's review the list one more time:

1. Leave this as is - comment #s can change

I guess that means as in #43? In that case this is a no-go, because it just doesn't work with threaded comments. Plus if you use a static see comment #54.

2. Use some sort of threded count: ex. #3, #4, #5, #5-1, #5-2, #5-2-1, #5-3, #6

That sounds doable.

3. Use the comment ID instead of a count: ex #313, #319, #394 (reply to #319), #325
4. Just use the comment number form the permalink url: #1, #2, #3, #6 (reply to #3), #5 (reply to #3), #4, #7

Those two are either the same or very different depending on how you look at it.

First, to clarify, the comment ID *is* the number from the permalink URL, so worded like that, those two are exactly the same.

(3) can be done easily.

I guess that (4) means: sequentially number the comments in the order they are entered. What happens if a comment gets deleted or unpublished? Do you renumber?

a. If you renumber, how is that a permalink? More importantly, if you want to compute this information, you have to take into account that comments might get paginated, and that comments on page 2 can change the numbering of comments on page 1. You don't have enough information in the theming layer to do that, this is a no-go.

b. If you don't renumber, you have to keep this information somewhere, because it is permanent. We don't store this information, and it's way too late to add it in the code. This is a no-go.

Conclusion: (4) is a no-go.

5. Dont show numbers at all - in this case what should we do with the permalink

Hide the # by default and show it only in hover?

Clarifying the list from #51 (based on comments in #56):

1. Simply show the comment numbers iterating in the order they appear and live with the fact that numbers will change (based on replies and on deletes) (as the patch in #43 behaves - however based on the comment from chx (#54) I would need to rework that code if we went with this option)
2. Use some sort of threaded count: ex. #3, #4, #5, #5-1, #5-2, #5-2-1, #5-3, #6 - its worth pointing out that every deep threads would be pretty awful: ex: #6-3-1-1-2-1-1
3. Use the comment ID instead of a count: ex #313, #319, #394 (reply to #319), #325
4. sequentially number comments in order that they were created, not in the order they appear on the page: #1, #2, #3, #6 (reply to #3), #5 (reply to #3), #4, #7 (Still have the problem with deleted comments here AND as Damian Tournoud confirmed in #56, I dont think we have enough info to do this at the theme level. - I would still consider this a "permalink" if we figured out how to do it because the href would be the actual permalink)
5. Dont show numbers at all - in this case what should we do with the permalink
6. ?

All this said ... I think the only workable solutions are going to be either 3 (use comment id) or a variation of #5 (maybe with Damian's suggestion of only showing on hover) and displaying a link that says "permanent link" or something and avoid numbering all together.

Based on recent comments, it indeed sounds like the comment numbering scheme method is a no-go. Ugh. Sorry, bleen. :\

One alternative is to make something else the permalink, like the date/time. That seems to be what wordpress and twitter does, so there's precedent for this (where i've never seen a "#" floating in space be a permalink).

I've seen sites where it literally says, "(permalink)". This would make it translatable, too, so people could switch it to a different word if they like.

Michelle

Is it possible to use a combination of both 1 & 3? Keep the comment ID as the permalink target but to increment the comments on the page.

This way the end user will see sequential numbers, making logical sense, and the permalink will remain static. Can this be done at theme level?

@cubix: no, that's (1) (or (4)), and it's not possible.

@cubix: that would be ill-advised too because people would likely be referencing other comments by number and if those numbers changed than teh reference wouldnt make sense ... for instance, this comment I'm writing is in response to #60 ... what happens if tomorrow #60 changes to to #61 because there is a new "reply" farther up the thead

re #59: Your suggestion is a variation on -5 from the list in #57. My only worry there is the "permalink" is IMO a very geeky word and despite its accuracy is a bad choice for the masses.

In truth, the comment's "title" is already a link to the permalink ... we could simply not output the "#" Thoughts?

Liking the sound of #58, indeed Wordpress and Twitter do this exactly. Pretty easy...

$uri = entity_uri('comment', $vars['comment']);
$vars['created'] = '<span class="date-time permalink">' . l($vars['created'], $uri['path'], $uri['options']) . '</span>';

Something like that?

Considering that the comment title already links to the comment permalink, I'm leaning towards simply not displaying the "#" ... but I could certainly be swayed by a good argument for making the date/time link... so, what's the good argument?

The good argument for not depending on the comment title for the permalink is that comment titles can be disabled. This would break permalink functionality. Like it's broken right now in BlueCheese theme for example. See http://association.drupal.org/node/624.

As far as using date/time or using a string called "permalink", either is fine with me. I defer to the Bartik maintainer. Probably add the permalink URL to the comment.tpl.php as a variable (if it isn't already) so that themers can stick it wherever they want.

That certainly qualifies as a good reason ...

I think that linking the date/time is just weird. So...

oops .. left a DSM() in that last patch

Great. But let's go ahead and add this new variable to core (http://api.drupal.org/api/function/template_preprocess_comment/7 + top of comment.tpl.php), rather than just to Bartik. It's an optional API "addition" so doesn't break any freshly ported themes.

Also, could you please post a screenshot so this patch can be visually reviewed?

screenshot in #67 (forgot to set input format) ...

as for the variable ... it already exists in core comment.module:2219:
$variables['permalink'] = l('#', $uri['path'], $uri['options']);

we're just changing it for bartik

Oh, I see. Weird. I wonder who thought an untranslatable '#' was a good idea for a label for permalink. :\

Anyway, totally separate issue.

Well, actually, no it's not.

We should fix this at the source. (but also make sure it looks good in Bartik)

This patch makes the permalink translatable in the comment module and makes it look pretty in Bartik:

+++ modules/comment/comment.module	4 Aug 2010 01:46:08 -0000
@@ -2215,8 +2215,10 @@ function template_preprocess_comment(&$v
+  $variables['permalink'] = l(t('permalink'), $uri['path'], $uri['options']);

Normally single words would start with a capital letter, not all lowercase. Use CSS to style (text-transform).

Perhaps we should be using rel='bookmark' on these permalinks?

Powered by Dreditor.

both excellent points ...

hmmm ... one more try

Tested in Safari, Firefox (Mac) and IE6, IE7 on Windows. Works well.

Committed to CVS HEAD. Thanks.

This just created a problem with the H3 title in comments which has the class of .permalink - so now comment titles are too small and lowercase. Also this is a naked anchor, this should be wrapped in some markup like a p element like the rest of the attribution elements.

The class to target the new permanlink should have been #comments .attribution .permalink {}

patch attached

You can't set the font size on the anchor anymore, you need a class such as class="comment-permalink" on the p wrapper and set the font size on that, i.e. the same as #comments .submitted .comment-time

Is this better?

On another note: We don't use the class .permalink that is attached to both the comment header and the actual permalink anywhere for styling. Do we need it at all?

That'll do it, lets get this baby in so we can re-roll the other patches for Bartik.

Committed to HEAD. Thanks!

Phew, good work team, thanks!

Really odd that this variable didn't make it into Garland. :-\ #975254: Garland completely ignores permalink variable for comments