Adium

Ticket #344 (closed defect: fixed)

Opened 3 years ago

Last modified 2 months ago

Need visual notification of blocked users

Reported by: Ludge Assigned to: Kiel
Priority: normal Milestone: Adium X 1.0
Component: Core Adium Version: 0.82
Severity: normal Keywords: block display icon
Cc: Patch:
Pending:

Description

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.

Attachments

KGContactBlockedPlugin.diff (8.7 kB) - added by Kiel Gillard on 09/06/2005 02:04:53 AM.
The proposed plugin, version 1.0
cl-strikeout-test.html (1.4 kB) - added by boredzo on 09/06/2005 03:30:55 AM.
demonstration of strike-out in various colour combinations. I don't think it looks so bad.
potentialSolution.png (6.1 kB) - added by Kiel Gillard on 09/06/2005 03:58:17 AM.
Mockup of a potential solution to this ticket (envision this as a row on the contact list).
blockedContactsAlpha.diff (12.9 kB) - added by Kiel Gillard on 09/07/2005 10:34:27 AM.
Experimental patch... please let me know what you think :-)
KGContactBlockedPlugin2.diff (48.3 kB) - added by Kiel Gillard on 11/09/2005 10:14:52 PM.
Potential patch to close ticket. Please extensively review!
KielBlocking.patch (38.9 kB) - added by Kiel Gillard on 11/29/2005 01:12:47 AM.
cbarrett's tamed svk version of my patch…
KGBlockedContacts3.diff (46.3 kB) - added by Kiel Gillard on 11/29/2005 01:44:29 AM.
Untamed version of cbarrett's patch.

Change History

05/28/2005 10:12:28 AM changed by catfish_man

  • severity changed from major to normal.
  • milestone changed from Adium X 0.82 to Adium X 0.90.

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.

07/20/2005 05:48:43 PM changed by durin42

  • milestone changed from Adium X 0.90 (Old) to Adium X 0.90.

09/06/2005 02:03:48 AM changed by 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 :-)

09/06/2005 02:04:53 AM changed by Kiel Gillard

  • attachment KGContactBlockedPlugin.diff added.

The proposed plugin, version 1.0

09/06/2005 02:17:43 AM changed by 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").

09/06/2005 02:50:02 AM changed by 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 :-)

09/06/2005 03:14:37 AM changed by boredzo

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

09/06/2005 03:30:55 AM changed by boredzo

  • attachment cl-strikeout-test.html added.

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

09/06/2005 03:50:37 AM changed by 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@scu.edu.au, I'll send you a mockup of how this might look - I sent one to Evan today to get his feedback).

09/06/2005 03:58:17 AM changed by Kiel Gillard

  • attachment potentialSolution.png added.

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

09/06/2005 04:00:29 AM changed by 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.

09/06/2005 05:10:33 AM changed by 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.

09/06/2005 07:20:18 AM changed by 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@dreskin.net) (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.

09/06/2005 07:34:35 AM changed by 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@dreskin.net

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

09/06/2005 07:57:36 AM changed by 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.

09/07/2005 10:33:19 AM changed by 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.

09/07/2005 10:34:27 AM changed by Kiel Gillard

  • attachment blockedContactsAlpha.diff added.

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

09/10/2005 12:10:44 AM changed by 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".

11/04/2005 04:29:57 PM changed by catfish_man

  • field_haspatch set to 1.

11/06/2005 12:40:24 AM changed by cbarrett

  • field_haspatch deleted.

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.

11/06/2005 12:41:01 AM changed by cbarrett

  • field_haspatch set to 1.

why'd it delete the flag?

11/07/2005 01:01:23 AM changed by 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?

11/07/2005 07:50:03 AM changed by evands

  • owner changed from anybody to Kiel Gillard.
  • field_haspatch deleted.

11/07/2005 06:41:35 PM changed by Kiel Gillard

Cheers :-)

11/09/2005 10:14:52 PM changed by Kiel Gillard

  • attachment KGContactBlockedPlugin2.diff added.

Potential patch to close ticket. Please extensively review!

11/09/2005 10:55:41 PM changed by 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!

11/09/2005 11:04:08 PM changed by 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.

11/10/2005 03:18:51 AM changed by cbarrett

  • field_haspatch set to 1.

11/10/2005 03:19:16 AM changed by cbarrett

  • owner changed from Kiel Gillard to cbarrett.

11/10/2005 03:19:51 AM changed by cbarrett

  • status changed from new to assigned.
  • field_haspatch deleted.

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

11/10/2005 03:20:05 AM changed by cbarrett

  • field_haspatch set to 1.

damn has patch.

11/17/2005 08:55:40 AM changed by 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.

11/17/2005 10:26:59 AM changed by cbarrett

  • field_haspatch deleted.

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.

11/17/2005 10:34:48 AM changed by cbarrett

  • field_haspatch set to 1.

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 :\)

11/17/2005 07:17:08 PM changed by 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.

11/17/2005 08:37:48 PM changed by cbarrett

  • field_haspatch deleted.

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 :)

11/17/2005 09:18:30 PM changed by 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.

11/17/2005 09:39:05 PM changed by evands

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

11/17/2005 10:33:38 PM changed by 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).

11/18/2005 09:33:30 PM changed by 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

11/18/2005 10:16:45 PM changed by 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.

11/18/2005 10:54:21 PM changed by cbarrett

Oh, duh. :) There too :D

11/29/2005 01:12:47 AM changed by Kiel Gillard

  • attachment KielBlocking.patch added.

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

11/29/2005 01:25:13 AM changed by 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 :-)

11/29/2005 01:44:29 AM changed by Kiel Gillard

  • attachment KGBlockedContacts3.diff added.

Untamed version of cbarrett's patch.

12/06/2005 02:32:27 PM changed by evands

What's the status of this patch, Colin?

12/08/2005 07:02:25 PM changed by 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).

12/12/2005 07:58:47 AM changed by sebhelyesfarku@freemail.hu

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

12/12/2005 07:59:58 AM changed by boredzo

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

I like it.

12/12/2005 09:00:24 PM changed by cbarrett

  • owner changed from cbarrett to durin42.
  • status changed from assigned to new.

this has been passed off to Augie.

12/29/2005 07:54:09 PM changed by durin42

  • status changed from new to closed.
  • resolution set to invalid.

Killing this as per IRC discussion.

12/29/2005 11:03:24 PM changed by 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.

12/29/2005 11:08:10 PM changed by tick

You will when we release 1.0. This stays closed.

01/02/2006 06:43:11 PM changed by catfish_man

  • milestone deleted.

02/12/2006 01:28:35 PM changed by evands

  • status changed from closed to reopened.
  • resolution deleted.

02/12/2006 01:29:40 PM changed by evands

  • owner changed from durin42 to Kiel.
  • status changed from reopened to new.
  • milestone set to Adium X 1.0.

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. :)

02/12/2006 01:33:18 PM changed by evands

  • status changed from new to closed.
  • resolution set to fixed.

(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.