@iamphill I think you had a prototype MR a little while ago that changed the way system notes are displayed?
@stanhu System notes are part of how the issue developed, just as comments are. The moment at which a assignee or milestone change, or a push, or the check-off of a task took place is very important.
Events that happened on an issue or MR are just as integral to the essence of that resource as user comments are. I think we could change the way they're displayed, but we shouldn't hide them, or move them outside of the timeline.
Again I think the "activity" of an issue being inline with the "discussion" of an issue is important for context, and I wouldn't want these in a separate tab.
But that MR was very hacky so I was going to make it store the type of the system in the DB & then display an icon based on that, I just haven't gotten around to starting it yet.
@JobV@jschatz1 For me system notes usually as important as people comments. Hiding it is absolutely unacceptable. Making it more compact like taking less vertical space is OK
Just a suggestion: what about a "collapse all system notes" button (at the top or on every system note)? This way system notes can be toggled if still needed to be seen, while instantly making the conversation more readable for people who do not need to see them.
I think designing them slightly different than comments and making them more compact would help so there is a little more visual distinction. I'd be happy to work on this for an upcoming milestone! Feel free to assign to me if so.
I've always wanted to see an icon in system updates because rows look empty without it. It could be a custom icon for each type of update but a GitLab or a recognizable System Update user icon would be fine, too.
Just to add my 2 cents: I like the "empty" rows without any icons. It allows me to skim through an issue or MR and find the "real" comments faster. If there was an user avatar in front of every mentioned in issue 123, it would get much more difficult to focus on the important content. After all I'm usually looking at the issue because of the discussion going on, not to find out that someone referenced this somewhere else. But in case I am looking for that reference, I can also find it faster.
I also find GitHub's solution awful. It makes it hard to focus on the discussion, but it's also annoying to find the references in case you are actually looking for them.
I agree that the long commit lists mentioned by @connorshea are not great (and it happens quite often), but the usual Assigned to or Mentioned in are almost perfect right now, IMHO.
@pelle I agree that a profile picture shouldn't be shown before every mention of a user, but I think icons would be useful for differentiating these from other items.
@hazelyang nice ^^ looks very readable, plus the icons really make a difference!
Only thing I thought of while looking at this was the following (but I think you experimented with this already):
Assigning different background color as well to secondary information such as the "untoggled system notes", so first rate information pops out even more
I disagree that the icons make a (positive) difference. For me they are unnecessary ballast. The "system note" looks completely different for the different types of messages anyways. Labels are rendered as such, commits show up as a list and then it's basically only mentions left.
Long lists of commits as mentioned by @connorshea are the real problem, affecting usability! Everything else is mostly a matter of taste and minor stylistic changes.
I think we should focus on 2. as this is a real problem in day-to-day work.
@dimitrieh Thanks for your suggestion. Currently, we don't have this pattern used in the comment threads (except the new comment your username is mentioned will be the light blue background), so maybe we should stick to the style we are using.
@pbr Thanks for you feedback. I think icons could help people to recognize the different system notes. Maybe we could collect more feedbacks about this and then we can decide if we need to do some changes.
@hazelyang@connorshea Sorry! Didn't see that "more" link. Sorry, I should look at things a bit more closely in the future or maybe just
But out of curiosity: Why are 5 commits shown for the "More.." case? Is that just an arbitrary number? Would 3 or 4 also do to make it even more compact? And what about fading out the list towards the bottom so idiots like me realize that there is more to come there?
Thanks @connorshea, that's exactly what I had in mind! I forgot where I had seen it, but most likely it was Slack They also do it for snippets, showing a "Click to expand" button on hover. Not sure we should do this hover thing, but I think the fading might work here.
Also, in that case I think we can reduce the number of lines/commits shown to 2-3 in many cases. If someone pushes 50 commits, it often doesn't really make a difference whether I see 2 or 5 right away. If I need more context, I will click "more"/"expand" anyways. But it will free up a lot of vertical space.
What do you think @hazelyang? Would love to hear your thoughts on this :)
@connorshea Exactly! Let's see whether the general idea works out visually and conceptually. Maybe we can then define two or three simple, sensible rules for how much to show in what situation I hate those UIs that show 5 lines and a "more" button, even though there is a total of 6 items. Printing the button sometimes takes up more space than showing the 6th line right away. Basing this on two or three rules instead of one can significantly improve UX, I think.
For one-commit pushes, I'd also suggest to not do a list at all, but show the hash inline (in @hazelyang's mockup that would be Job van der Voort @JobV added commit 480aaaaa *list of features* one month ago instead of ... added 1 commit one month ago: [newline] [list]). This kind of breaks the pattern, but the icon to the left of that "system note" will already give me an indication that this message is about added commits
@hazelyang I like that you changed it so the text aligns with other comments I echo chris' comment about seeing what it looks like with centered icons.
I like @pbr's idea about defining a rule for displaying the number of lines. I think defaulting to 3 lines, unless there has been 4 commits, is a good start.
@hazelyang They do look a little too far from the text. I think making them a tiny bit darker might draw the eye to them more and compensate. I don't know if it'll work, but we can try.
@hazelyang Rather than "Click to expand" what if we had "+# more" to show how many more commits there were? This is similar to how we show participants on issues.
I'm with @cperessini here.
Also, @hazelyang , have you considered having border-top/bottom that are not as wide as regular comments for system notes? Or perhaps removing them entirely for system notes?
Thanks @hazelyang.
Last idea before going to sleep: the problem with system notes is that they look too much like regular comments, making them harder to read. They are too intrusive. So what about this:
Add more left padding to system notes: at the moment the alignment for notes and comments is the same. Perhaps it shouldn't be.
Reduce the font size for system notes: do they really need to have the same font size? By definition they are less important than regular comments, hence change their styles?
I agree with @regisF, really nice work @hazelyang :thumbsup::smiley:
Personally, I think the system notes could use less vertical padding, making them more compact, but I don't feel that strongly about it, assuming that the new ones are not taller than the old/current ones.
Is this still expected to be implemented in time for %8.11?
Now that 8.11 is out, I'd like to bring this up again. I think we should also consider doing #20948 (closed) (show issue/MR title in the system note on mention) at the same time.
Is Frontend aware of this issue? UX did a great job on this and I think it would be great to get this into the early RCs for 8.12 to get some feedback before the 22nd.
I am a bit concerned about this issue. UX spent considerable time and effort on this, it is long ready for implementation, and entire Frontend is ignoring it for months.
A last try to direct some attention to this ready-for-implementation Frontend issue
Mentioning @JobV as you started this issue.
Mentioning @jschatz1 as you are heading the Frontend team.
Mentioning @tauriedavis as you tried getting people to look at this before.
Thanks, @dimitrieh! Looks great, @hazelyang. I love how clean and readable the design feels. I know there is a lot going on, and so many great ideas across the board, but I definitely don't want to lose this one. I'll keep my eye on this and see how I can help.
@dimitrieh: Instead of a mass toggle, what if the diff discussions are collapsed by default? Depends on how important all of the details of the diff discussions are when you are reading the issue vs. just knowing that the discussion is happening, and being able to easily drill in if you want to know more. I'm worried the mass toggle by itself on the page is yet another thing for a complicated page, and state for users to manage, as opposed to letting things naturally unfold out as needed. Thoughts?
about the idea for natural state being collapsed, I don't think that would work, unless we make it very clear in collapsed state, there may be some very important information in there still to be read.
It comes down to the fact, what you are compared to the issue.. For us UX'rs the system messages are mostly not going to matter, but for dev's it almost always is.
I have thought in the past about a toggle for hiding all system messages, but thats too much. I do think a toggle should be possible
I would expect it here i think:
like (quick mockup just to represent location):
or with an eye symbol .. although we I know there is much discussion going on for different meanings for icons ;)
@dimitrieh the button on the right is directly related to the # discussions resolved text, whereas the button on the left would not be. It should have some margin separating them since they're unrelated.
@dimitrieh@awhildy Thanks, good to see people are looking at this again. Still, I'd like to urge Frontend to implement @hazelyang's design from a few months ago. There were a number of design iterations already (as can be seen in this issue) and this would be a huge leap forward. Further iterations - for example regarding your proposal for hiding all system messages - can then be taken afterwards. If we now keep adding more and more to this issue, I'm afraid it will become too big and never be realized.
Have an option (at the server level and profile level) that allows you to show/hide specific types of messages in the issue tracker.
The options could be something like:
Issues:
reassignments
milestones
labels
Project:
commits
etc
The server level should specify the defaults for each profile as a boolean, and the profile have a 3 state checkbox for Show/Hide/Use Default, with use default using the setting that is setup at the server level.
it would be nice if there was a "+4 events" (for eg) that you could click on to then show all of the hidden events in between the issue comments.
Eg:
If I just have commits and reassignments showing it may be:
Commit 12345678
Reasign
+3 events
Then clicking +3 would expand all of the hidden ones such as:
Added Label X
Commit 12345678
Added Label Y
Reasign
Milestone Changed
Italic ones are the ones that were hidden
It would also be great if there was a setting in the same section to show commit messages so the full commit text could be listed rather than having to hold your mouse over each commit hash
Move rebase commits information into Commits tab with highlighted background + add a checkbox Show rebased comments . Rebase commits are still commits.
Add a Show events checkbox to Discussion tab which will just hide/close event messages (like assignee, milestone, etc) in the feed.
Rebase commits are still commits. True. But new commits also show up in the activity/discussion part of a MR. And I think this is extremely important information that should not get hidden/moved. Also, it can be important to know whether one comment was posted before or after a rebase.
That Show events thing might be an OK compromise, if UX can figure out a way of doing it elegantly. Personally, I don't consider it a high priority thing, though.
I propose to implement the design from a few months ago and try it. I think things will improve considerably. If that is not sufficient, we can take another iteration.
@i00
Sorry, not a fan of any of your proposals . Also, additional config options are very unlikely to be added.
Rebase commits are still commits. True. But new commits also show up in the activity/discussion part of a MR. And I think this is extremely important information that should not get hidden/moved. Also, it can be important to know whether one comment was posted before or after a rebase.
Why not to move all commits information into Commits tab?
The first tab is Discussion. What does commits information in this tab discuss?
What does commits information in this tab discuss?
The commit information provides essential background information for the discussion. As mentioned many times before in this issue, the system notes including "added commits" are very important and it is not acceptable to move (i.e. remove) them.
A toggle (that is switched off by default) to hide them - as you proposed - might be a compromise, for example for non-technical people following the events in a MR discussion.
I agree with @pbr. Let's keep this issue focused on implementing the designs from a couple months ago as they are definitely a step in the right direction. I've added them to the issue body up top so that they are easy to find.
I really appreciate the thoughts and conversation with other ideas on how to improve system messages even further, but want to keep this issue manageable. Additional thoughts on how to handle some specific system messages, etc. should be captured in another issue. Thanks!
@hazelyang I tried Menlo monospace font as per your suggestion but feels like it is not matching fine with other fonts fine. Please check the snapshot. Can we use other monospaced font?
@dimitrieh@tauriedavis
To implement this, we need to do some work on the backend to categorise notes. At the moment, all system notes have the system: true property, but no other identifying information. We can classify them based on the content (which is what @nmrony started doing in !6755 (merged)), but I wonder if that's the most robust solution. As far as I know, we don't guarantee that the text of these will always be the same.
Going forward, we can add a new field to the database for this. To handle legacy notes, we could just use a default icon, or classify them based on the content anyway - and then fill in the metadata. Or we could have a background task to classify the legacy notes. I don't really have a strong opinion on how we do this as it stands, but it definitely needs some Backend / Discussion work too
As I believe I stated before in this issue, I don't think the icons for system notes are that important. I'd suggest implementing this without those icons in the first iteration and then finding a decent solution for that icon-issue. IMHO, the big improvements by the design proposed in this issue are the reduced padding for system notes (making them less intrusive as this issue title mentions) and the collapsing of long commit lists.
@SeanPackham@pedroms Yeah agreed with you. We can remove the icon for 1st iteration until we find a good solution for that. Do you have any suggestion, @hazelyang ?
@nmrony@pbr@pedroms Agreed. Although I think the icons will help people to differentiate the system notes visually, I am not sure whether it's necessary for them. We can drop the icons in the first iteration and then do some improvements depends on the feedbacks from people in the future.
@hazelyang I vote in favor of icons; I find it pretty hard to scan what happened in the screenshot below, since the verbs of the system note messages are no longer horizontally aligned like they were before, because of the variable length of the username/name, and because the messages have the same text color as the username. It takes me more effort to find the place in the message that indicate what happened than before: "reassigned", "unmarked as work in progress", "started a discussion" "resolved all discussions", etc
I agree with @DouweM that the RC now deployed to .com makes it pretty hard to differentiate between things. In other words: I take everything back I said before about dropping the icons!
Plus, we'll need a design/icon for the "resolved discussions" system note that was introduced in the meantime (between the concept/design and implementation of this issue).
Also, the implementation does not match the design in some key elements:
When "shortening" commit lists, it says "Toggle commit list". Where was that decided? The design in this issue has "+N commits", which is very different, as it conveys a lot more important information. I think this should be changed ASAP.
The titles in the commit lists use a monospaced font, which does not really make sense to me. Also, the font size differs for lines containing commit ranges:
I haven't had the chance to check (against the design) properly, yet - but I feel there is still too much vertical padding around the system notes in the current implementation.
PS: Also, the "Compare with previous version" should be shown next to "Toggle commit list" "+N commits", IMHO. Currently, you have to expand the list to see that link.
(Yet another feature that was introduced while this issue was waiting for implementation.)
Thanks @nmrony! I think it makes sense to show the commit SHA in monospace, but the following text/title should have the normal font, IMHO. It looks particularly weird for 123 commits from master.
@pbr thanks for digging in to this so much! I'd actually be happy if we always showed the commit 'title' in monospace, but we don't, so we probably shouldn't do that here.
Thanks for the suggestion, @brodock! However, I would prefer to stick with the icons if we can. Visually quicker to parse, and cleaner type (as having so many different type styles in the same line can be a bit distracting).
I think it makes sense to show the commit SHA in monospace, but the following text/title should have the normal font
Monospaced fonts have fixed-width characters which are beneficial for having vertically aligned characters (typographers called them tabular figures) and they improve the legibility of software code that includes compressed syntax. Since we don't have tabular figures in Source Sans Pro, we use a monospaced font for both code and situations that required fixed-width characters (such as commit SHA's).
Commit SHA's benefit from monospaced fonts not only because of fixed-width characters but also because most of those fonts have slashed zeros, which improves legibility since commit SHA's usually mix digits and letters: fc035011 vs fc035011 (curious to see that GitLab doesn't render commit SHA's in comments in a monospaced font)
For commit messages, which are longer passages of text, monospaced fonts actually impair legibility. For these cases, proportionally spaced fonts are better (such as Source Sans Pro).
If you don't know how tabular figures work/look, check this example with “Numeral spacing” as “Tabular numerals”.
This solves the problem of keeping the hashes aligned, but still have the titles in our regular typeface. Unless there are any concerns with this, I think it makes sense to fix this. @nmrony: let us know if it helps to open this as a separate issue, or whatever would be easiest for you. Thanks!
@awhildy creating a new issue regarding commit list font. Second iteration MR needs more things to resolve to get it merged mentioned in here #24784 (closed). Please ignore my last comment
@nmrony should we close this issue in favour of the follow-up issues, as the system notes already look way better than they did before this was first opened?