Adium

Opened 12 years ago

Closed 9 years ago

Last modified 9 years ago

#9920 closed patch (fixed)

Timestamp on Growl notifications

Reported by: bigjoeystud Owned by:
Milestone: Adium 1.5 Component: Adium UI
Version: Severity: normal
Keywords: Cc:
Patch Status: Accepted

Description

Would it be possible to have time stamps on each of the growl notifications? I'd be in interested in knowing when people log in/off.

Attachments (3)

GrowlTimeStamp.png (18.1 KB) - added by Stuart Connolly 12 years ago.
Growl notifications with time stamps
GrowlAlertNib.tar.bz2 (3.8 KB) - added by Stuart Connolly 12 years ago.
GrowlAlert.nib with new option to show time stamp
GrowlWithTimeStamps.diff (6.7 KB) - added by Stuart Connolly 12 years ago.
Growl notifications with option of showing time stamps

Download all attachments as: .zip

Change History (29)

comment:1 Changed 12 years ago by Jordan

Milestone: Adium X 1.4

Sounds reasonable - and especially useful for those using sticky notifications.

comment:2 Changed 12 years ago by Stuart Connolly

I'm new to Adium development but I'm currently working on implementing this.

comment:3 Changed 12 years ago by Robert

Cool! :)

comment:4 Changed 12 years ago by Stuart Connolly

I've finished implementing this but I think I need some input on how the time stamp is to be displayed in the notification. The way that I have currently coded it, is to simply prefix the notification description with the time stamp surrounded by square brackets. For example:

<title>
[<time>] <description>

I can easily change it to be included in the title rather than the description but I really need some feedback on this. I'll attach a screenshot of some actual notifications with time stamps.

Changed 12 years ago by Stuart Connolly

Attachment: GrowlTimeStamp.png added

Growl notifications with time stamps

comment:5 Changed 12 years ago by Peter Hosey

That looks fine to me. Hopefully, you're using the user's short time format (see NSDateFormatter).

comment:6 Changed 12 years ago by Stuart Connolly

I wasn't but I am now. I've also added this as a new option when adding a Growl notification to a particular event, so along with having the option to make the notification 'Sticky' there is now also the option to 'Show time stamp'. I figured users may not want the time stamp on every different type of notification. Also, how do I got about getting the interface strings localized?

comment:7 Changed 12 years ago by David Munch

Nice work stuartc! Looks fine to me, but would it be possible to put the time in the top-right corner instead? I believe this lower the focus that time has in your screenshot.

comment:8 Changed 12 years ago by Stuart Connolly

David, unfortunately this is not possible as the Growl API currently does not allow the option to place specific text, it only allows two different lines, the notification title and description.

Changed 12 years ago by Stuart Connolly

Attachment: GrowlAlertNib.tar.bz2 added

GrowlAlert.nib with new option to show time stamp

comment:9 Changed 12 years ago by Zachary West

Patch Status: NoneNeeds Dev Review

One comment I have from just reading it over quickly: remember the case of RTL languages; [NSString stringWithFormat:AILocalizedString(@"[%@] %@", "A growl notification with a timestamp. The first %@ is the timestamp, the second is the description text"), timestamp, description); might be a better idea.

comment:10 Changed 12 years ago by Stuart Connolly

You're absolutely right, I completely overlooked this requirement. I'll will update the patch with your suggestion included. Thanks.

Changed 12 years ago by Stuart Connolly

Attachment: GrowlWithTimeStamps.diff added

Growl notifications with option of showing time stamps

comment:11 Changed 12 years ago by Zachary West

Actually the above is mostly completely wrong. I, also, overlooked the fact that description is the message text when not including the subject. Just because a user's localization is RTL doesn't mean a message is. I'm not sure Growl even handles RTL text (at least, our implementation; we're passing around NSStrings, which lack the NSParagraphStyleAttributeName.

comment:12 Changed 12 years ago by Zachary West

The above being my AILocalizedString() 'hack', not the patch.

comment:13 Changed 12 years ago by Stuart Connolly

I see your point. The AILocalizedString() 'hack', simply changes the ordering of the time stamp and description. As you said this also does not taking into account whether or not the received message is in a RTL localization. Even if Growl did support RTL text then this would still be an issue, but only for messages sent from a contact and possibly for custom statuses. Am I right in thinking that all other notification text is effectively generated by Adium.

comment:14 Changed 11 years ago by Zachary West

Milestone: Adium 1.4Adium 1.3.x

comment:15 Changed 11 years ago by Robert

Type: enhancementpatch

comment:16 Changed 10 years ago by Robert

Ticket #13546 has been marked as a duplicate of this ticket.

comment:17 Changed 10 years ago by assetburned

isn't this ticket a similar? #7923

comment:18 Changed 10 years ago by Robert

I would disagree as I did in #8771. However, this seems like an aspect of #8771. Seen as this ticket contains a patch I'd keep it though.

comment:19 Changed 10 years ago by Robert

Milestone: Adium 1.4.xAdium 1.4.1

comment:20 Changed 10 years ago by Robert

Milestone: Adium 1.4.1Adium 1.5

comment:21 Changed 9 years ago by Robert

Owner: nobody deleted

comment:22 Changed 9 years ago by Adrian Godoroja <robotive@…>

(In f34e6e092b22) Added timestamp on Growl notifications. r=robotive. Refs #9920.

comment:23 Changed 9 years ago by Adrian Godoroja

Patch Status: Needs Dev ReviewAccepted
Resolution: fixed
Status: newclosed

comment:24 Changed 9 years ago by Adrian Godoroja

Manually applied, nib redone. Thanks.

comment:25 Changed 9 years ago by Robert

stuartc, thanks for your patch. Sorry it took so long to apply it.

Last edited 9 years ago by Robert (previous) (diff)

comment:26 Changed 9 years ago by Robert Vehse

(In 2679e525bcd8) Credit Stuart Connolly for his patch. Refs #9920.

Again, some minor changelog improvements.

Note: See TracTickets for help on using tickets.