GNOME Bugzilla – Bug 687809
Misaligned time stamps in conversation history
Last modified: 2013-06-24 15:57:33 UTC
Created attachment 228315 [details] screenshot Bug 680989 cleaned up the timestamps that are shown in the conversation history. However, the timestamps still aren't aligned correctly. See the screenshot I took earlier. The timestamps are * not on the same line as the message that they refer to * in the middle of the bubble - they should be right-aligned
Created attachment 228453 [details] [review] TelepathyClient: only test purposes. Make each clutter text in chat notifications a BoxLayout to allow a timestamp beside.
There's two problems I need help, sorry for that. The boxlayout don't fit the size of the text if the boxlayout is set vertical=false, in consequence, line wrapping seems not working. If I put boxlayout in vertical = true it's works fine, but I need the boxlayout on horizontal to put a timestamp beside. I tried with all properties of the actor(expand, fill, etc) but not luck. In the other hand, I don't know which is the best solution to put the timestamp here. The problem is that if the line has wrappinp I can't divide the line into single lines of the chat, so the boxlayout grows and the timestamp takes all the right side of all wrapped lines. So it is ugly. Thanks in advance to anyone can help a little.
Created attachment 228458 [details] line wrapping not working
Created attachment 228459 [details] Manually set a bigger size to box layout There's you can see that the timestamp takes all the rigth side. Also, I had to manually set a bigger size to box layout to see line wrapping is working fine. So boxlayout is not fitting correctly the size of the text.
We've known about this issue, but found it hard to reproduce in a fixed setting. If you can create a standalone test that exhibits this behavior, like the ones in tests/interactive/, I'll take a look.
Ok Jasper, thanks for comment, I was like desperate with this behaviour of the boxlayout, I tought I was doing something wrong. Also, do you have some idea how can I divide wrapped lines without doing a bad and ugly hack? or some better idea how to add a timestamp at the end of a wrapped line? Thanks
Also Jasper, I will see how to make a test for the boxlayout.
Give us a test with the StBoxLayout, and we'll try to fix it. Bonus points if you can make one solely using Clutter and ClutterBoxLayout.
Created attachment 228699 [details] tests/interactive/box-layout.js: Add some functionality to allow to test line wrapping inside a box layout.
Jasper, I tried hard to make the test in solely clutter, but I was not able to make line wrapping works well in clutter in any manner. Sorry. I hope the patch helps you to debug.
The timestamps should ideally contain references to the day the message was sent (if it wasn't today): eg. Yesterday, 10:04 PM So if we can't get wrapping to work well, we might have to use a separate line for the time stamp and just make sure that they aren't shown too often.
Allan, the current behaviour contain references to the day the message was sent as I did in this patch: https://bugzilla.gnome.org/show_bug.cgi?id=680989 Is it not working?
(In reply to comment #12) > Is it not working? Looking at a chat that happened last week, I do indeed see the day in the time stamp. Maybe I got this wrong... not sure.
Allan, With the new patch in https://bugzilla.gnome.org/show_bug.cgi?id=680989 I think it has to works fine... if not, tell me something in the other bug report. Allan, Jasper Jasper, if you have not a lot of time to debug the wrap thing we can align to the right and make a new line for the timestamp like we had some time ago. What do you think Allan, Jasper?
(In reply to comment #14) > Allan, > > With the new patch in https://bugzilla.gnome.org/show_bug.cgi?id=680989 I think > it has to works fine... if not, tell me something in the other bug report. OK, let's go for that. > Allan, Jasper > Jasper, if you have not a lot of time to debug the wrap thing we can align to > the right and make a new line for the timestamp like we had some time ago. What > do you think Allan, Jasper? That certainly sounds like it would be an improvement.
Created attachment 230826 [details] rigth aligment Allan, This is the timestamp align to the right. Do you like it? I think it is not enough visible now we cleaned up the timestamps and it is not enough padding between the corner of the bubble and the timestamp.
Created attachment 230831 [details] half padding between timestamp and normal chat (In reply to comment #11) > So if we can't get wrapping to work well, we might have to use a separate line > for the time stamp and just make sure that they aren't shown too often. Ok, maybe show timestamp each 4 minutes instead of 1 like now? Also we can reduce some paddings between different "group chat types" (like the padding between my last sentence and the timestamp and the next sentence) to make more space for real chat sentences. See attached picture to see half of the padding, 4px instead of 8px we have now. I think it is enough for the worst case I made in the picture. All of this thinking about if I can't find a solution to wrapping and timestamp beside chat sentences.
Sorry for the slow response, Carlos. (In reply to comment #17) > > So if we can't get wrapping to work well, we might have to use a separate line > > for the time stamp and just make sure that they aren't shown too often. > Ok, maybe show timestamp each 4 minutes instead of 1 like now? Right, the timing is important here. Since the space is constrained, the goal is to indicate the time when it is significant but not the rest of the time. The time is interesting to see if you have not responded for a while, and want to know when the last message was. It is also handy when looking at a log to get a sense of how sporadic a conversation was. In my mind, this roughly translates to length of time between messages. In an ideal world, we would show a time stamp to indicate a break in the conversation (maybe breaks of over 3 or 4 minutes?) If that's far too difficult, showing the time stamp every 4 or 5 minutes makes sense. > Also we can reduce some paddings between different "group chat types" (like the > padding between my last sentence and the timestamp and the next sentence) to > make more space for real chat sentences. > See attached picture to see half of the padding, 4px instead of 8px we have > now. I think it is enough for the worst case I made in the picture. I'd need to try it, but that sounds good. :)
Hi Carlos, this looks good to me. The only thing that seems slightly amiss is that the timestamps look slightly too far to the right to me, but I suspect that is a separate bug to do with the frame containing the conversation as a whole.
Allan, comment #19 The timestamp or all the text? If it is all the text yes(in fact, I commented it in the comment#16), it is a separate bug and probably I will look into it if do you want =) Also, can you confirm the final configuration? Time between timestamps: 3 min (I tested it with 4 or 5, but it's too long, because when someone doesn't answer, you think: she/he is not answering, what time has passed? And I tested it(with me) and it is 3 min). Also google talk have 1 min timestamp, so we are sure improving with 3 min. But 5 min for me it's also good. Also, in your comment #18 you told "in an ideal world..." currently, the timestamp works as expected, showing it when 1 min passed and nobody has talked. So it is in fact already an ideal world =) Padding between chat groups(separation between system prints, other person prints, and self prints) equal, not double as currently we have. This is showed at comment #17 Timestampt in other line, not in the current line In fact we have three options: 1- Large(as needed) timestampt in new line. Google talk behaviour(but improved, google talkt always show the entire timestamp). 2-Only hour:min of the timestamp and put it in the same line as the conversation, ocupiying a vertical space for it. Whatsapp behaviour. 3-Large(as needed) timestamp and in the same line sa conversation, ocupying a lot of vertical space(IMHO not good for me).
Also we have to solve the problem that the timestamp at the rigth is barely visible as I commented on comment #16. Maybe when we solve the "too much rigth text" it will be solved?
(In reply to comment #20) > Time between timestamps: 3 min (I tested it with 4 or 5, but it's too long, > because when someone doesn't answer, you think: she/he is not answering, what > time has passed? And I tested it(with me) and it is 3 min). Also google talk > have 1 min timestamp, so we are sure improving with 3 min. But 5 min for me > it's also good. Also, in your comment #18 you told "in an ideal world..." > currently, the timestamp works as expected, showing it when 1 min passed and > nobody has talked. So it is in fact already an ideal world =) 3 minutes seems good then! > Timestampt in other line, not in the current line > In fact we have three options: > 1- Large(as needed) timestampt in new line. Google talk behaviour(but improved, > google talkt always show the entire timestamp). > 2-Only hour:min of the timestamp and put it in the same line as the > conversation, ocupiying a vertical space for it. Whatsapp behaviour. > 3-Large(as needed) timestamp and in the same line sa conversation, ocupying a > lot of vertical space(IMHO not good for me). As discussed on IRC: it would make sense to have short time stamps on the same line and longer ones on their own line.
After talked to Jasper, I tried to solve 662795 to be able to fix this bug, but I need more clutter knowledge to understand the clutter core to solve this. Also I tried to use a raw Clutter.BoxLayout to work around that bug, but it was very difficult to add CSS without ST library. So I'll wait until 662795 is fixed to solve this bug. The problem is St.BoxLayout doesn't implement https://git.gnome.org/browse/clutter/commit/?id=d037890fc4a4d488a521af666ddcb3945fe64aff clutter behaviour. So we can't allocate a boxlayout with vertical:false. Thanks to Jasper to point out the commit at clutter.
Created attachment 242661 [details] [review] TelepathyClient: timestamps aligned to rigth. Reverting the changes in aligment of timestamps at 680989 https://bugzilla.gnome.org/show_bug.cgi?id=687809
As a workaround, timestamps aligned to rigth like we have before.
Created attachment 242664 [details] [review] TelepathyClient: Timestamp timeout to 3 minutes. It is intended to make sure we don't show a timestamp too much times in the same conversation, since it will decrease the context of the conversation in the chat bubble. https://bugzilla.gnome.org/show_bug.
As we talk Allan, timestamp now is 3 minutes
I've just tested the patches and the behaviour seems good.
Review of attachment 242661 [details] [review]: The commit message here needs work. It should be split across multiple lines, and be in an "action sentence" form like: telepathyClient: Align timestamps to the right again Commit 3de0ebf changed timestamps to be center-aligned. New design updates change this, so revert it. http://bugzilla.gnome.org/show_bug.cgi?id=687809 ::: js/ui/components/telepathyClient.js @@ +969,3 @@ styles: ['chat-meta-message'], + childProps: { expand: true, x_fill: false, + x_align: St.Align.END }, The indentation here is wrong. It should align with the line above it.
Review of attachment 242664 [details] [review]: Same with this one. The commit message needs work: telepathyClient: Increase the timestamp timeout to 3 minutes The timestamp timeout specifies how long we should wait before adding a timestamp to the notification. A timeout of one minute ended up showing a lot of timestamps, so increase it to 3 minutes. https://bugzilla.gnome.org/show_bug.cgi?id=687809 Code looks fine.
Created attachment 247523 [details] [review] telepathyClient: Align timestamps to the right again Commit 3de0ebf changed timestamps to be center-aligned. New design updates change this, so revert it. http://bugzilla.gnome.org/show_bug.cgi?id=687809
Created attachment 247524 [details] [review] telepathyClient: Increase the timestamp timeout to 3 minutes The timestamp timeout specifies how long we should wait before adding a timestamp to the notification. A timeout of one minute ended up showing a lot of timestamps, so increase it to 3 minutes.
Thanks Jasper for the reviews, the commits were from some time ago, and I remember I had some troubles with eclipse commit and I didn't notice. Sorry for that!