Merge branch 'less-intrusive-system-note' into 'master'

Less intrusive system note

## What does this MR do?
This MR is for making system notes less intrusive by implementing proposed design
 
## Are there points in the code the reviewer needs to double check?
System notes are reusing in issues, commits and MR discussions. May need to double check to make sure that the design is not breaking anywhere

## Why was this MR needed?
This merge request solve issue #19797 

## Screenshots (if relevant)
**Before:**
![merge-request-before](/uploads/91953598087a613078d80333a43815a7/merge-request-before.png)
**After**
![2016-11-15_17.41.46](/uploads/c809ba53cc34ee83bb439ed741dc39f3/2016-11-15_17.41.46.gif)

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [ ] API support added
- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

## What are the relevant issue numbers?
#19797, #23129

See merge request !6755

8 files changed, 153 insertions, 28 deletions

diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js
index 2a38ac28172..d83c41fae9d 100644
--- a/app/assets/javascripts/lib/utils/common_utils.js
+++ b/app/assets/javascripts/lib/utils/common_utils.js
@@ -125,6 +125,11 @@
       // Close any open tooltips
       $('.has-tooltip, [data-toggle="tooltip"]').tooltip('destroy');
     };
+
+    gl.utils.isMetaKey = function(e) {
+      return e.metaKey || e.ctrlKey || e.altKey || e.shiftKey;
+    };
+
   })(window);
 
 }).call(this);
diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js
index 44079bc3ca3..6cb87f9ba81 100644
--- a/app/assets/javascripts/notes.js
+++ b/app/assets/javascripts/notes.js
@@ -12,7 +12,7 @@
   var bind = function(fn, me){ return function(){ return fn.apply(me, arguments); }; };
 
   this.Notes = (function() {
-    var isMetaKey;
+    const MAX_VISIBLE_COMMIT_LIST_COUNT = 3;
 
     Notes.interval = null;
 
@@ -33,6 +33,7 @@
       this.resetMainTargetForm = bind(this.resetMainTargetForm, this);
       this.refresh = bind(this.refresh, this);
       this.keydownNoteText = bind(this.keydownNoteText, this);
+      this.toggleCommitList = bind(this.toggleCommitList, this);
       this.notes_url = notes_url;
       this.note_ids = note_ids;
       this.last_fetched_at = last_fetched_at;
@@ -46,6 +47,7 @@
       this.setPollingInterval();
       this.setupMainTargetNoteForm();
       this.initTaskList();
+      this.collapseLongCommitList();
     }
 
     Notes.prototype.addBinding = function() {
@@ -81,10 +83,13 @@
       $(document).on("click", ".js-add-diff-note-button", this.addDiffNote);
       // hide diff note form
       $(document).on("click", ".js-close-discussion-note-form", this.cancelDiscussionForm);
+      // toggle commit list
+      $(document).on("click", '.system-note-commit-list-toggler', this.toggleCommitList);
       // fetch notes when tab becomes visible
       $(document).on("visibilitychange", this.visibilityChange);
       // when issue status changes, we need to refresh data
       $(document).on("issuable:change", this.refresh);
+
       // when a key is clicked on the notes
       return $(document).on("keydown", ".js-note-text", this.keydownNoteText);
     };
@@ -114,9 +119,10 @@
 
     Notes.prototype.keydownNoteText = function(e) {
       var $textarea, discussionNoteForm, editNote, myLastNote, myLastNoteEditBtn, newText, originalText;
-      if (isMetaKey(e)) {
+      if (gl.utils.isMetaKey(e)) {
         return;
       }
+
       $textarea = $(e.target);
       // Edit previous note when UP arrow is hit
       switch (e.which) {
@@ -156,10 +162,6 @@
       }
     };
 
-    isMetaKey = function(e) {
-      return e.metaKey || e.ctrlKey || e.altKey || e.shiftKey;
-    };
-
     Notes.prototype.initRefresh = function() {
       clearInterval(Notes.interval);
       return Notes.interval = setInterval((function(_this) {
@@ -263,6 +265,7 @@
         $notesList.append(note.html).syntaxHighlight();
         // Update datetime format on the recent note
         gl.utils.localTimeAgo($notesList.find("#note_" + note.id + " .js-timeago"), false);
+        this.collapseLongCommitList();
         this.initTaskList();
         this.refresh();
         return this.updateNotesCount(1);
@@ -433,9 +436,9 @@
       var $form = $(xhr.target);
 
       if ($form.attr('data-resolve-all') != null) {
-        var projectPath = $form.data('project-path')
-            discussionId = $form.data('discussion-id'),
-            mergeRequestId = $form.data('noteable-iid');
+        var projectPath = $form.data('project-path');
+        var discussionId = $form.data('discussion-id');
+        var mergeRequestId = $form.data('noteable-iid');
 
         if (ResolveService != null) {
           ResolveService.toggleResolveForDiscussion(projectPath, mergeRequestId, discussionId);
@@ -844,9 +847,9 @@
       return this.notesCountBadge.text(parseInt(this.notesCountBadge.text()) + updateCount);
     };
 
-    Notes.prototype.resolveDiscussion = function () {
-      var $this = $(this),
-          discussionId = $this.attr('data-discussion-id');
+    Notes.prototype.resolveDiscussion = function() {
+      var $this = $(this);
+      var discussionId = $this.attr('data-discussion-id');
 
       $this
         .closest('form')
@@ -855,6 +858,36 @@
         .attr('data-project-path', $this.attr('data-project-path'));
     };
 
+    Notes.prototype.toggleCommitList = function(e) {
+      const $element = $(e.target);
+      const $closestSystemCommitList = $element.siblings('.system-note-commit-list');
+
+      $closestSystemCommitList.toggleClass('hide-shade');
+    };
+
+    /**
+    Scans system notes with `ul` elements in system note body
+    then collapse long commit list pushed by user to make it less
+    intrusive.
+     */
+    Notes.prototype.collapseLongCommitList = function() {
+      const systemNotes = $('#notes-list').find('li.system-note').has('ul');
+
+      $.each(systemNotes, function(index, systemNote) {
+        const $systemNote = $(systemNote);
+        const headerMessage = $systemNote.find('.note-text').find('p:first').text().replace(':', '');
+
+        $systemNote.find('.note-header .system-note-message').html(headerMessage);
+
+        if ($systemNote.find('li').length > MAX_VISIBLE_COMMIT_LIST_COUNT) {
+          $systemNote.find('.note-text').addClass('system-note-commit-list');
+          $systemNote.find('.system-note-commit-list-toggler').show();
+        } else {
+          $systemNote.find('.note-text').addClass('system-note-commit-list hide-shade');
+        }
+      });
+    };
+
     return Notes;
 
   })();
diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss
index 9bfa1c96a5d..cc16d83c210 100644
--- a/app/assets/stylesheets/pages/notes.scss
+++ b/app/assets/stylesheets/pages/notes.scss
@@ -35,11 +35,83 @@ ul.notes {
 
   .system-note {
     font-size: 14px;
-    padding-top: 10px;
-    padding-bottom: 10px;
-    background: #fdfdfd;
+    padding: 0;
+    clear: both;
+
+    &.timeline-entry::after {
+      clear: none;
+    }
+
+    .system-note-message {
+      text-transform: lowercase;
+
+      a {
+        color: $gl-link-color;
+        text-decoration: none;
+      }
+    }
+
+    .timeline-content {
+      padding: 14px 10px;
+    }
+
+    .note-body {
+      overflow: hidden;
+
+      .system-note-commit-list-toggler {
+        display: none;
+        padding: 10px 0 0;
+        cursor: pointer;
+      }
+
+      .note-text {
+        & p:first-child {
+          display: none;
+        }
+
+        &.system-note-commit-list {
+          max-height: 63px;
+          overflow: hidden;
+          display: block;
+
+          ul {
+            margin: 3px 0 3px 15px !important;
+
+            li {
+              font-family: $monospace_font;
+              font-size: 12px;
+            }
+          }
+
+          p:first-child {
+            display: none;
+          }
+
+          &::after {
+            content: '';
+            width: 100%;
+            height: 20px;
+            position: absolute;
+            left: 0;
+            bottom: 50px;
+            background: linear-gradient(rgba($gray-light, .3) 0, $white-light);
+          }
+
+          &.hide-shade {
+            max-height: 100%;
+            overflow: auto;
+
+            &::after {
+              background: transparent;
+            }
+          }
+        }
+      }
+    }
 
     .timeline-icon {
+      display: none;
+
       .avatar {
         visibility: hidden;
 
@@ -65,6 +137,12 @@ ul.notes {
     position: relative;
     border-bottom: 1px solid $table-border-gray;
 
+    &.note-discussion {
+      &.timeline-entry {
+        padding: 14px 10px;
+      }
+    }
+
     &.is-editting {
       .note-header,
       .note-text,
@@ -88,10 +166,8 @@ ul.notes {
         overflow: auto;
         word-wrap: break-word;
         @include md-typography;
-
         // Reset ul style types since we're nested inside a ul already
         @include bulleted-list;
-
         ul.task-list {
           ul:not(.task-list) {
             padding-left: 1.3em;
@@ -111,6 +187,11 @@ ul.notes {
       padding-bottom: 3px;
       padding-right: 20px;
 
+      p {
+        display: inline;
+        margin: 0;
+      }
+
       @media (min-width: $screen-sm-min) {
         padding-right: 0;
       }
@@ -238,6 +319,10 @@ ul.notes {
   }
 }
 
+.discussion-header {
+  font-size: 14px;
+}
+
 .note-headline-light,
 .discussion-headline-light {
   color: $notes-light-color;
diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml
index 077e8e64e5f..e4b4ea675d2 100644
--- a/app/views/discussions/_discussion.html.haml
+++ b/app/views/discussions/_discussion.html.haml
@@ -1,9 +1,6 @@
 - expanded = discussion.expanded?
 %li.note.note-discussion.timeline-entry
   .timeline-entry-inner
-    .timeline-icon
-      = link_to user_path(discussion.author) do
-        = image_tag avatar_icon(discussion.author), class: "avatar s40"
     .timeline-content
       .discussion.js-toggle-container{ class: discussion.id, data: { discussion_id: discussion.id } }
         .discussion-header
@@ -13,9 +10,7 @@
                 = icon("chevron-up")
               - else
                 = icon("chevron-down")
-
               Toggle discussion
-
           = link_to_member(@project, discussion.author, avatar: false)
 
           .inline.discussion-headline-light
@@ -38,8 +33,6 @@
 
             = time_ago_with_tooltip(discussion.created_at, placement: "bottom", html_class: "note-created-ago")
 
-          = render "discussions/headline", discussion: discussion
-
         .discussion-body.js-toggle-content{ class: ("hide" unless expanded) }
           - if discussion.diff_discussion? && discussion.diff_file
             = render "discussions/diff_with_notes", discussion: discussion
diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml
index afff15228c1..89ae64554c0 100644
--- a/app/views/projects/notes/_note.html.haml
+++ b/app/views/projects/notes/_note.html.haml
@@ -14,6 +14,9 @@
           = note.author.to_reference
           - unless note.system
             commented
+          - if note.system
+            %span{class: 'system-note-message'}
+              = h(note.note_html.downcase.html_safe)
           %a{ href: "##{dom_id(note)}" }
             = time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago')
         - unless note.system?
@@ -67,7 +70,9 @@
           = render 'projects/notes/edit_form', note: note
         .note-awards
           = render 'award_emoji/awards_block', awardable: note, inline: false
-
+        - if note.system
+          .system-note-commit-list-toggler
+            Toggle commit list
       - if note.attachment.url
         .note-attachment
           - if note.attachment.image?
diff --git a/changelogs/unreleased/less-intrusive-system-note.yml b/changelogs/unreleased/less-intrusive-system-note.yml
new file mode 100644
index 00000000000..87a5a3be246
--- /dev/null
+++ b/changelogs/unreleased/less-intrusive-system-note.yml
@@ -0,0 +1,4 @@
+---
+title: Make system notes less intrusive
+merge_request: 6755
+author:
diff --git a/spec/features/merge_requests/diff_notes_resolve_spec.rb b/spec/features/merge_requests/diff_notes_resolve_spec.rb
index d5e3d8e7eff..eab64bd4b54 100644
--- a/spec/features/merge_requests/diff_notes_resolve_spec.rb
+++ b/spec/features/merge_requests/diff_notes_resolve_spec.rb
@@ -201,7 +201,7 @@ feature 'Diff notes resolve', feature: true, js: true do
           expect(first('.line-resolve-btn')['data-original-title']).to eq("Resolved by #{user.name}")
         end
 
-        expect(page).to have_content('Last updated')
+        expect(page).not_to have_content('Last updated')
 
         page.within '.line-resolve-all-container' do
           expect(page).to have_content('0/1 discussion resolved')
diff --git a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb
index c3c3ab33872..8eceaad2457 100644
--- a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb
+++ b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb
@@ -73,7 +73,7 @@ feature 'Merge When Build Succeeds', feature: true, js: true do
       expect(page).to have_button "Merge When Build Succeeds"
 
       visit_merge_request(merge_request) # refresh the page
-      expect(page).to have_content "Canceled the automatic merge"
+      expect(page).to have_content "canceled the automatic merge"
     end
 
     it "allows the user to remove the source branch" do
@@ -101,7 +101,7 @@ feature 'Merge When Build Succeeds', feature: true, js: true do
       expect(page).not_to have_link "Merge When Build Succeeds"
     end
   end
-  
+
   def visit_merge_request(merge_request)
     visit namespace_project_merge_request_path(merge_request.project.namespace, merge_request.project, merge_request)
   end