Hide '+1 Subscribe' comments

subscribing

+1, me too

+1,2349852057 !!!

subscribe

Not a feature.

$('#comments .comment .content > :not(.project-issue)').each(function () { $.debug($(this).text()); });

Not sure whether this can be done easily. $.text() will return plain-text content only, but a comment could easily contain an image containing further annotations along with a simple amendment "+1 for this!", so we'd mistakenly hide important comments.

we shoudn't hide the messages with images.

i think the easiest way for now, is to make a (configurable) list, of things we don't want to see, and if the message is FULLY made up of items on the list (spaces etc. not counting), and there is NOTHING else (except spaces etc.), we should hide the message. (won't fix #3, but that's an edgy case.)

Don't sweat the edge cases. A "+1 for FOO" is not an important comment IMO. Hide em all.

This would be good. Any message that matches ^[\W]*sub(scrib(e|ing)[\W]*$ and has only the one line should be killed.

Instead of completely hiding the message though, it'd be awesome if they were just collapsed, possibly even with a link to uncollapse them.

Also, "new" messages should not be hidden, even if they are subscription messages, otherwise this may cause some confusion when coming from dashboard.

Oh yeah, subscribe. ;)

+1 subscribe :)

definitely subscribing for this awesome new feature!

Try to filter this one ;-)

Actually I would be very interested in this functionality, every single time I see this I get a tiny bit frustrated. Because to me a "+1" comment equals, a uninformed and lazy response.

based on http://build2be.com/content/searching-issue-drupalorg-jquerygrep I cooked the following js

var list = jQuery('.comment');
var result = jQuery.grep( list, function(elem,i) {
    var $elem = $(elem);
    var $content = jQuery('.content', $elem);
    var text = $content.text();
    var subbing = text.match(/sub(scrib(e|ing))/i);
    // alert(text + "(" + text.length + ")");
    if (subbing && (text.length < 36)) {
      return true;
    }
    return false;
});

result;
$(result).wrap('<fieldset class="collapsible collapsed"></fieldset>').before('<legend><a href="#">subscriber</a></legend>');
$(result).css('backgroundColor','lightgrey');

It wraps the subscribe comments into a fieldset. Unfortunately clicking to open is not working as expected.

Checking the length is still a problem as the filtering in #6 tries to solve. Although changing the issue status should not be placed into a fieldset.

And what about the +1.

Haven't tried this, but based on the "edit issue settings" fieldset, looks like you need more than that:
<fieldset class=" collapsible collapsed"><legend class="collapse-processed"><a href="#">Edit issue settings</a></legend><div style="display: none;" class="fieldset-wrapper">...fieldset contents...
Where the div.fieldset-wrapper is the actual thing that gets hidden...

Hmmm ... what is "Edit issue settings" fieldset? jQuery('fieldset') does not reveal this :(

Eh.. sorry, I have no idea what I was looking at when I wrote that.. I thought it was this issue, but now it's not there. In any case, I just checked, and if you look at any of the collapsible fieldsets in the "create new issue" page, you'll see what I mean (try opening/collapsing them with firebug open).

clemens: have you got that working in dreditor.js? I'm not sure how to put it in there and make it work - I'm not very good with javascript yet.. if I can get it working to some degree, I can have a play with it...

ok, this works for me. hides all of sun's first 4 comments, and #10
I would also like to hide empty comments, but text.length< 30 doesn't seem to work. It would also be nice to really reduce the size of these a bit, and maybe make the "subscribe" blue a little less saturated, so it doens't break the flow of text so much.

/**
 * Hide subscribe comments
 */
// Drupal.behaviors.dreditorHideSubs = function(context) {
  var list = jQuery('.comment');
  var result = jQuery.grep(list, function(elem, i) {
      var $elem = $(elem);
      var $content = jQuery('.content', $elem);
      var text = $content.text();
      var subbing = text.match(/(\+1|sub(scrib(e|ing)|))/i);
      if ((subbing && (text.length < 100)) || (text.length < 20)) {
	return true;
      }
      return false;
    }	
  );

  result;
  $(result).wrap('<fieldset class="collapsible collapsed"></fieldset>')
    .before('<legend>Subscriber</legend>')
    .wrap('<div class="fieldset-wrapper"></div>');
  $(result).css('backgroundColor','lightgrey');
// }

Not sure where this should be, but if it goes in Drupal.behaviours, it breaks, because it runs twice.

btw, this works well on #1224666: [meta] Unofficial Drupal 8 Framework initiative:

41 positives (hidden) out of 92 comments.
1 false positive (#72 - tagging)
4 false negatives (2 empty, one "follow", one "go go go!"), all of which would be caught by a text.length < 20 or something.

I say that's close enough

The attached patch solves the issue change attrib problem (false positive from #19)

It does not solve the 'sub' only as #1224666-85: [meta] Unofficial Drupal 8 Framework initiative shows so the regex needs some tuning.

I set the length to 50 as an empty reply is 39 chars already

sub

The collapsed fieldset could contain a '+1' but the fieldset reports 'Subscriber'.

Should we split these?

Put a ? after the (scrib(e|ing)). I had a bar in the group, and that also works.
+1 is basically a subscription, just as annoying to read, leave it in.

@naught101 I had trouble with your regex somehow or my coding skills :-p ... thanks for the '?'

The text 'sub' in #21 is hidden due to the length I added in #20 which is now fixed by the '?'.

Please change the issue status to 'needs work' or 'RTBC' if applicable ;)

Works nice, also tried it on #34496: [meta] Add Flag module to allow users to subscribe/unsubscribe without posting a comment

This currently breaks the "new" link functionality, in that if you click "4 new" on the issue queue/post tracker, it sends you to node/#new, and THEN hides the subscriber comments, which moves the page relative to the viewport. The viewport needs to be refocussed on #new after the subscribe comments are hidden.

Could you

+++ b/dreditor.user.js
@@ -1360,6 +1360,52 @@ Drupal.behaviors.dreditorIssueSummary = function (context) {
+  $('.node.node-type-project_issue').once('dreditorHideSubscibeAndPlusOneComments', function() {

Is this filter too broad?

Could you check this on the mentioned page please?

[edit] Is the result of

$('.node.node-type-project_issue')

not good enough to check for the issue page?

Did you mean we have to filter like this

$('div.comment:not(.comment-new)')

Patch attached.

#28 No, I meant that the viewport is mis-aligned after the comments are hidden (ie. the browser viewport ends up looking at the comment box, or even the footer, because the rest of the page has moved up (shrunk). We need to move the browser viewport to #new again after hiding comments.

But yeah, I think it's probably a good idea not to filter new comments, as that's the main reason most power users are likely to click on an updated issue from their queue/tracker.

#28 indeed fails when linking to a comment directly; e.g. http://drupal.org/node/34496#comment-5011670

Would something like this be an option? http://plugins.jquery.com/project/ScrollTo

Aah ... a phone call helps a lot ... tnx helmo

$('a#new').position()
Object { top=28720, left=0}
>>> $('a#new').position().top
28720
>>> $('html').animate({ scrollTop: 28720 }, 'slow')

subscribe

I added the scrollTo paradigm

test subscribe

the more systematic fix over at #34496: [meta] Add Flag module to allow users to subscribe/unsubscribe without posting a comment is likely to be fixed this week (probably tuesday/wednesday, depending on your timezone). I don't think this is worth putting in dreditor after that, as subscribe comments will dwindle very quickly, and it's not really worth having the potential false positives just for a few old issues.

Tentatively calling this a won't fix. If anyone disagrees, feel free to re-open.

I'm not sure what #34496: [meta] Add Flag module to allow users to subscribe/unsubscribe without posting a comment will do with issue having old 'subscribe' texts. How long will these bother us?

Hiding new comments having 'subscribe' would be a bad idea because we need to educate those users.

So agreed to a won't fix.

Hrm... As sun pointed out in another thread, this is mostly going to be annoying to more experienced queue users, who are working on big bugs that have been around for years, and have tons of comments on them. Those users are the ones most likely to use dreditor, too. Maybe we should still implement this. We can always remove it later, when this becomes a non-issue.

Setting to needs work, because I'm wondering if there's a way of turning the comment div into a fieldset, rather than putting it in a fieldset, thus making the expanded comment take up less space?

#1303644: comment validation to prevent '+1 subscribe' comments

I have this patch installed through my sandbox and it works like a charm. I put this to needs review as I don't expand the collapsed fieldset ... it contains subscribe text anyway right :-)

Updated issue summary.

Removed the direct install. By clemens.tolboom

The scrollTo is applied using "$('a#new').each" which would run if a new comment is present.

Besides '#new' the request url could also have e.g. #comment-5160156 in which case that comment would have to be scrolled to after collapsing the subscribe comments.
And when the request url contains no #something part and there is a new comment then the focus should remain on the top of the page

I think the key might be to use $(document.location.hash).each() and test if document.location.hash has a value.

But still: When I e.g. go to http://drupal.org/node/34496#comment-1335726 it first positions on #87 on the top of my window, and a moment later #87 drops 10cm lower.

@helmo: Nice catch and improvement.

I fixed my issue branch and attached the new patch.

I was trying to tell the 10cm drop has nothing to do with this patch ... but with the original it scrolls a tiny bit. So it has to do with the scrollTop()

Checking with
1. Install original dreditor http://drupalcode.org/project/dreditor.git/blob_plain/HEAD:/dreditor.use...
2. Navigate to http://drupal.org/node/34496#comment-1335726
3. Notice the tiny scrollbar jump
4. Install the issue-branch version http://drupalcode.org/sandbox/clemenstolboom/1125712.git/blob_plain/refs...
5. Navigate to http://drupal.org/node/34496#comment-1335726

I found the solution with using Drupal.dreditor.goto(document.location.hash); instead of my homebrew solution.

Looks good

Well so much for this...

I'm closing this as no-one is willing to merge this.

Sorry for leaving this alone for too long. :-/

But yeah, meanwhile, drupal.org enabled fasttoggle module, which allows to unpublish useless comments with a single click. So most of the +1/subscribe comments are actually getting unpublished over time, which hides them for all users (not only Dreditor users) and eliminates the need for hiding them via Dreditor.

That said, they're still visible for privileged users, so I've created #1570824: Collapse/hide unpublished comments

I would think that delete makes more sense than unpublish for these comments. Then they are also hidden for admins and aren't clogging up the DB. The issue queue doesn't allow threading anyway.

Unfortunately, the delete operation is not covered by fasttoggle (I guess that's by design), so that makes a huge difference in terms of (human) moderation/cleanup performance. Right now, I can quickly iterate over all comments in an issue and unpublish the useless with a single click on the go.

Additionally, I'd have to check the Follow flag integration code once more to be sure, but I believe a user is unsubscribed from an issue when his comment is deleted (if it's the only one, which is often typically the case for subscribe comments).

Fixed link for real :( by clemens.tolboom