Need visual notification of blocked users

[Ludge]

Now that the blocking works OK, it would be good if we could have some way of seeing which buddies are blocked within the contact list. Currently if you want to see if a user is blocked, you have to right click on their name and see if the menu says "unblock" or not. I think a new status icon would probably be best, something like the normal icon but with a crossed circle on it (like this character: Ø). Also, having their display name as a different colour or strikethrough text could work.

[catfish_man]

I like the idea, but I can't see it happening for 0.82. Pushing back to 0.9 for now until we decide what to do with it. For future reference, please let us set the severity and milestone.

[Kiel Gillard]

I've toyed around with having tooltips display whether a contact is blocked or not. This is probably not enough to address the needs of this ticket, but it's a start - until a better solution reveals itself.

I've attached a .diff file which contains a plugin that I wrote to add a "Blocked: Yes" to the body of a tooltip of blocked contacts. It should be noted that this .diff file adds the KGContactBlockedPlugin to the CoreComponents.plist file.

Please let me know what you think :-)

[Kiel Gillard]

The proposed plugin, version 1.0

[boredzo]

haven't tried the component, but what I suggest is displaying the contact in strike-out type, and taking out the ': Yes' if possible (leaving just "Blocked").

[Kiel Gillard]

I personally think that striking out type would look bad... and make it difficult to read the contact's screen name and/or UID.

I'm all for making the tooltip say "Blocked" instead of "Blocked: Yes", but I was attempting to keep consistent with similar features of Adium - for example, if a contact is Away, the tooltip says "Away: Yes" (this may be only for the Messenger protocol, but this is how it appears). Also, the InterfaceController requires a valid entry to each label - I guess I could get away with @" " but that would leave the tooltip saying "Blocked: ". Maybe we should see what others say...

Thanks for your feedback, looking forward to some more :-)

[boredzo]

maybe make the InterfaceController omit the ': ' when the value is an empty string. the same could be then done with 'Away'.

[boredzo]

demonstration of strike-out in various colour combinations. I don't think it looks so bad.

[Kiel Gillard]

The InterfaceController could omit the colon when the label's entry is an empty string, but it might be better to have Adium say "Status: Away" instead. While I'm at it (and this is probably something for another ticket), what do you think of changing the tooltips altogether, so it looks more like:

<image> <screen name> (Blocked) (if applicable)
-seperator-
Service: <service>
Status: <status> (if not available)
User ID: <formattedUID> (if not different to screen name)

As for the strike out test, I personally think it looks ugly :-P I mean, the black strike thru with a dark background colour looks crap. I'd favour a little block icon for a blocked contact on the contact list (if you email me at kgilla10@…, I'll send you a mockup of how this might look - I sent one to Evan today to get his feedback).

[Kiel Gillard]

Mockup of a potential solution to this ticket (envision this as a row on the contact list).

[Kiel Gillard]

Eh, I attached the mockup anyway, just to get some feedback from others who stumble upon this ticket. It looks bad because this is a screen grab of Preview sitting behind my shadowless Adium contact list, so it isn't aligned properly and things... but hopefully you get the idea.

[catfish_man]

Neat idea..., though I rather like the reporter's idea of having it be a status icon (perhaps because the status gems are the only non-contact-image icons on my buddy list :) ). I tried to implement strikethrough a while back, in a kinda halfhearted way. I forget what ended up stopping me.

[evands]

I like Status: Away like better than Away: Yes.

I'm not sure about adding (Blocked) after the name... it can already get really long: This Is My Long Display Name (evan.s@…) (MSN) (Blocked)' would be crazy.

Blocked: Yes makes some degree of sense. I suggest the following changes to the patch (which I think is appropriate even with the status icon thing implemented, as not all lists use status icons and not all status icon packs have a blocked icon):

• Make it check first for whether the object is an AIListContact
• If yes, check that contact's account. If no, just break.

However, how to handle an AIMetaContact also needs to be resolved.

[Kiel Gillard]

Catfish_man, I feel that having a status icon for a blocked buddy is problematic because then a user would not be able to tell what the blocked contact's real status is. To work around this, you have a blocked variant of each status but that means twice the work for the people that create status packs. If I was to implement a little block icon, I would design the solution to use the block icon of a status pack and enable the user to choose if it is shown or not, so you don't need to worry about it taking up space on your contact list :-)

Evan, I agree with your statement that the title of the tooltips could get really long, but I suggested that the tooltips be re-designed to the following format:

<image> <screen name> (Blocked) (if applicable)
-seperator-
Service: <service>
Status: <status> (if not available)
User ID: <formattedUID> (if not different to screen name)

So, given your example, the tooltip would look like:
<image> This is my really long display name (Blocked)
-seperator-
Service: MSN
Status: Away
User ID: evan.s@…

I will take your suggestions to heart and look into these issues :-)

[evands]

(In [13366]) Contacts who are away without an away message now show Status: Away rather than Away: Yes in their tooltips. Refs #344 as it results from discussion there.

[Kiel Gillard]

Ok, this one took a good study break :-)

Attached is a patch which incorporates a few interesting and serious changes:
1) AIListContact now responds to the methods -(BOOL)isBlocked and -(void)setIsBlocked:(BOOL)yesOrNo, using the statusObjectForKey: methods.
2) CBGaimAccount methods that add and remove contacts from privacy lists now instruct the AIListContact instances to store whether or not they're blocked. According to durin42, I think I've used the wrong methods...
3) ESMetaContentsContactPlugin (I think that it's name from the top of my head) has been changed to append @" (Blocked)" if one or more (but not all) of the Meta's contacts are blocked.
4) The KGContactBlockedPlugin creates a secondary tooltip entry which says "Blocked: Yes" (for the time being) of ListContacts or Metas whose contacts are all blocked.

Please be aware that this plugin is added to the CoreComponents plist.

Patch attached to get feedback from the devs.

[Kiel Gillard]

Experimental patch... please let me know what you think :-)

[Kiel Gillard]

I noticed a contact on my contact list had the secondary tooltip entry "Idle: Yes". Perhaps this should be changed to "Status: Idle" as well? Or perhaps for contacts who are both away and idle (is that even at all possible?) "Status: Away, Idle".

[cbarrett]

Kiel: If you want to tackle the Status: Idle change, that'd be neat. It's probably a fairly simple change, basically just looking to see how the Away plugin does things, and porting that over to the Idle plugin.

If we had a "Good for patchwriters" flag, I'd hit set it.

[cbarrett]

why'd it delete the flag?

[Kiel Gillard]

I think I can tackle the "Status: Idle" change, but I need to know one thing: can a contact on any service be Away AND Idle? How about metas? Or should I code the solution to take into consideration the possibility that a contact/meta can be both Away and Idle just for the principle's sake?

I'll give this a bit more thought, my instincts tell me there's a better solution for this...

Aside: Currently I'm working on this ticket and (shortly) #1117. Is there any way I can assign myself to these tickets so nobody else does the work I do?

[Kiel Gillard]

Cheers :-)

[Kiel Gillard]

Potential patch to close ticket. Please extensively review!

[Kiel Gillard]

Ok, it's been a while in the making (no thanks to uni life) but here it is - a massive patch.

I need the devs to pay particular attention to the small changes I've made to CBGaimAccount (the privacy methods). Augie suggested these were the wrong places to put them and I can't remember why I disagreed with him. I think the other methods weren't called at startup.

Included in this patch is 2 plug ins for organising tooltips so they're not insanely long (as they could get).

Everything else should be fairly self explanatory - pretty sure I met the coding guidelines but if I didn't then tell me to try again!

[Kiel Gillard]

I forgot to mention, if this patch is committed, status packs need to start using a Blocked icon. Which means the default status packs need to be updated. Included in this patch is an updated Gems status icons pack with a blocked icon from Catfish_Man. People can use the Gems pack as an example to upgrade their packs. No difficult change, just another key and value to implement.

Also, localised variants of the ListLayout nibs need to be updated to sport a "Show Blocked Icons" check box and complementary popup menu. This checkbox and menu needs to be connected to the File's Owner like all the other GUI widgets.

[cbarrett]

assiging this to me (since I'm reviewing the patch), and accepting it.

[cbarrett]

damn has patch.

[Kiel Gillard]

The patch needs a minor update (have recently discovered the wonders of svn st). Once svn is fixed, the best place to call setIsBlocked: on any subclass of AIListContact has been determined and I've finished ticket #1117 (which I hope will be soon), I'll attach an updated patch. Therefore, an updated patch will also include code to close ticket #1117.

cbarrett, can you please contact me with any other issues you may have with my patch for this ticket which could correct in an update. I welcome feedback from other people, too. My email address is on #1117.

[cbarrett]

My only complaints about this are that I believe you are duplicating some code in the blocking plugin with this new method. Take a look at the functions for blocking and unblocking there, see if those are of any use.

[cbarrett]

argh, stupid has patch. Evan, could you reverse apply that diff? The template for the ticket page is expecting different input, and it's not coming (I haven't had time to set up a trac testbed. 24 hours in the day :\)

[Kiel Gillard]

Where in the patch attached to this ticket do I enumerate through accounts of a particular service, check to see if the current account conforms to a privacy protocol and if it does add or remove the contact of that account to/from the privacy list? *scratches head*

The only major problem with this patch is the fact that I (and nobody else, apparently) knows where the proper place is to send an AIListContact instance the setIsBlocked: message.

Minor problems to the patch include the fact that I hadn't reverted files that I had changed to compile a revision I had download from the svn repository, as well as a dodgy bit of code that I swear I had removed.

[cbarrett]

My initial reaction would be that you'd just want to check the privacy lists, rather than bothering with a new method to AIListContact. But since you've got it there, look in the blocking plugin. Wherever the same actions as setIsBlocked are performed (i.e. adding to the privacy lists), then replace it with setIsBlocked. I would still probably advise you to just smoke the privacy lists straight at this point. A better privacy API is coming from Gaim soon.

Also, why are there extra files added, like the "service: " tooltip? Do these relate to the ticket in some way I've missed? Note that I'm not being hostile and saying IT SUX. On the contrary, I think your code for the actual drawing and adding the icon was well hacked. You just got a little overzealous with abstraction :)

[Kiel Gillard]

The whole reason there's an isBlocked status object is because IMHO it is a much more efficient way of determining if a contact is blocked, rather than calling a method that loops through all the accounts of a service kind for an account to look for a UID. This could add up to a sizeable amount of processing time if you're wondering if every contact on your contact list (potentially 100+) is blocked.

However, your idea of AIListContacts blocking themselves (I think that's what you implied) is a good one. I'll look into that.

The "Service: " tooltip is relevant to this ticket because if you have a look at the discussion about half way through this page (in particular a comment from Evan), the title of tooltips could get crazy long, particularly on MSN. It's fine if you think it sucks, personally I thought the way it is in 0.86 sucks too lol. Having them in a secondary position made much more sense. Something had to be done to address the issue. Same applies with the UID.

I'll wait until Evan has updated libgaim to 2.0.0 until I think about the better privacy API in Gaim.

[evands]

svn now has libgaim 2.0.0, such as it is -- I don't think the privacy API there changed :\

[catfish_man]

I just wanted to say that I highly approve of having list contacts know whether they're blocked (even if it's internally just iterating over the privacy list, I like the API).

[cbarrett]

I agree. I was mostly just complaining that this introduces an inconsistency. The blocking plugin does not use this method, and other areas do. The correct time to call setIsBlocked: is in the blocking plugin, when they select the blocking menu.

I didn't do this whole thing at first because I wanted to get rudimentary blocking support working and done. First make it work, etc...

The privacy API is something that's slated for further down the line, but it is coming. It may or may not be in 2.0

[evands]

"The correct time to call setIsBlocked: is in the blocking plugin, when they select the blocking menu."

I would think it'd be in account code when we find out that the contact is blocked.

[cbarrett]

Oh, duh. :) There too :D

[Kiel Gillard]

cbarrett's tamed svk version of my patch...

[Kiel Gillard]

Ok, I've attached cbarrett's 'tamed' version of my patch. Why did it need to be tamed? Because I'd originally made some changes that are both controversial and slightly beyond the scope of my patch (that is, re-arranging information on tooltips). Tooltips are not going to provide a visual notification of blocked contacts until we reach some sort of agreement about the layout of tooltips. I will send an email to the list shortly detailing the problem and encouraging discussion about the whole issue.

Please let me know what you think about the patch :-)

[Kiel Gillard]

Untamed version of cbarrett's patch.

[evands]

What's the status of this patch, Colin?

[Kiel Gillard]

Augie and I are working on it. I've been away for a couple of days but I'll try and wrap this up soon (Augie doesn't seem to be online much, when I'm online at least).

[sebhelyesfarku@…]

What about having a Show/Hide Blocked Contacts item in the View menu? Good idea, isn't it?

[boredzo]

hmmm. '(Show|Hide) Blocked Contacts'.

I like it.

[cbarrett]

this has been passed off to Augie.

[durin42]

Killing this as per IRC discussion.

[anonymous]

"Killing this as per IRC discussion."

Occasional visitors like me have absolutely no idea of what the IRC discussion was and why this ticket was flagged invalid.

[tick]

You will when we release 1.0. This stays closed.

[evands]

The end discussion of this patch makes no sense to me, so I'm ignoring it. The original purpose of this ticket was "need visual notification of blocked users," and that's a valid enhancement request. Furthermore, it's one which Kiel has no fulfilled. :)

[evands]

(In [15210]) Patch from Kiel Gillard which implements a "blocked" status icon. There is a default blocked icon which will be used if the status icon set does not supply one, which it can do with a "Blocked" key. Fixes #344. Good work! :)

The patch also improves the the blocking API a bit, allowing contacts to block themselves.