Community
Participate
Working Groups
Created attachment 247369 [details] Screenshot Ubuntu 14.04 Mylyn Reviews 2.4.0.v20140902-1709 1. Open a review that contains comments with multi-line, for example https://git.eclipse.org/r/#/c/33720/ 2. Open the Review Explorer (view) to see the comments The comments that span over multiple lines can be difficult to distinguish from each other because there are no lines between them (see screenshot). I suggest that the lines could be enabled for this widget.
Patch: https://git.eclipse.org/r/#/c/33806/
Created attachment 247370 [details] Screenshot after patch
Marc-Andre, I assume you're runnning in 4.x? I only have one line visible in 3.8, I didn't even realize that a line could pan mulitple rows. I'll fire up Luna and check it out.
(In reply to Miles Parker from comment #3) > Marc-Andre, I assume you're runnning in 4.x? I only have one line visible in > 3.8, I didn't even realize that a line could pan mulitple rows. I'll fire up > Luna and check it out. I tested with 4.4.0 and 4.5.0.I20140916-0800 on GTK2. I think GTK3 doesn't draw lines from memory but that's not for Mylyn to worry. Are you testing on Linux? On Windows, I don't think it's possible to have tables with multiple lines using SWT (win32 limitation??).
I'm on the fence on this one, or leaning against it. ;) With a quick look most (but not all) of the table style views in Eclipse tools don't use visible lines. But I agree that there is an issue here. I think the real problem is that the behaviour of showing multiple lines is actually unexpected. It should be doing the same thing as other implementations. In which case we shouldn't be displaying the whole comment description at all (ReviewsLabelProvider L437, I *think*) but either the first line or (perhaps better) one where we've taken the line breaks out. Could you give that a try? Sam, any opinion on this?
IIRC, the behaviour on Windows is that only the first line is shown with no indication that there is more to the comment. We should have the same behaviour on all systems. I think I like the idea of showing the whole comment, but maybe there should be a max number of lines shown per comment (say, 5).
(In reply to Sam Davis from comment #6) > IIRC, the behaviour on Windows is that only the first line is shown with no > indication that there is more to the comment. It's possible that this behavior works "by accident" because SWT/win32 doesn't support multiple lines in table. > We should have the same > behaviour on all systems. I think I like the idea of showing the whole > comment, but maybe there should be a max number of lines shown per comment > (say, 5). Showing the whole comment on one line (line breaks removed) or over multiple lines in one cell? I do prefer having the whole comment. If we want it to be consistent on all systems then that means no multi line in one cell. So I can try removing the line breaks if that's the consensus.
I would prefer to keep the line breaks but I'm surprised that doesn't work on Windows. It's probably better to go for consistency and simplicity rather than trying to do different things on different platforms, so I guess I'm in favour of removing the line breaks. The whole comment can be viewed by hovering over the row so it's not so bad that it will be truncated, as long as you can tell that there is more.
(In reply to Sam Davis from comment #8) > I would prefer to keep the line breaks but I'm surprised that doesn't work > on Windows. Actually, after looking into this again, the limitation is that rows cannot vary in height: http://stackoverflow.com/questions/4082082/swt-table-with-variable-row-height-working-on-linux-but-not-mac-windows
I wonder what percentage of comments would require more than one line. Would it be worth it to set the row height to 3 lines?
(In reply to Sam Davis from comment #10) > I wonder what percentage of comments would require more than one line. Would > it be worth it to set the row height to 3 lines? I got the feeling that most comments are larger than one line, even three lines. Assuming we are settled for x lines, how does it help the user identify the beginning and end of a comment ? I am not quite sure we should try to align the look across platforms, SWT really makes sure to have a different look and feel per platform so it did not feel that strange to me to have a one liner on win32 and multiple lines on linux. What is really inconvenient for the user is not being able to distinguish the limits of a comment. Another possibility is to probably alternate a darker background shade on the rows, so the limits are visible to the user on both platforms.
(In reply to comment #11) > Another possibility is to probably alternate a darker background shade on the > rows, so the limits are visible to the user on both platforms. Yeah, I like that idea. Isn't that what you showed in your screenshot? But I think we have a different problem on Windows which is that you can't tell when there is more than 1 line to the comment. I would like to solve that in a way that doesn't require platform-dependent code. In other words, come up with a good design that works on all platforms.
(In reply to Sam Davis from comment #12) > (In reply to comment #11) > > Another possibility is to probably alternate a darker background shade on the > > rows, so the limits are visible to the user on both platforms. > > Yeah, I like that idea. Isn't that what you showed in your screenshot? > Yes, the patch from Marc-Andre is solving the actual need, nicely ! > But I think we have a different problem on Windows which is that you can't > tell when there is more than 1 line to the comment. I would like to solve > that in a way that doesn't require platform-dependent code. In other words, > come up with a good design that works on all platforms. True, this is definitely a different problem. I suggest we track this issue on a different bug and move forward with this improvement
It's a different problem but it's related to this in that if we solve that one by limiting table rows to one line, there won't be a need for this change. That's why I wanted to get some input on how to handle that before moving forward with this. Maybe we should experiment with setting the row height to a fixed small number of lines to see if it works, in combination with alternating the colours?
(In reply to Sam Davis from comment #14) > It's a different problem but it's related to this in that if we solve that > one by limiting table rows to one line, there won't be a need for this > change. I think even when limiting table rows to one line, I would like to have setLinesVisible(true). The Problems view, Error log, etc all have it and it works quite well. The multi-lines just amplify the problem.
That seems reasonable. I personally find the lines make the view a bit cluttered but I can see that they also provide value.
So, is the patch acceptable as it is?
Yes, I've merged the change. Thanks for the contribution, and sorry that this languished for so long. Please close this bug unless you think further changes are needed.
(In reply to Sam Davis from comment #18) > Thanks for the contribution, and sorry that this languished for so long. No worries, there was no rush :) Thank you for accepting the patch! I'll create a new bug to discuss the inconsistencies of showing multiple lines on Linux vs Windows.
