Configurable number of "recent comments"
karna@drupalcon.org - September 1, 2006 - 20:38
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | comment.module |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
The number of items shown in the 'recent comments' block is hard coded to be 10. Please make this configurable as for other similar blocks.
I changed the hard coded '10' in the theme_comment_block function to 5, and it works for me now, but I think it would make drupal more consistent when this is configurable as well (just as active forum topics).
Thanks, karna
#1
new features go into devel version.
#2
(Moving x.y.z version to a real version number) Feature Request => 6.x-dev
#3
Attached is a patch against release 5.1 that makes the amount of recent comments displayed in the block configurable by the site administrator.
Your feedback is very welcome.
I tried to create a patch against CVS HEAD as well. However, the entire comments module is broken for me (the version that is in CVS now) so I can't verify if my changes work (or if they would break the module even more).
Some screenshots here: http://www.guusbosman.nl/article/1730
#4
The patch in #3 is a start but I think it could use a bit of cleaning up.
Having a parameter for the number of comments to retrieve in comment_get_recent() is probably still useful. It might be better to change the default to NULL, and then only use the configuration variable if the parameter is NULL.
Rather than hardcoding possible values using the select box, it would be better to use a text input and let the user exactly specify the number of comments imo.
Finally, there are a few coding style errors and this isn't a true patch. Please see http://drupal.org/node/318 and http://drupal.org/patch respectively.
#5
Chris, thanks for your feedback. Good advice, didn't think about such a NULL construction.
** @param $number (optional) The maximum number of comments to find. If not set,
* the number configured by the administrator will be used.
* @return $comments An array of comment objects each containing a nid,
* subject, cid, and timstamp, or an empty array if there are no recent
* comments visible to the current user.
*/
function comment_get_recent($number = NULL) {
if ($number == NULL) {
$number = variable_get('comment_block_display_amount', 10);
}
I have no strong preference for either select box or a text box but I saw that a select box is used for similar goals in a couple of other places (like admin/content/aggregator/settings and admin/content/node-settings).
I'll take another look at creating a patch (I haven't submitted patches before), and include the NULL changes above. Are there any specific coding style errors? I tried to stick to the code conventions but must have overlooked some things.
#6
Yeah, but I would check for the NULL with the type-safe === operator.
Sorry, I should have been more specific about coding style. I would just capitalize "number" in your comment, or add a $ in front of it.
I think the text box is better; if a user wanted 12 comments for example with the select list it wouldn't be possible. But you're right that in other places a select box is often used.
#7
Attached an updated patch.
I respectfully disagree with Chris and would rather keep the drop down list with values instead of using a text box. This is more consistent with the rest of Drupal and it leaves less room for the user to make typo's.
However, I've adjusted the list with actual values in the drop down so users can now select all values between 1 and 20, as well as 25 and 30.
#8
I applied this patch and thought all was well, until I noticed my forum comments missing completely, and actually the enter forum is disabled. In modules, it is now disabled and can't be reenabled cuz it says Depends on Comment missing. I restored my original backed-up comment.module, then several others, then the whole of the drupal directory. What did it changed in the database that needs to be restored?
Drupal 5.1
MySQL database 5.0.32
PHP 5.2.0-8+etch1
#9
srlinuxx, I read your comment only now, sorry. The patch makes absolutely no changes to the database -- it only affects the configuration screen and the query that is used to generate the Recent Comments block.
Could your problems have been a side effect of other changes that may have been made to your code base?
#10
The patch works fine.
#11
<?php+function comment_get_recent($number = NULL) {
+ if ($number === NULL) {
+ $number = variable_get('comment_block_display_amount', 10);
+ }
?>
That check should not be there. Instead, call comment_get_recent() using:
<?phpcomment_get_recent(variable_get('comment_block_display_amount', 10));
?>
Should it be 'Amount of' or 'Number of'?
#12
Can't wait to have this feature :)
+1 for the idea.
#13
Another vote for this.
Would be nice for both 5.* and 6.*
#14
Fixed the following issues:
- Renamed "Amount" to "number" in various places -- Dries is right; "amount" refers to something that you can't count (like amount of space), vs. "number" which are things you can (number of people).
- Made the adjustment Dries asked for so that comment_get_recent was passed in the variable.
- Moved the setting from the comment settings page to the recent comments block configuration page; this is more consistent with the way forum.module does its block.
- Pulled the drupal_map_assoc back in as default value and removed the extra function; this is more consistent with the way this function is used throughout core. At some point it might be cool to abstract this so that _all_ blocks that display a "number of foos" selection with the same values.
HOWEVER. :) For some reason, it's not showing up. Forum module (which I used for inspiration) has the same problem. Any ideas??
#15
I suspect it's blocked by http://drupal.org/node/152585
Great theoretical improvements though.
#16
Hey, thanks! Yeah, that was totally it. :)
Ok, here's a patch that should work, as long as the above patch referenced by ChrisKennedy is applied.
#17
review steps taken:
* clean drupal6 install.
* applied http://drupal.org/files/issues/blocks_fapi3_fix.patch
* applied this patch.
* enabled recent comments block
* created a new node
* commented on it
* changed comment block settings
* watched the number of comments in the block change
settings are in a sane place. The drop-down is long, but gives plenty of options with no room for error. works as advertised.
RTBC.
#18
Didn't make it into D6, moving.
#19
This doesn't change any apis - and it's basically just a usability improvement right? Dropping version back to 6.x and status to review just in case, since it'd be a small thing to commit with no side-effects I can see.
#20
Still applies with offset, bumping to 7.x
#21
Still applies with offset.
#22
I just tested this, it still applies with offset. I really like this patch- I'd prefer a text input box, though.
#23
I've made some small changes and committed this patch to CVS HEAD. Thanks.
#24
May I suggest this small follow up patch?
Number of comments to display in block with recent comments. is perhaps not as clear as it could be.
#25
RTBC, that's a good text change.
#26
Yep, much better text.
#27
The updated text is much clearer. Thanks keith.smith.
#28
Committed to CVS HEAD. Thanks.
#29
Automatically closed -- issue fixed for two weeks with no activity.