Merge branch '14400-diff-redesign' into 'master'

Diff redesign

Fixes #14400 
Fixes #13500 

![diffs_redesign](/uploads/16da9557e497114d886ef5ece2f306b3/diffs_redesign.gif)

See merge request !3476

app/assets/stylesheets/framework/buttons.scss

@@ -7,6 +7,7 @@
   &:focus,
   &:active {
     outline: none;
+    background-color: $btn-active-gray;
     @include box-shadow($gl-btn-active-background);
   }
 }
@@ -27,7 +28,8 @@
     color: $color;
   }
 
-  &:active {
+  &:active,
+  &.active {
     @include box-shadow ($gl-btn-active-background);
 
     background-color: $dark;
@@ -61,7 +63,7 @@
 }
 
 @mixin btn-white {
-  @include btn-color($white-light, $border-white-light, $white-normal, $border-white-normal, $white-dark, $border-white-dark, #313236);
+  @include btn-color($white-light, $border-color, $white-normal, $border-white-normal, $white-dark, $border-white-dark, $btn-white-active);
 }
 
 .btn {
@@ -218,3 +220,26 @@
     margin-right: 5px;
   }
 }
+
+.btn-text-field {
+  width: 100%;
+  text-align: left;
+  padding: 6px 16px;
+  border-color: $border-color;
+  color: $btn-placeholder-gray;
+  background-color: $background-color;
+
+  &:hover,
+  &:active,
+  &:focus {
+    cursor: text;
+    box-shadow: none;
+    border-color: $border-color;
+    color: $btn-placeholder-gray;
+    background-color: $background-color;
+  }
+}
+
+.btn-file-option {
+  background: linear-gradient(180deg, $white-light 25%, $gray-light 100%);
+}

app/assets/stylesheets/framework/files.scss

@@ -15,12 +15,13 @@
 
   .file-title {
     position: relative;
-    background: $background-color;
+    background-color: $background-color;
     border-bottom: 1px solid $border-color;
     margin: 0;
     text-align: left;
     padding: 10px $gl-padding;
     word-wrap: break-word;
+    border-radius: 3px 3px 0 0;
 
     .file-actions {
       float: right;
@@ -49,7 +50,7 @@
       }
     }
 
-    a {
+    a:not(.btn) {
       color: $gl-dark-link-color;
     }
 

app/assets/stylesheets/framework/timeline.scss

@@ -14,10 +14,6 @@
       background: $row-hover;
     }
 
-    &:last-child {
-      border-bottom: none;
-    }
-
     .avatar {
       margin-right: 15px;
     }

app/assets/stylesheets/framework/variables.scss

@@ -10,10 +10,10 @@ $gutter_inner_width: 258px;
 /*
  * UI elements
  */
-$border-color:       #efeff1;
+$border-color:       #e5e5e5;
 $focus-border-color: #3aabf0;
 $table-border-color: #eef0f2;
-$background-color:   #faf9f9;
+$background-color:   #fafafa;
 
 /*
  * Text
@@ -81,7 +81,7 @@ $provider-btn-not-active-color: #4688f1;
 
 $white-light: #fff;
 $white-normal: #ededed;
-$white-dark: #ededed;
+$white-dark: #ececec;
 
 $gray-light: #faf9f9;
 $gray-normal: #f5f5f5;
@@ -108,6 +108,8 @@ $red-light: #e52c5a;
 $red-normal: #d22852;
 $red-dark: darken($red-normal, 5%);
 
+$black-transparent: rgba(0, 0, 0, 0.3);
+
 $border-white-light: #f1f2f4;
 $border-white-normal: #d6dae2;
 $border-white-dark: #c6cacf;
@@ -150,15 +152,22 @@ $gl-success: $green-normal;
 $gl-info: $blue-normal;
 $gl-warning: $orange-normal;
 $gl-danger: $red-normal;
-$gl-btn-active-background: rgba(0, 0, 0, 0.12);
-$gl-btn-active-gradient: inset 0 0 4px $gl-btn-active-background;
+$gl-btn-active-background: rgba(0, 0, 0, 0.16);
+$gl-btn-active-gradient: inset 0 2px 3px $gl-btn-active-background;
 
 /*
  * Commit Diff Colors
  */
 $added: #63c363;
 $deleted: #f77;
-
+$line-added: #ecfdf0;
+$line-added-dark: #c7f0d2;
+$line-removed: #fbe9eb;
+$line-removed-dark: #fac5cd;
+$line-number-old: #f9d7dc;
+$line-number-new: #ddfbe6;
+$match-line: #fafafa;
+$table-border-gray: #f0f0f0;
 /*
  * Fonts
  */
@@ -191,6 +200,13 @@ $dropdown-toggle-hover-border-color: darken($dropdown-toggle-border-color, 15%);
 $dropdown-toggle-icon-color: #c4c4c4;
 $dropdown-toggle-hover-icon-color: $dropdown-toggle-hover-border-color;
 
+/*
+* Buttons
+*/
+$btn-active-gray: #ececec;
+$btn-placeholder-gray: #c7c7c7;
+$btn-white-active: #848484;
+
 /*
  *  Award emoji
  */

app/assets/stylesheets/highlight/solarized_light.scss

@@ -6,7 +6,7 @@
   }
 
   .diff-line-num, .diff-line-num a {
-    color: rgba(0, 0, 0, 0.3);
+    color: $black-transparent;
   }
 
   // Code itself
@@ -30,7 +30,7 @@
     }
 
     .line_content.match {
-      color: rgba(0, 0, 0, 0.3);
+      color: $black-transparent;
       background: rgba(255, 255, 255, 0.4);
     }
   }

app/assets/stylesheets/highlight/white.scss

@@ -6,12 +6,12 @@
   }
 
   .diff-line-num, .diff-line-num a {
-    color: rgba(0, 0, 0, 0.3);
+    color: $black-transparent;
   }
 
   // Code itself
   pre.code, .diff-line-num {
-    border-color: $border-color;
+    border-color: $table-border-gray;
   }
 
   &, pre.code, .line_holder .line_content {
@@ -23,36 +23,36 @@
   .line_holder {
     .diff-line-num {
       &.old {
-        background: #fdd;
-        border-color: #f1c0c0;
+        background-color: $line-number-old;
+        border-color: $line-removed-dark;
       }
 
       &.new {
-        background: #dbffdb;
-        border-color: #c1e9c1;
+        background-color: $line-number-new;
+        border-color: $line-added-dark;
       }
     }
 
     .line_content {
       &.old {
-        background: #ffecec;
+        background: $line-removed;
 
         span.idiff {
-          background-color: #f8cbcb;
+          background-color: $line-removed-dark;
         }
       }
 
       &.new {
-        background: #eaffea;
+        background-color: $line-added;
 
         span.idiff {
-          background-color: #a6f3a6;
+          background-color: $line-added-dark;
         }
       }
 
       &.match {
-        color: rgba(0, 0, 0, 0.3);
-        background: #fafafa;
+        color: $black-transparent;
+        background: $match-line;
       }
     }
   }

app/assets/stylesheets/pages/diff.scss

@@ -2,6 +2,7 @@
 .diff-file {
   border: 1px solid $border-color;
   margin-bottom: $gl-padding;
+  border-radius: 3px;
 
   .diff-header {
     position: relative;
@@ -10,6 +11,7 @@
     padding: 10px 16px;
     color: #555;
     z-index: 10;
+    border-radius: 3px 3px 0 0;
 
     .diff-title {
       font-family: $monospace_font;
@@ -31,6 +33,7 @@
     overflow-y: hidden;
     background: #fff;
     color: #333;
+    border-radius: 0 0 3px 3px;
 
     .unfold {
       cursor: pointer;
@@ -325,6 +328,16 @@
   float: right;
 }
 
+.diffs {
+  .content-block {
+    border-bottom: none;
+  }
+}
+
+.files-changed {
+  border-bottom: none;
+}
+
 // Mobile
 @media (max-width: 480px) {
   .diff-title {

app/assets/stylesheets/pages/note_form.scss

 /**
  * Note Form
  */
-.reply-btn {
-  @extend .btn-primary;
-  margin: 10px $gl-padding;
+.comment-btn {
+  @extend .btn-create;
 }
+
 .diff-file .diff-content {
   tr.line_holder:hover > td .line_note_link {
     opacity: 1.0;
@@ -113,13 +113,12 @@
 .discussion-body,
 .diff-file {
   .notes .note {
-    border-color: #ddd;
     padding: 10px 15px;
   }
 
   .discussion-reply-holder {
-    background: $background-color;
-    border-top: 1px solid $border-color;
+    background-color: $white-light;
+    padding: 10px 16px;
   }
 }
 

app/assets/stylesheets/pages/notes.scss

@@ -58,6 +58,7 @@ ul.notes {
   .note {
     display: block;
     position: relative;
+    border-bottom: 1px solid $table-border-gray;
 
     &.is-editting {
       .note-header,
@@ -117,9 +118,6 @@ ul.notes {
       padding-bottom: 3px;
     }
 
-    &:last-child {
-      border-bottom: 1px solid $border-color;
-    }
   }
 }
 
@@ -137,14 +135,14 @@ ul.notes {
   font-family: $regular_font;
 
   td {
-    border: 1px solid #ddd;
+    border: 1px solid $table-border-gray;
     border-left: none;
 
     &.notes_line {
       vertical-align: middle;
       text-align: center;
       padding: 10px 0;
-      background: #fff;
+      background: $background-color;
       color: $text-color;
     }
     &.notes_line2 {
@@ -175,9 +173,6 @@ ul.notes {
     }
   }
 
-  .author_link {
-    font-weight: 600;
-  }
 }
 
 .note-headline-light,
@@ -203,14 +198,26 @@ ul.notes {
   line-height: 24px;
 
   .fa {
+    color: $notes-action-color;
     position: relative;
     top: 1px;
     font-size: 17px;
   }
 
-  .fa-trash-o {
-    top: 0;
-    font-size: 16px;
+  &.js-note-delete {
+    i {
+      &:hover {
+        color: $gl-text-red;
+      }
+    }
+  }
+
+  &.js-note-edit {
+    i {
+      &:hover {
+        color: $gl-link-color;
+      }
+    }
   }
 }
 

app/helpers/blob_helper.rb

@@ -27,9 +27,9 @@ module BlobHelper
                                      link_opts)
 
     if !on_top_of_branch?(project, ref)
-      button_tag "Edit", class: "btn btn-default disabled has-tooltip", title: "You can only edit files when you are on a branch", data: { container: 'body' }
+      button_tag "Edit", class: "btn disabled has-tooltip btn-file-option", title: "You can only edit files when you are on a branch", data: { container: 'body' }
     elsif can_edit_blob?(blob, project, ref)
-      link_to "Edit", edit_path, class: 'btn'
+      link_to "Edit", edit_path, class: 'btn btn-file-option'
     elsif can?(current_user, :fork_project, project)
       continue_params = {
         to:     edit_path,
@@ -38,7 +38,7 @@ module BlobHelper
       }
       fork_path = namespace_project_forks_path(project.namespace, project, namespace_key: current_user.namespace.id, continue: continue_params)
 
-      link_to "Edit", fork_path, class: 'btn', method: :post
+      link_to "Edit", fork_path, class: 'btn btn-file-option', method: :post
     end
   end
 

app/helpers/commits_helper.rb

@@ -197,7 +197,7 @@ module CommitsHelper
     link_to(
       namespace_project_blob_path(project.namespace, project,
                                   tree_join(commit_sha, diff.new_path)),
-      class: 'btn view-file js-view-file'
+      class: 'btn view-file js-view-file btn-file-option'
     ) do
       raw('View file @') + content_tag(:span, commit_sha[0..6],
                                        class: 'commit-short-id')

app/helpers/notes_helper.rb

@@ -69,10 +69,7 @@ module NotesHelper
       line_type:     line_type
     }
 
-    button_tag class: 'btn btn-nr reply-btn js-discussion-reply-button',
-               data: data, title: 'Add a reply' do
-      link_text = icon('comment')
-      link_text << ' Reply'
-    end
+    button_tag 'Reply...', class: 'btn btn-text-field js-discussion-reply-button',
+                           data: data, title: 'Add a reply'
   end
 end

app/views/projects/diffs/_diffs.html.haml

@@ -3,7 +3,7 @@
 
 - diff_files = safe_diff_files(diffs, diff_refs)
 
-.content-block.oneline-block
+.content-block.oneline-block.files-changed
   .inline-parallel-buttons
     .btn-group
       = inline_diff_btn

app/views/projects/diffs/_file.html.haml

@@ -3,7 +3,7 @@
     - if diff_file.diff.submodule?
       %span
         = icon('archive fw')
-        %strong
+        %span
           = submodule_link(blob, @commit.id, project.repository)
     - else
       = blob_icon blob.mode, blob.name
@@ -11,13 +11,13 @@
       = link_to "#diff-#{i}" do
         - if diff_file.renamed_file
           - old_path, new_path = mark_inline_diffs(diff_file.old_path, diff_file.new_path)
-          %strong.filename.old
+          .filename.old
             = old_path
           →
-          %strong.filename.new
+          .filename.new
             = new_path
         - else
-          %strong
+          %span
             = diff_file.new_path
           - if diff_file.deleted_file
             deleted
@@ -28,8 +28,8 @@
 
       .file-actions.hidden-xs
         - if blob_text_viewable?(blob)
-          = link_to '#', class: 'js-toggle-diff-comments btn active has-tooltip', title: "Toggle comments for this file" do
-            = icon('comments')
+          = link_to '#', class: 'js-toggle-diff-comments btn active has-tooltip btn-file-option', title: "Toggle comments for this file" do
+            = icon('comment')
           \
 
         - if editable_diff?(diff_file)

app/views/projects/notes/_diff_notes_with_reply.html.haml

@@ -3,9 +3,6 @@
 - if !defined?(line) || line == note.diff_line
   %tr.notes_holder
     %td.notes_line{ colspan: 2 }
-      %span.discussion-notes-count
-        %i.fa.fa-comment
-        = notes.count
     %td.notes_content
       %ul.notes{ data: { discussion_id: note.discussion_id } }
         = render notes

app/views/projects/notes/_diff_notes_with_reply_parallel.html.haml

@@ -4,9 +4,6 @@
 %tr.notes_holder
   - if note1
     %td.notes_line.old
-      %span.btn.disabled
-        %i.fa.fa-comment
-        = notes_left.count
     %td.notes_content.parallel.old
       %ul.notes{ data: { discussion_id: note1.discussion_id } }
         = render notes_left
@@ -19,9 +16,6 @@
 
   - if note2
     %td.notes_line.new
-      %span.btn.disabled
-        %i.fa.fa-comment
-        = notes_right.count
     %td.notes_content.parallel.new
       %ul.notes{ data: { discussion_id: note2.discussion_id } }
         = render notes_right

app/views/projects/notes/_note.html.haml

@@ -17,8 +17,8 @@
               %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: 'note-action-button js-note-delete' do
+              = icon('pencil')
+            = 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')
       .note-body{class: note_editable?(note) ? 'js-task-list-container' : ''}
         .note-text

features/steps/shared/diff_note.rb

@@ -155,7 +155,7 @@ module SharedDiffNote
 
   step 'I should see a discussion reply button' do
     page.within(diff_file_selector) do
-      expect(page).to have_button('Reply')
+      expect(page).to have_button('Reply...')
     end
   end
 

spec/features/notes_on_merge_requests_spec.rb

@@ -210,7 +210,7 @@ describe 'Comments', feature: true do
           is_expected.to have_content('Another comment on line 10')
           is_expected.to have_css('.notes_holder')
           is_expected.to have_css('.notes_holder .note', count: 1)
-          is_expected.to have_button('Reply')
+          is_expected.to have_button('Reply...')
         end
       end
     end