Merge branch 'notes-actions-colours' into 'master'

Notes/discussions header

Part way towards getting the design for issues/merge requests to look more like the design in #3401 - I have standardised the headers for both issues & merge requests comments and discussions.

See merge request !3441

8 files changed, 112 insertions, 114 deletions

diff --git a/app/assets/stylesheets/framework/mobile.scss b/app/assets/stylesheets/framework/mobile.scss
index 5ea4f9a49db..475340f3f0c 100644
--- a/app/assets/stylesheets/framework/mobile.scss
+++ b/app/assets/stylesheets/framework/mobile.scss
@@ -107,7 +107,7 @@
   }
 
   .page-title {
-    .note_created_ago, .new-issue-link {
+    .note-created-ago, .new-issue-link {
       display: none;
     }
   }
diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss
index 2ea6aabe156..98fe794d362 100644
--- a/app/assets/stylesheets/framework/variables.scss
+++ b/app/assets/stylesheets/framework/variables.scss
@@ -209,3 +209,11 @@ $location-badge-bg: $gray-normal;
 $location-icon-color: #e7e9ed;
 $location-active-color: $gl-text-color;
 $location-active-bg: $search-input-border-color;
+
+/*
+ *  Notes
+ */
+$notes-light-color: #8e8e8e;
+$notes-action-color: #c3c3c3;
+$notes-role-color: #8e8e8e;
+$notes-role-border-color: #e4e4e4;
diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss
index 4bd2016bdcf..072de68c8e2 100644
--- a/app/assets/stylesheets/pages/notes.scss
+++ b/app/assets/stylesheets/pages/notes.scss
@@ -22,7 +22,7 @@ ul.notes {
     margin-left: 55px;
   }
 
-  .note_created_ago, .note-updated-at {
+  .note-created-ago, .note-updated-at {
     white-space: nowrap;
   }
 
@@ -39,53 +39,6 @@ ul.notes {
     }
   }
 
-  .discussion-header,
-  .note-header {
-    @extend .cgray;
-
-    a:hover {
-      text-decoration: none;
-    }
-
-    .avatar {
-      float: left;
-      margin-right: 10px;
-    }
-
-    .discussion-last-update,
-    .note-last-update {
-      &:before {
-        content: "\00b7";
-      }
-
-      a {
-        color: $gl-gray;
-
-        &:hover {
-          text-decoration: underline;
-        }
-      }
-    }
-    .author {
-      color: #4c4e54;
-      margin-right: 3px;
-
-      &:hover {
-        color: $gl-link-color;
-      }
-    }
-    .author-username {
-    }
-
-    .note-role {
-      float: right;
-      margin-top: 1px;
-      border: 1px solid #bbb;
-      background-color: transparent;
-      color: $gl-gray;
-    }
-  }
-
   .discussion-body {
     padding-top: 15px;
   }
@@ -205,33 +158,81 @@ ul.notes {
   }
 }
 
+.discussion-header,
+.note-header {
+  a {
+    color: inherit;
+
+    &:hover {
+      color: $gl-link-color;
+      text-decoration: none;
+    }
+  }
+
+  .author_link {
+    font-weight: 600;
+  }
+}
+
+.note-headline-light,
+.discussion-headline-light {
+  color: $notes-light-color;
+}
+
 /**
  * Actions for Discussions/Notes
  */
 
-.discussion,
-.note {
-  .discussion-actions,
-  .note-actions {
-    float: right;
-    margin-left: 10px;
+.discussion-actions,
+.note-actions {
+  float: right;
+  margin-left: 10px;
+  color: $notes-action-color;
+}
 
-    a {
-      margin-left: 5px;
-      color: $gl-gray;
+.note-action-button,
+.discussion-action-button {
+  display: inline-block;
+  margin-left: 10px;
+  line-height: 24px;
 
-      i.fa {
-        font-size: 16px;
-        line-height: 16px;
-      }
+  .fa {
+    position: relative;
+    top: 1px;
+    font-size: 17px;
+  }
 
-      &:hover {
-        @extend .cgray;
-        &.danger { @extend .cred; }
-      }
-    }
+  .fa-trash-o {
+    top: 0;
+    font-size: 16px;
   }
 }
+
+.discussion-toggle-button {
+  line-height: 20px;
+  font-size: 13px;
+
+  .fa {
+    margin-right: 3px;
+    font-size: 10px;
+    line-height: 18px;
+    vertical-align: top;
+  }
+}
+
+.note-role {
+  position: relative;
+  top: -2px;
+  display: inline-block;
+  padding-left: 4px;
+  padding-right: 4px;
+  color: $notes-role-color;
+  font-size: 12px;
+  line-height: 20px;
+  border: 1px solid $notes-role-border-color;
+  border-radius: $border-radius-base;
+}
+
 .diff-file .note .note-actions {
   right: 0;
   top: 0;
diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml
index 2cf32e6093d..34fe1743f4b 100644
--- a/app/views/projects/notes/_note.html.haml
+++ b/app/views/projects/notes/_note.html.haml
@@ -5,28 +5,21 @@
         = image_tag avatar_icon(note.author), alt: '', class: 'avatar s40'
     .timeline-content
       .note-header
+        = link_to_member(note.project, note.author, avatar: false)
+        .inline.note-headline-light
+          = "#{note.author.to_reference} commented"
+          %a{ href: "##{dom_id(note)}" }
+            = time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago')
         - if note_editable?(note)
           .note-actions
-            = link_to '#', title: 'Edit comment', class: 'js-note-edit' do
+            - access = note.project.team.human_max_access(note.author.id)
+            - if access
+              %span.note-role
+                = access
+            = link_to '#', title: 'Edit comment', class: 'note-action-button js-note-edit' do
               = icon('pencil-square-o')
-
-            = link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'js-note-delete danger' do
+            = link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'note-action-button js-note-delete danger' do
               = icon('trash-o')
-
-        - unless note.system
-          - access = note.project.team.human_max_access(note.author.id)
-          - if access
-            %span.note-role.label
-              = access
-
-        = link_to_member(note.project, note.author, avatar: false)
-
-        %span.author-username
-          = '@' + note.author.username
-
-        %span.note-last-update
-          %a{name: dom_id(note), href: "##{dom_id(note)}", title: 'Link here'}
-            = time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note_created_ago')
       .note-body{class: note_editable?(note) ? 'js-task-list-container' : ''}
         .note-text
           = preserve do
diff --git a/app/views/projects/notes/discussions/_active.html.haml b/app/views/projects/notes/discussions/_active.html.haml
index 4f15a99d061..cd8a5f0bd02 100644
--- a/app/views/projects/notes/discussions/_active.html.haml
+++ b/app/views/projects/notes/discussions/_active.html.haml
@@ -1,22 +1,20 @@
 - note = discussion_notes.first
 .discussion.js-toggle-container{ class: note.discussion_id }
   .discussion-header
+    = link_to_member(@project, note.author, avatar: false)
+    .inline.discussion-headline-light
+      = "#{note.author.to_reference} started a discussion"
+      = link_to diffs_namespace_project_merge_request_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code) do
+        on the diff
     .discussion-actions
-      = link_to "#", class: "js-toggle-button" do
+      = link_to "#", class: "discussion-action-button discussion-toggle-button js-toggle-button" do
         %i.fa.fa-chevron-up
         Show/hide discussion
-    %div
-      = link_to_member(@project, note.author, avatar: false)
-      started a discussion
-      = link_to diffs_namespace_project_merge_request_path(note.project.namespace, note.project, note.noteable, anchor: note.line_code) do
-        %strong on the diff
     .last-update.hide.js-toggle-content
       - last_note = discussion_notes.last
       last updated by
       = link_to_member(@project, last_note.author, avatar: false)
-
-      %span.discussion-last-update
-        #{time_ago_with_tooltip(last_note.updated_at, placement: 'bottom', html_class: 'discussion_updated_ago')}
+      #{time_ago_with_tooltip(last_note.updated_at, placement: 'bottom', html_class: 'discussion_updated_ago')}
 
   .discussion-body.js-toggle-content
     = render "projects/notes/discussions/diff", discussion_notes: discussion_notes, note: note
diff --git a/app/views/projects/notes/discussions/_commit.html.haml b/app/views/projects/notes/discussions/_commit.html.haml
index 8d0a1af787c..46f2ba4bbcf 100644
--- a/app/views/projects/notes/discussions/_commit.html.haml
+++ b/app/views/projects/notes/discussions/_commit.html.haml
@@ -3,21 +3,20 @@
 - commit_description = commit ? 'commit' : 'a deleted commit'
 .discussion.js-toggle-container{ class: note.discussion_id }
   .discussion-header
+    = link_to_member(@project, note.author, avatar: false)
+    .inline.discussion-headline-light
+      = "#{note.author.to_reference} started a discussion on #{commit_description}"
+      - if commit
+        = link_to(commit.short_id, namespace_project_commit_path(note.project.namespace, note.project, note.noteable), class: 'monospace')
     .discussion-actions
-      = link_to "#", class: "js-toggle-button" do
+      = link_to "#", class: "note-action-button discussion-toggle-button js-toggle-button" do
         %i.fa.fa-chevron-up
         Show/hide discussion
-    %div
-      = link_to_member(@project, note.author, avatar: false)
-      started a discussion on #{commit_description}
-      - if commit
-        = link_to(commit.short_id, namespace_project_commit_path(note.project.namespace, note.project, note.noteable), class: 'monospace')
     .last-update.hide.js-toggle-content
       - last_note = discussion_notes.last
       last updated by
       = link_to_member(@project, last_note.author, avatar: false)
-      %span.discussion-last-update
-        #{time_ago_with_tooltip(last_note.updated_at, placement: 'bottom', html_class: 'discussion_updated_ago')}
+      #{time_ago_with_tooltip(last_note.updated_at, placement: 'bottom', html_class: 'discussion_updated_ago')}
   .discussion-body.js-toggle-content
     - if note.for_diff_line?
       = render "projects/notes/discussions/diff", discussion_notes: discussion_notes, note: note
diff --git a/app/views/projects/notes/discussions/_outdated.html.haml b/app/views/projects/notes/discussions/_outdated.html.haml
index 218b0da3977..f8e000b424f 100644
--- a/app/views/projects/notes/discussions/_outdated.html.haml
+++ b/app/views/projects/notes/discussions/_outdated.html.haml
@@ -1,19 +1,18 @@
 - note = discussion_notes.first
 .discussion.js-toggle-container{ class: note.discussion_id }
   .discussion-header
+    = link_to_member(@project, note.author, avatar: false)
+    .inline.discussion-headline-light
+      = "#{note.author.to_reference} started a discussion"
+      on the outdated diff
     .discussion-actions
-      = link_to "#", class: "js-toggle-button" do
+      = link_to "#", class: "note-action-button discussion-toggle-button js-toggle-button" do
         %i.fa.fa-chevron-down
         Show/hide discussion
-    %div
-      = link_to_member(@project, note.author, avatar: false)
-      started a discussion on the
-      %strong outdated diff
-    %div
+    .last-update.hide.js-toggle-content
       - last_note = discussion_notes.last
       last updated by
       = link_to_member(@project, last_note.author, avatar: false)
-      %span.discussion-last-update
-        #{time_ago_with_tooltip(last_note.updated_at, placement: 'bottom', html_class: 'discussion_updated_ago')}
+      #{time_ago_with_tooltip(last_note.updated_at, placement: 'bottom', html_class: 'discussion_updated_ago')}
   .discussion-body.js-toggle-content.hide
     = render "projects/notes/discussions/diff", discussion_notes: discussion_notes, note: note
diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb
index 91fe19dd477..a4f02b590ea 100644
--- a/features/steps/project/merge_requests.rb
+++ b/features/steps/project/merge_requests.rb
@@ -326,7 +326,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
 
   step 'I should see a discussion has started on diff' do
     page.within(".notes .discussion") do
-      page.should have_content "#{current_user.name} started a discussion"
+      page.should have_content "#{current_user.name} #{current_user.to_reference} started a discussion"
       page.should have_content sample_commit.line_code_path
       page.should have_content "Line is wrong"
     end
@@ -334,7 +334,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
 
   step 'I should see a discussion by user "John Doe" has started on diff' do
     page.within(".notes .discussion") do
-      page.should have_content "#{user_exists("John Doe").name} started a discussion"
+      page.should have_content "#{user_exists("John Doe").name} #{user_exists("John Doe").to_reference} started a discussion"
       page.should have_content sample_commit.line_code_path
       page.should have_content "Line is wrong"
     end
@@ -350,7 +350,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
 
   step 'I should see a discussion has started on commit diff' do
     page.within(".notes .discussion") do
-      page.should have_content "#{current_user.name} started a discussion on commit"
+      page.should have_content "#{current_user.name} #{current_user.to_reference} started a discussion on commit"
       page.should have_content sample_commit.line_code_path
       page.should have_content "Line is wrong"
     end
@@ -358,7 +358,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
 
   step 'I should see a discussion has started on commit' do
     page.within(".notes .discussion") do
-      page.should have_content "#{current_user.name} started a discussion on commit"
+      page.should have_content "#{current_user.name} #{current_user.to_reference} started a discussion on commit"
       page.should have_content "One comment to rule them all"
     end
   end