Comments for Activity 2.x
Scott Reynolds - March 5, 2009 - 07:50
| Project: | Activity |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Scott Reynolds |
| Status: | needs review |
Description
With the new direction, we have put the comments on the way side first to get the stuff running. Attached patches install the comment table just like in the 1.0 release and the JS and the form does much the same thing as 1.0.
Of course, the major difference is here we are doing it via Views. And so it begs the question, should this be a field in Views or should it be a style plugin?
The advantage to doing a style plugin is that you can control the view cleaner. Adjust how the fields are laid on on the screen. And it opens up theming possibilities.
On the other hand, is it really needed? Does it really add anything? Im not so sure.
For displaying see #391740: Need to display comments to activities
| Attachment | Size |
|---|---|
| activity.css_.patch | 764 bytes |
| comment_form.patch | 3.95 KB |
| comment_install.patch | 1.62 KB |
#1
#2
Postponed until we make sure that we're actually on the right track here. Let's get a solid framework down first and then start in on these features.
#3
Is there perhaps any update on this pls? Have tried the patch, but unfortunately not happening :)
#4
I've a working activty_comments module, I'll post it here, when I finish it - but I have to talk about this with sirkitree.
#5
Yes there have been many changes to the module since this original patch was made which is why I'm sure it applies no longer. If anyone has updated code that applied to the latest dev that would be most appreciated.
#6
initial commit, only for review
#7
added doxygen, next step is the default view code
#8
Hi alippai,
So excited, thank you. Alippai, installed and added 'Activity comments: Comment text' to the view. What I'm perhaps missing is the css in the activity views, so that users can add a comment?
Look forward to your updates :)
Lilian
#9
added the default view and @todo comments
#10
Liliplanet: this is still an alpha, or pre-alpha module which will add comment functionality (like on Facebook) to the activity module. I've attached a default view in comment #9, you can try it, but it's completely untested. I've more projects to work on today, so I'm slow now, maybe tomorrow I can work more quickly.
#11
Looking forward, thank you Alippai!
#12
better and more flexible views integration coming today
#13
So here is a better solution, it's uncompleted, but sirkitree you can see that I've changed the views integration from style plugins to fields. I don't know how to fix it, maybe we don't need to fix it, but now there is db_query call in pre_render.
This code needs review only by sirkitree or who wants to help with designing the db and the code.
#14
Hi,
So excited! Testing .. and form shows fine on view. When I add a comment ..
receive following error: "user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'WHERE 60 = unsupported type for db_type_placeholder' at line 1 query: UPDATE activity SET WHERE 60 = unsupported type for db_type_placeholder in /Users/local/includes/common.inc on line 3436.
This will be a fabulous enhancement, thank you!
#15
more work, there are now new fields, works only with js enabled
#16
missed the attachment
#17
Comments:
1.) remove all JavaScript and use AHAH (http://drupal.org/node/331941 and http://api.drupal.org/api/drupal/developer--topics--forms_api_reference....)
<?php$form['submit'] = array(
'#type' => 'submit',
'#value' => t('add'),
'#ahah' => array(
'path' => 'activity/comments/insert',
'wrapper' => 'activity_comments-' . $aid,
'method' => 'replace',
'effect' => 'fade',
),
'#submit' => array('activity_comments_form_submit'),
);
?>
and
<?php
/**
* Insert comments menu callback.
*/
function activity_comments_comment_insert() {
drupal_set_message('go');
// We're starting in step #3, preparing for #4.
$form_state = array('storage' => NULL, 'submitted' => FALSE);
$form_build_id = $_POST['form_build_id'];
// Step #4.
$form = form_get_cache($form_build_id, $form_state);
// Preparing for #5.
$args = $form['#parameters'];
$form_id = array_shift($args);
$form_state['post'] = $form['#post'] = $_POST;
$form['#programmed'] = $form['#redirect'] = FALSE;
// Step #5.
drupal_process_form($form_id, $form, $form_state);
$limit = $form_state['values']['limit']; // The form item will have a '#type' => 'value' called limit for this purpose
// Step #9.
$output = theme('status_messages') . theme('activity_comments_comments', $comments, $limit); // @TODO add in all comments for this activity here
// Final rendering callback.
drupal_json(array('status' => TRUE, 'data' => $output));
}
?>
2.) theme function for all comments needs to render ALL the comments and those that are below the limit need to be hidden by CSS. Add a simple link to then show All
<?php/**
* Views row style theme function for embed comments display.
*
* Don't add a wrapper around the comments, it breaks the AJAX functionality.
*
* @todo move to tpl.php file?!
*/
function theme_activity_comments_comments($comments = array(), $limit) {
$items = array();
$count = 0;
foreach($comments as $comment) {
if (++$count > $limit) {
theme('activity_comments_comment', $comment, TRUE); // where TRUE tells the activity_comments_comment theme function to hide it
}
else {
$items[] = theme('activity_comments_comment', $comment);
}
}
return theme('item_list', $items) . l('show all', '#', array('html' => TRUE, 'attributes' => array('class' => 'activity-comments-show-all'))));
}
?>
Write a JS file that uses Drupal.behaviors instead of document.ready(), and attach to show all a click function.
Drupal.behaviors.activityCommentsShowAll = function (context) {$('.activity-comments-show-all:not(activity-comments-show-all-processed)'.click(
// show the rest and flip the direction so next click will hide them
).addclass('activity-comments-show-all-processed');
};
3.) Remove drupal_add_js('jquery.forms') this is added by the expand_ahah process function, no need for it in the handler.
Ok so there is some of the issues I see. Not all code here will work as written, i have written them only here in this textarea not in an IDE. There are other issues as well with things like code style http://drupal.org/coding-standards
But this should get you started again.
#18
Ok so pretty excited about this. i reworked it and I think i got it setup so it should be easy to extend. I compressed the 3 fields (activity all link, comment form, and the comment listing) into one field. Its a form with a bunch of markup in it. Using form_alters you should be able to move things around pretty easily.
It also works without JS (even the CSS rules handle that case as well).
Uses AHAH for the form submission. Lil bit of JS to handle the Default text (focus, blur events) and the 'show all' link.
What I need help with is getting rid of the damn list styling...
.item-list ul.activity-comment-list {list-style: none;
margin: 0.35em 0 0 0.5em;
}
that didn't work. And any other styling, Im pretty blah at CSS.
Heres a screen shot
http://skitch.com/supermanscott/ndcc5/scotts-contrib
#19
Forgot to add in the additional permission to the form wrapper so anon users for instance couldn't post if they weren't allowed via admin/user/permissions
#20
I also like it better without the form label ('#title')
<?php$form['activity_form_items']['activity_comment'] = array(
'#type' => 'textfield',
'#maxlength' => 250,
'#size' => 32,
'#default_value' => t('Write a comment'),
'#attributes' => array('class' => 'activity-comment-text'),
);
?>
#21
Hey Scott and alippai, great work this. I'm going to try this on a fresh copy of my site but I'm getting the following when trying to post a comment:
warning: array_shift() [function.array-shift]: The argument should be an array in /Users/thomascermak/Sites/londonfuse/sites/all/modules/activity_comments/activity_comments.module on line 71.warning: call_user_func_array() [function.call-user-func-array]: First argument is expected to be a valid callback, '' was given in /Users/thomascermak/Sites/londonfuse/includes/form.inc on line 366.
Again, I'm going to try this on a fresh install.
#22
That has to do with the AHAH stuff and its pretty strange. It basically is telling us that Drupal.ahah didn't post the form values properly.
Have you used a previous version of this before?
Are you sure the activity_comments.js is the proper one? As the old JS file grab the form button and posted other values that wouldn't match what AHAH posts. And because I didn't change the menu path, it will cause problems. Problems just like this.
#23
Will check all these things later today. Thanks Scott.
#24
Ok spent some more time on this today
http://skitch.com/supermanscott/ndi5i/activity-comments
Improvements / Fixes
TODO
Delete comment. Write an menu access callback and leverage that when rendering the link in the 'form'JS cleanupThis is much closer to a finished product. Please take a chance to review.
@sirkitree do we want this to be rolled in with Activity download or would it be nicer as a separate project? drupal.org/project/activity_comments is open.
Also, I looked into auto growing the comment form like Facebook does. Involves a lil more JS but doable. There is a jQuery plugin that might do this ? sortof...
http://plugins.jquery.com/project/autogrow
Really what Facebook does is on focus add rows to the textarea and show the submit button.
Open Questions
#25
Probably also need some JS clean up
form_id = $(this).parents('form').parents('.activity-comments-hide-comments').removeClass('activity-comments-hide-comments');becomes
$(this).parents('.activity-comments-hide-comments').removeClass('activity-comments-hide-comments');#26
I've checked your code, it works perfectly except that it should not load all the comments. We should do that Facebook way, so load the extra comments via ajax.
On a page with 50 activities it means additional ~1-500 comments, which is not a good idea imho. (we need to load, process, transfer them and load them into the dom).
#27
Scott - another option for the facebook-style growing of the box is jQuery Elastic. I have successfully used this on my drupal sites for a similar function. I am using this on my site as a textarea and autogrow is enabled with no ability to turn it off.
I have a question about the intent of this module. Let's take an activity message like this:
"User XYZ commented 'this is a comment' about the node XYZ."
With this module, would an activity comment on that message be related to the activity message itself, or would it be posted as the next comment attached to the node XYZ? I.E. is it a new conversation about the activity message or is it a continuation of the conversation taking place on the node? I could see some value in both approaches, but for my personal use I'd want the conversations to relate back to the node itself.
Thanks in advance.
#28
I took my facebook feeds a 'representative' sample of the number of comments on messages. I had a total of 64 comments on 65 Facebook messages for an average of .98 comments per activity.
And thats Facebook. Who here knows of any site doing the same amount of traffic as Facebook? Not so sure its worth the effort.
I guess I would argue I don't want this performance improvement to hold it up. And it does add a level of trouble. It will slow down frontend performance (now its just a class toggle which means Show All is nearly instantaneous). You have to keep track of values and add another menu callback. There is some work there and its non-trivial.
Edit: The mode of the data was 8.
#29
The comment is attached only to the activity message.
#30
Confirmed. #24 fixed all my problems. Great stuff!
#31
Addressed my TODOs from #24
One issue is the delete. It uses the drupal_get_destionation(). Which is fine usually. But if you were to submit a new comment. Then delete that comment, the redirect would be to the JS callback not back to the View.
It is because the drupal_get_destination() for the AHAH callback. The form will have to pass in the current path to the AHAH callback somehow? Not sure on how to achieve just yet.
#32
Sorry guys, I've been extremely busy with client work and am just now able to spend some time on this, so first of great work! You've really been kicking ass lately Scott!
Secondly - I really think this is large enough to warrant another project. I'm already having trouble keeping up with my current module maintainer-ship responsibilities and another large functionality piece is more than I think I can handle.
That being said, Scott, you've pretty much taken over the 2.x branch (and kudos!) so if you would rather maintain the comment portion within this project as a submodule, I say go for it. I leave it to you as I'm only really able to take a co-maintainer-ship role on the 2.x branch at this point.
I'll try to give this a proper review tomorrow.
#33
alippai asked me what was missing for this project so far
Here is my short todo
1.) Delete after you post a comment redirects you to the JSON response page. yuck
2.) Should we toggle expand and collapse instead of a show all. FB doesn't do that, but...
3.) Do we make the comment a textarea with a format? This would allow for WYSIWG as well as the autogrow thingie. Perhaps a textarea is more appropriate its what is used for node comments.
At least for number 3, we need to come up with an answer only because as a Feature request this will surface, so we better have a thought out answer for it.
#34
Any thoughts on #33's three points. Would like to commit a patch this weekend
#35
2) fb actually shows you the original message, how many are in between, and then like the last one or two. I think it is sufficient to simply show them all and leave any toggling up to themers.
3) I do not think it being a textarea with a format is necessary, but a small textarea makes sense. K.I.S.S.
#36
I just fixed 1.) from #33 with a few lines of javascript. Since this bug only happens when js is enabled it is ok to fix it with js.
I added this at line 231 in activity_comments.module. This just passes the current destination to our settings in js.
drupal_add_js(array('activity_comments' => array('destination' => drupal_get_destination())), 'setting');This was added to activity_comments.js.
/*** Hanldes the destination for the delete link
*/
Drupal.behaviors.activityDestination = function(context){
//alert();
$('.activity-comment-wrapper .comment-link a').each(function() {
var href = $(this).attr('href').split('?destination=')[0] + '?' + Drupal.settings.activity_comments.destination;
$(this).attr('href', href);
});
};
#37
Other than the small typo in the main comment and not needing that commented alert(); - Looks alright.
#38
ok change that to this as the above didn't affect the delete link which was the problem. And it prevents it from running on all the comments that have already been processed
/*** Hanldes the destination for the delete link
*/
Drupal.behaviors.activityDestination = function(context){
$('.activity-comments-delete a:not(.activity-destination-processed)').each(function() {
var href = $(this).attr('href').split('?destination=')[0] + '?' + Drupal.settings.activity_comments.destination;
$(this).attr('href', href).addClass('activity-destination-processed');
});
};
still a problem with Views3. we hard code aid
<?phpreturn drupal_get_form('activity_comments_form', $values->aid, $this->options['limit'], $this->options['order']);
?>
Hopefully, i can improve once more then I will commit
#39
I think this is it. Would love a review
#40
It seems to be working nicely, I’ve got a comment field in place and they’re updating/submitting fine. Only problem I see is within the view itself, I’ve got “Error: handler for activity > updates doesn't exist!” showing in the Sort criteria and “Missing style plugin” showing for the Style, the latter then disables Fields. In terms of the actual output, it’s emitting a HTML List (it reverts to this because of the missing style?). Additionally, when there’s more than one comment added to an activity it creates duplicates of that activity in the stream (turning on Distinct rectifies that). As initially stated though, it’s definitely functioning ok and not spitting out any errors.
#41
Ahh right so that default view needs to go. I have forgotten about that.
What you really want to do to create a View is this
http://skitch.com/supermanscott/nek79/edit-view-all-activity-scotts-contrib
#42
Meant to note it needs work
#43
Ok so in order to post comments, you need to use the field as describe in 41 (http://skitch.com/supermanscott/nek79/edit-view-all-activity-scotts-contrib)
But I decided to add a 'feature' that was partically implemented before, create a View of all the comments. So now you can do this:
1.) Create a View just of Activity Comments: http://skitch.com/supermanscott/nexim/views-scotts-contrib
2.) Relate this View to bring in more fields/filters/arguments http://skitch.com/supermanscott/nexic/edit-view-comment-test-scotts-contrib
3.) Add in these fields: http://skitch.com/supermanscott/nexit/edit-view-comment-test-scotts-contrib
I also removed the broken default view that referenced the old style plugin. Fixed a number of bad views handlers (views_handler_(field/filter)_number doesn't exist, its numeric)
We are close. I would like to store the last comment id in the stats table so that it can be pulled in along side the Activity View if you aren't using the comment form handler.
Does all this make sense?