GNOME Bugzilla – Bug 680989
Conversations - time stamps are noisy, misaligned with message
Last modified: 2012-12-03 14:15:06 UTC
I can currently seeing the following string in a conversation bubble: "Sent at 14:26:29 on Wednesday". The string contains unecassary information. The message was sent today, so I don't need to see the day. I also don't need to know which second I received it at. "Sent at" is also noise. In short, it should read "14:26" [1]. It should also be vertically aligned with the most recent message. [1] If the message was received on a different day, you can add it to the string. It would be great if it could read "Yesterday" if the message was received, well, yesterday.
Created attachment 225427 [details] [review] Change format of timestamps of conversations bubbles. Now we only show the hour if the message is on today, and the word yesterday if it was on yesterday. Also don't show the seconds in the timestamps. Now the timestamp is align to the middle of the conve
I only test it with today, because I don't know how test it for messages from one year before. But I look carefully and the code seems correct.
Review of attachment 225427 [details] [review]: In no way will this ever translate.
Created attachment 225441 [details] [review] Change the timestamp code to call the function of translation to allow the translation of the timestamp. Also, erase the line that align the timestamp in the notification to middle, since the default value is MIDDLE and that's the align we want.
Review of attachment 225441 [details] [review]: Please squash with the previous patch. Also, please limit commit message lines to 79 characters. We follow the format described in http://lists.cairographics.org/archives/cairo/2008-September/015092.html
Created attachment 225636 [details] [review] Change string formats of the timestamps from conversation bubles, since before the timestamps contain unnecessary information. Also change the timestamp align to the middle for design reasons.
Review of attachment 225636 [details] [review]: Your commit message is too long. Try something like: messageTray: Change timestamp string formats The timestamps before contained unnecessary information. Additionally, align the timestamps to be in the middle of the bubble for design reasons. ::: js/ui/components/telepathyClient.js @@ +938,3 @@ var daysAgo = (now.getTime() - date.getTime()) / (24 * 60 * 60 * 1000); + let timestamp; Why the change? @@ +942,3 @@ + // Show only the hour if date is on today + if(daysAgo < 1){ + timestamp = "<b>%H:%M</b>"; I'm not sure if we need to translate this. Probably not. @@ +968,3 @@ } + return now.toLocaleFormat(timestamp); "now" is incorrect. This needs to be "date".
(In reply to comment #7) > Review of attachment 225636 [details] [review]: > > Your commit message is too long. Try something like: > > messageTray: Change timestamp string formats > > The timestamps before contained unnecessary information. > Additionally, align the timestamps to be in the middle of the > bubble for design reasons. > Ok > ::: js/ui/components/telepathyClient.js > @@ +938,3 @@ > var daysAgo = (now.getTime() - date.getTime()) / (24 * 60 * 60 * > 1000); > > + let timestamp; > > Why the change? > For me was more clear, but no big reason, so I revert the changes > @@ +942,3 @@ > + // Show only the hour if date is on today > + if(daysAgo < 1){ > + timestamp = "<b>%H:%M</b>"; > > I'm not sure if we need to translate this. Probably not. > I didn't change it. If you want to change, please tell me. > @@ +968,3 @@ > } > > + return now.toLocaleFormat(timestamp); > > "now" is incorrect. This needs to be "date". Sorry for the silly error.
Created attachment 225867 [details] [review] messageTray: Change timestamp string formats The timestamps before contained unnecessary information. Additionally, align the timestamps to be in the middle of the bubble for design reasons.
Review of attachment 225867 [details] [review]: Looks good, but you're going to have to wait until String Freeze goes away for this one to work.
(In reply to comment #10) > Review of attachment 225867 [details] [review]: > > Looks good, but you're going to have to wait until String Freeze goes away for > this one to work. Do we want this one in 3.6.1? Florian, do you want to file a freeze break request?
(In reply to comment #11) > (In reply to comment #10) > > Review of attachment 225867 [details] [review] [details]: > > > > Looks good, but you're going to have to wait until String Freeze goes away for > > this one to work. > > Do we want this one in 3.6.1? Florian, do you want to file a freeze break > request? Do you have a screenshot of the patch in action? I don't have a working JHBuild right now, unfortunately.
This can be landed now.
AS far as I know I have to push to upstream now...can I do this?
Do you have commit rights?
I don't think so. It's my first patch to be commited.
Comment on attachment 225867 [details] [review] messageTray: Change timestamp string formats Attachment 225867 [details] pushed as 3de0ebf - messageTray: Change timestamp string formats Here we go then - are the other patches obsolete and the bug can be closed? Or is there anything else left?
Florian, the other patches were my own errors, so are obsolete. Close bug as fixed
Yeah, the other patches are previous attempts. There's nothing left here.
OK, closing then - thanks for the patch!
Is it possible to have a screenshot of the fix in action?
I happened to notice this commit while browsing through Shell git. The "yesterday" strings say: + format = _("<b>%H:%M</b> on Yesterday"); which seems strange to me as a native en_GB speaker: I would expect it to say "<b>%H:%M</b> yesterday". I'm not aware of this being different in en_US?
Created attachment 227377 [details] [review] MessageTray: change timestamp wrong text "on Yesterday" to "yesterday"
Simon, Yeah, it's my English fault. Sorry for that and thanks to take notice. I assume that you're English is correct for the last patch.
Created attachment 227379 [details] patch working Aday, That's what do you want?
Review of attachment 227377 [details] [review]: Makes sense.
Aday, can you tell me the good format? I saw that in this bug report https://bugzilla.gnome.org/show_bug.cgi?id=687809 you told a different format. Can you specify the correct format? AS far I understood: 1 day: Yesterday, 14:30 PM (mayus at yesterday, coma, hour, space, PM/AM) [2, 7] days: Monday, 14:30 PM 1 year: May 25 on Monday, 14:30 PM >1 year: 2012, May 25 on Monday, 14:30 PM Or backwards? 14:30 PM on Monday, May 25, 2012
(In reply to comment #27) > Aday, can you tell me the good format? > I saw that in this bug report https://bugzilla.gnome.org/show_bug.cgi?id=687809 > you told a different format. Can you specify the correct format? > AS far I understood: > > 1 day: > Yesterday, 14:30 PM (mayus at yesterday, coma, hour, space, PM/AM) > > [2, 7] days: > Monday, 14:30 PM > > 1 year: > May 25 on Monday, 14:30 PM > > >1 year: > 2012, May 25 on Monday, 14:30 PM > > Or backwards? > > 14:30 PM on Monday, May 25, 2012 In general I would put the time after the date (would be good to follow the localisation/format settings for that), and I wouldn't bother with "on" - it's extra text and might be tricky for translations. I would also only include the day of the week if the message was sent in the current week. So: 1 day: Yesterday, 14:30 PM Some time this week: Monday, 14:30 PM Further back than this week: May 25, 14:30 PM A year or more: May 25 2012, 14:30 PM There are other places where these nice time stamps have been implemented, so you might want to check there too. Nautilus springs to mind...
Created attachment 229074 [details] [review] messageTray: Clean-up timestamps for design reasons and correct some English issues in the timestamp string.
Created attachment 229075 [details] patch working
Aday, I follow your indications, nautilus do it in a different manner, using the entire date, and I think your manner is more logical and compact. Jasper, The previous patches not commited are obsoletes with this new patch. If the code is correct and no problems, please commit the new patch.
(In reply to comment #30) > Created an attachment (id=229075) [details] > patch working Looks good to me. The time stamp should be right aligned though...
Allan, Yes, but we are following this in the other bug report https://bugzilla.gnome.org/show_bug.cgi?id=687809 If do you want, I can fix the align in this patch.
(In reply to comment #33) > Yes, but we are following this in the other bug report > https://bugzilla.gnome.org/show_bug.cgi?id=687809 Ah sorry, my mistake. Then yes, I like this. :)
Reopening this. Let's try and land the patch in comment 29.
Pushed according to comment 31. Carlos, time to get a git account? :)
Giovanni, Yeah, that would be cool
(In reply to comment #36) > Carlos, time to get a git account? :) +1 :)
(In reply to comment #37) > Yeah, that would be cool Then ask for it[0] :-) [0] https://live.gnome.org/NewAccounts/
I will do it, thanks Florian for the link. Thanks Giovanni and Allan for the recommendation.