Threaded comments

[BrianKoontz]

Implementation of threaded comments.

Related tickets: #400

[BrianKoontz]

Implementation in changeset [281].

[BrianKoontz]

Multiple changesets: [280], [281]

[BrianKoontz]

Changeset [282] (Minor edit to updater code)

[BrianKoontz]

Re-opened for further enhancements...stay tuned!

[DotMG]

Consider merging your CSS rules:

echo '<div class="commentthread '.$levels[$comment['level']].'">'."\n"

and

.commentthread {
	background-color: #EEEEEE;
	padding: 10px;
	border: 1px solid #CCCCCC;
}
.commentL1 {
	margin-left: 15px;
}
.commentL2 {
	margin-left: 30px;
}

[DotMG]

Or simply by aligning the common rules in one declaration

.commentL1, .commentL2, .commentL3, .commentL4, .commentL5, .commentL6, .commentL7, .commentL8, .commentL9, .commentL10 {
	background-color: #EEEEEE;
	padding: 10px;
	border: 1px solid #CCCCCC;
}
...

[DarTar]

Actually I have been discussing with Brian an alternative (nested) layout for threads which would only need two CSS selectors for threads of any size.

[anonymous]

Changeset [289]: Threaded comments now nested per Dario's suggestion. Rudimentary CSS change made for testing; someone feel free to dress this up (I'm not much of a CSS person).

[BrianKoontz]

Changeset [290]: Alternating shading for comments.

[DarTar]

Well done, off to test it- I'll play with the CSS soon

[DarTar]

Brian, I've fixed in [293] an input element that was not correctly closed. My webdev toolbar also reports invalidly closed div's.

My suggestion (a best practice to be used also when building a page template): every time a div is closed, generate an HTML comment with the corresponding id so we can immediately see in the source whether div's are correctly closed and where, e.g.:

</div><!--close comment_9-->

[DarTar]

Also, consider adding titles to form elements wherever it's possible. See #388.

[DarTar]

There's another glitch you should fix: when you create a thread like the following, the alternate CSS selectors are not correctly assigned (bold is for .comment, plain is for .comment2).

Current:

1.

1.1

1.1.1

1.1.1.1

1.2

1.2.1

Should be:

1.

1.1

1.1.1

1.1.1.1

1.2

1.2.1

[BrianKoontz]

Replying to DarTar:

There's another glitch you should fix: when you create a thread like the following, the alternate CSS selectors are not correctly assigned (bold is for .comment, plain is for .comment2). Current: 1. 1.1 1.1.1 1.1.1.1 1.2 1.2.1 Should be: 1. 1.1 1.1.1 1.1.1.1 1.2 1.2.1

Actually, I had it the latter way too! I can't figure out which is best. In the "big scheme of things", choice 1 is most logical. Choice 2 blurs the transition from 1.1.1.1 to 1.2 if you're reading the comments that way. Guess we'll try both ways and see who likes what.

[DarTar]

children of the same parent should have the *same* color in threaded mode, right?

[DarTar]

Brian, see the screenshot I've just attached to this ticket.

[BrianKoontz]

Replying to DarTar:

Brian, see the screenshot I've just attached to this ticket.

Not according to the link you pointed me at to use as a model:

 http://meidell.dk/archives/2004/09/04/nested-comments/

Personally, I don't like the alternating colors. It's quite distracting, and to me duplicates the purposes of the nesting (to point out logical orienation of message threads).

[JavaWoman]

Brian, in testing I discovered some nesting and XHTML validation problems; fixed in changeset [308] - see there for comments.

BTW, an "in reply to" link added to comments would be useful (so you can jump to that comment), especially when they are not shown in threaded format. Doable?

[JavaWoman]

One more XHTML fix in [309].

[BrianKoontz]

More fixes and enhancements in [461] (JW, the "in reply to" links shouldn't be difficult...I'll work on it).

[DotMG]

(In [465]) refs #418, [461]

Minor correction in phpdoc. Moved inside correct templated @var string.

[DarTar]

Comment display icons

[DarTar]

Brian, what about using icon links (with titles) to toggle the different comment displays?

[DotMG]

(In [466]) Cleaning up (coding guidelines). refs #418, #438, [461]

[DarTar]

(In [469]) Adding icons for threaded comments navigation, refs #380 and #418

[DarTar]

(In [470]) Adding CSS selectors for comments navigation, refs #380 and #418

[BrianKoontz]

(In [483]) Various enhancements for nested comments (refs #418): --Implemented stateful icons for various threading modes --Using new FormatUser() method

[DarTar]

(In [486]) reverting [482] that inadvertently replaced tabs with spaces (Brian make sure you check your editor settings), refs #418 and #221

[DarTar]

(In [487]) Reimplementing FormatUser() as per [482] (see [486]), refs #418 and #221

[DarTar]

(In [491]) Fixing XHTML validation issues with threaded comments (class parameter was not closed and div's were incorrectly nested); moved navigation div inside header, refs #418

[DarTar]

(In [492]) Slightly improved styling for comments header, refs #418

[DarTar]

(In [493]) Changed wording of comment-related options in UserSettings to match the navigation options, refs #418

[BrianKoontz]

Brian make sure you check your editor settings

Tabs only on this project...no tabs on other projects...what we need is a "tace" (or "spab" -- pick your poison): A combination tab and space that performs double-duty across all projects that fall on one side of the issue or the other!

[DarTar]

(In [501]) Fixing invalid XHTML and adding styling to comment options in UserSettings, refs #418

[DarTar]

(In [502]) Fixing minor XHTML validation issue, refs #418

[DarTar]

(In [508]) Minor cosmetic changes for threaded comments, refs #418 and #380

[BrianKoontz]

(In [513]) Cleared session comment settings on logout; per page comment formats now saved correctly across the session (so when you return, the comments are displayed the same way); "in reply to" displays the parent author (thanks JW for the suggestion); minor spelling edit; revised UserWantsComments() method to check for system-wide and user comment style defaults; comments now displayed starting with light comment on dark background; siblings are now the same color...entering a new comment does not change shading scheme (thanks DarTar for the last two suggestions). Refs #418.

[BrianKoontz]

(In [514]) Code clean-up, refs #418.

[DarTar]

(In [518]) Fixing style and adding comments in the stylesheet for threaded comments, refs #418 and #380

[DarTar]

Brian, I just realized that the icons for the flat display mode should be swapped. If the triangle points towards newest posts, then sort_asc.gif should be used for Flat (newest first).

[BrianKoontz]

(In [544]) Deleted comments no longer returned as part of RecentComment/RecentlyCommented queries. Refs #418. (Thanks Nils!)

[BrianKoontz]

Reversed sort asc/desc icons to better reflect their functionality. Also reversed order of icons so that earliest first is the first icon (probably the most useful of the two). Refs. #418. Changeset [545].

[BrianKoontz]

If there are no oustanding tasks for this ticket (other than cosmetic changes, which can be made anytime), I'd like to close this out.

[DarTar]

(In [556]) Removing PHP notices from threaded comments, refs #418

[DarTar]

(In [557]) Reverting partial fix in [556] because it breaks functionality, refs #418

[DarTar]

(In [569]) Minor change to stylesheets (trimming end of file space) to test SVN connection, refs #418

[JavaWoman]

Reopening... we're not done yet.

Bugs: 1. I mentioned this on #wikka before, but the button that looks like "make a comment" but of which the title says "Threaded" cannot toggle between flat and threaded mode. If I start out in flat mode, I can switch to threaded mode - and then I'm stuck there.

2. Also, we need two different buttons that visually indicate the difference between "flat" and "threaded" mode, instead of one that commonly means "make a comment": the usage of this button is very confusing.

Small enhancements (usability): 3. Third, the configuration comment only mentions what the "default" stylesheet for the comments is but fails to mention what the provided alternative is (or even that there is one. Both should be mentioned, with an indication as to their difference. Mentioning only the one, it doesn't look as though an admin has any choice.

4. Finally, I'd like to see the name "nested-comments.css" changed to "indented-comments.css" which more clearly indicates the visual difference with "boxed-comments.css".

[JavaWoman]

Trying to tweak the "indented-comments" style to get a bit more usable layout (not a HUGE author and date below each other above a normal-sized actual comment, for starters, and a box around each separate comment) - but I find I cannot get the layout I want (boxes indented below each other) because the comments are still physically nested inside each other.

The solution with just a stylesheet to show a normal indented layout is just not a real solution.

I really, really want threaded comments for my site (I'm not going to put it online without that), but I feel I'm going to have to heavily hack the code before I can put it online. Sorry, Brian, you had it right to start with...
What I need is a configuration parameter that results in either separate boxes (to be indented) or boxes nested inside each other (which I wouldn't use anyway). And I want to have a signature (--, name, timestamp) within the comment body (at the end), not as a separate emphasized "header" - that also requires a code change (and should be controlled by a configuration setting).

[JavaWoman]

Another bug (small): boxed-comments.css still contains two rules that just read "none", which is invalid.

If you want no styling at all, just use the format selector { }, which is valid.

[JavaWoman]

Replying to JavaWoman:

Another bug (small): boxed-comments.css still contains two rules that just read "none", which is invalid.

Yet another:

.commentheader is missing from boxed-comments.css - that should also be included (as an empty style, as indicated above)

[JavaWoman]

Replying to BrianKoontz:

(In [513]) (...) "in reply to" displays the parent author (thanks JW for the suggestion);

By now, I see a style for .commentparent in both stylesheets, but no "parent author" let alone a link to the parent comment in the generated HTML. Any reason that was removed? It's especially important to have a link back in long, flat lists.

[JavaWoman]

Replying to JavaWoman:

It's especially important to have a link back in long, flat lists.

To clarify:
Just look at what Trac is doing here! It's a perfect example for what I'm referring to (though only backlinks are really essential, not forward links).

[BrianKoontz]

(In [697]) Fixed out-of-scope variable notice, refs #418

[DarTar]

(In [711]) Minor change to styling for comments form, refs #418

[DarTar]

(In [715]) Minor change to styling for comments form, see [711], refs #418

[JavaWoman]

(In [716]) * fix for NOTICE on opening new comment form

• added @@@ todo marker

refs #418

[OlivierBorowski]

(In [798]) Removed "none;" as CSS doesn't recognize such attribute. refs #418

[DarTar]

(In [916]) Minor changes to comments layout/style, refs #418

[DarTar]

(In [917]) Minor changes to comments layout/style, refs #418

[raffa]

sorry, didn't read all this, but:

(1) the icons are hardly visible (2) threaded view should be the default

based on the comments on the 1.1.7-docs version. cheers, raff

[BrianKoontz]

Dartar, any chance of darkening up the icons? They are rather light, especially gvien the low level of background contrast...

[BrianKoontz]

(In [1169]) Set 'threaded' as default mode, change DB storage type for default comment display as enum, minor fixes to default comment radio buttons on UserSettings page. Refs #418.

[BrianKoontz]

(In [1170]) Sanitized GET params, fixed initial display of page comments to display as user default. Refs #418.

[BrianKoontz]

(In [1171]) Revision count update. Refs #418.

[DarTar]

(In [1215]) Fixed notice in TraverseComments(), refs #418 and #496

[DarTar]

Brian, in my user settings, the comment layout preference doesn't "stick": when I come back to UserSettings no option is selected (even if the preference is stored in the DB).

[BrianKoontz]

Need to add 'status' field to installer switch statement; otherwise, upgrades from stable->unstable fail with 'status' column not found error.

[BrianKoontz]

Works with fresh 1.1.6.2 install, must have been something local to my dev machine.

[BrianKoontz]

(In [1495]) Initial port of threaded comments from trunk to 1.2 branch. There are some CSS issues, but I wanted to get this checked in before too many changes diverged from my set of changes. Refs #418.

[BrianKoontz]

(In [1497]) Fixed layout issue. Refs #418.

[DarTar]

light and kubrick need to be updated to display threaded comments in a nice way. We probably also need to rethink the "comment style" icons (maybe just remove this option for registered users as they can already set this in their preferences?)

[DarTar]

I am going to take care of the comment stylesheets. Brian what if we:

• leave comment display mode in the UserSettings only? This would keep the page layout cleaner IMO
• avoid bundling multiple stylesheets for comments and integrate the most appropriate one in the main theme stylesheet? We need otherwise to maintain 6 different stylesheets every time there is an upgrade!

[BrianKoontz]

Clarification on bullet #1: You mean remove the 3 icons to the right that switch between modes?

I'm certainly OK with #2.

[DarTar]

yes, that's what I mean

[DarTar]

Replying to BrianKoontz:

Clarification on bullet #1: You mean remove the 3 icons to the right that switch between modes?

so what do you think?

[DarTar]

Brian, are you ok with this before I proceed?

[BrianKoontz]

I personally like the icons, but if you feel they're unnecessary, then by all means remove them.

[DarTar]

Note to self - I still have to apply a few tweaks to the stylesheet!

[DarTar]

(In [1648]) Merging threaded comments stylesheet into main stylesheet (light template), refs #380 and #418

[DarTar]

(In [1649]) removing separate boxed/nested comments stylesheets (light template), refs #380 and #418

[DarTar]

(In [1650]) Adapted stylesheet for processcomment handler (light template), fixed broken layout in processcomment, now displaying the whole page before the commentform (which I think is important if one has to comment on the content!). Brian, note that because of this change when processcomment is loaded the user has to scroll to the bottom to see the comment form, we will need to find a better solution. Once we have settled on the final implementation I'll modify the other templates accordingly, refs #380 and #418

[BrianKoontz]

(In [1659]) Fixed notice; anonymous deletion functionality restored. Refs #418.

[DarTar]

(In [1662]) Fixed comments style in kubrick theme. Note: the comment navigation icons and the commentsnav div will need to be removed from the show handler and CSS. The label of the fieldset in the user settings can then drop the word "default". refs #418 and #912

[DarTar]

(In [1663]) Fixed comments style in default theme. Note: the comment navigation icons and the commentsnav div will need to be removed from the show handler and CSS. The label of the fieldset in the user settings can then drop the word "default". refs #418 and #912 and #380

[DarTar]

Brian, as agreed I am removing the controls to change the style of comments on-the-fly, as this can be set in the user settings. Could you please remove from the show.php handler all the code for setting/getting the comment display style via the form in div#commentsnav? If you don't mind I'd prefer you do this as you remember better than I do the parts of the handler that have been modified as part of the threaded comments implementation.

[BrianKoontz]

Replying to DarTar:

Brian, as agreed I am removing the controls to change the style of comments on-the-fly, as this can be set in the user settings. Could you please remove from the show.php handler all the code for setting/getting the comment display style via the form in div#commentsnav? If you don't mind I'd prefer you do this as you remember better than I do the parts of the handler that have been modified as part of the threaded comments implementation.

See #380.

[DarTar]

(In [1693]) Removing obsolete comment stylesheet option from config settings, refs #912 and #418

[BrianKoontz]

Updated milestone to 1.3.1