Merge branch 'commits-ui' into 'master'

Updated commits UI

Closes #14633 

See merge request !4271

10 files changed, 140 insertions, 106 deletions

diff --git a/app/assets/stylesheets/pages/commits.scss b/app/assets/stylesheets/pages/commits.scss
index c8c6bbde084..5a6e55cf63f 100644
--- a/app/assets/stylesheets/pages/commits.scss
+++ b/app/assets/stylesheets/pages/commits.scss
@@ -7,84 +7,111 @@
   margin-right: 9px;
 }
 
-.lists-separator {
-  margin: 10px 0;
-  border-color: #ddd;
+.commit-header {
+  padding: 5px 10px;
+  background-color: $background-color;
+  border-top: 1px solid #eee;
+  border-bottom: 1px solid #eee;
+  font-size: 14px;
+
+  &:first-child {
+    border-top-width: 0;
+  }
 }
 
-.commits-row {
-  ul {
-    margin: 0;
+.commit-row-title {
+  line-height: 1;
+  margin-bottom: 7px;
 
-    li.commit {
-      padding: 8px 0;
-    }
+  .notes_count {
+    float: right;
+    margin-right: 10px;
+  }
+
+  .str-truncated {
+    max-width: 70%;
   }
 
-  .commits-row-date {
-    font-size: 15px;
-    line-height: 20px;
-    margin-bottom: 5px;
+  .commit-row-message {
+    color: $gl-dark-link-color;
+  }
+
+  .text-expander {
+    display: inline-block;
+    background: $gray-light;
+    color: $gl-placeholder-color;
+    padding: 0 5px;
+    cursor: pointer;
+    border: 1px solid $border-gray-dark;
+    border-radius: $border-radius-default;
+    margin-left: 5px;
+
+    &:hover {
+      background-color: darken($gray-light, 10%);
+      text-decoration: none;
+    }
   }
 }
 
-li.commit {
-  list-style: none;
+.commit-actions {
+  @media (min-width: $screen-sm-min) {
+    float: right;
+    margin-left: $gl-padding;
+    margin-top: 2px;
+    font-size: 0;
+  }
 
-  .commit-row-title {
-    font-size: $list-font-size;
-    line-height: 20px;
-    margin-bottom: 2px;
+  .btn-transparent {
+    padding-left: 0;
+    padding-right: 0;
+  }
 
-    .btn-clipboard {
-      margin-top: -1px;
+  .btn {
+    &:not(:first-child) {
+      margin-left: $gl-padding;
     }
+  }
+}
 
-    .notes_count {
-      float: right;
-      margin-right: 10px;
-    }
+.commit-short-id {
+  font-family: $monospace_font;
+  font-weight: 600;
+}
 
-    .commit_short_id {
-      min-width: 65px;
-      color: $gl-dark-link-color;
-      font-family: $monospace_font;
-    }
+.commit {
+  padding: 10px 0;
 
-    .str-truncated {
-      max-width: 70%;
-    }
+  @media (min-width: $screen-sm-min) {
+    padding-left: 46px;
+  }
 
-    .commit-row-message {
-      color: $gl-dark-link-color;
+  &:not(:last-child) {
+    border-bottom: 1px solid #eee;
+  }
 
-      &:hover {
-        text-decoration: underline;
-      }
-    }
+  a,
+  button {
+    color: $gl-dark-link-color;
+    vertical-align: baseline;
+  }
 
-    .text-expander {
-      background: #eee;
-      color: #555;
-      padding: 0 5px;
-      cursor: pointer;
-      margin-left: 4px;
-      &:hover {
-        background-color: #ddd;
-      }
-    }
+  .avatar {
+    margin-left: -46px;
   }
 
   .item-title {
     display: inline-block;
-    max-width: 70%;
+
+    @media (min-width: $screen-sm-min) {
+      max-width: 70%;
+    }
   }
 
   .commit-row-description {
     font-size: 14px;
     border-left: 1px solid #eee;
     padding: 10px 15px;
-    margin: 5px 0 10px 5px;
+    margin: 10px 0;
     background: #f9f9f9;
     display: none;
 
@@ -102,7 +129,7 @@ li.commit {
 
   .commit-row-info {
     color: $gl-gray;
-    line-height: 24px;
+    line-height: 1;
 
     a {
       color: $gl-gray;
@@ -111,10 +138,6 @@ li.commit {
     .avatar {
       margin-right: 8px;
     }
-
-    .committed_ago {
-      display: inline-block;
-    }
   }
 
   &.inline-commit {
diff --git a/app/assets/stylesheets/pages/tree.scss b/app/assets/stylesheets/pages/tree.scss
index f16fc7f388f..cfb6e2e888e 100644
--- a/app/assets/stylesheets/pages/tree.scss
+++ b/app/assets/stylesheets/pages/tree.scss
@@ -101,7 +101,7 @@
   margin: 0;
 
   .commit {
-    padding: 0;
+    padding: 0 0 0 55px;
 
     .commit-row-title {
       .commit-row-message {
diff --git a/app/helpers/button_helper.rb b/app/helpers/button_helper.rb
index f742922d926..07a3f452460 100644
--- a/app/helpers/button_helper.rb
+++ b/app/helpers/button_helper.rb
@@ -17,7 +17,15 @@ module ButtonHelper
   def clipboard_button(data = {})
     content_tag :button,
       icon('clipboard'),
-      class: 'btn btn-clipboard',
+      class: "btn",
+      data: data,
+      type: :button
+  end
+
+  def clipboard_button_with_class(data = {}, css_class: 'btn-clipboard')
+    content_tag :button,
+      icon('clipboard'),
+      class: "btn #{css_class}",
       data: data,
       type: :button
   end
diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb
index 07e5c146844..8e4ae1e6aec 100644
--- a/app/helpers/ci_status_helper.rb
+++ b/app/helpers/ci_status_helper.rb
@@ -38,10 +38,10 @@ module CiStatusHelper
     icon(icon_name + ' fw')
   end
 
-  def render_commit_status(commit, tooltip_placement: 'auto left')
+  def render_commit_status(commit, tooltip_placement: 'auto left', cssclass: '')
     project = commit.project
     path = builds_namespace_project_commit_path(project.namespace, project, commit)
-    render_status_with_link('commit', commit.status, path, tooltip_placement)
+    render_status_with_link('commit', commit.status, path, tooltip_placement, cssclass: cssclass)
   end
 
   def render_pipeline_status(pipeline, tooltip_placement: 'auto left')
@@ -57,10 +57,10 @@ module CiStatusHelper
 
   private
 
-  def render_status_with_link(type, status, path, tooltip_placement)
+  def render_status_with_link(type, status, path, tooltip_placement, cssclass: '')
     link_to ci_icon_for_status(status),
             path,
-            class: "ci-status-link ci-status-icon-#{status.dasherize}",
+            class: "ci-status-link ci-status-icon-#{status.dasherize} #{cssclass}",
             title: "#{type.titleize}: #{ci_label_for_status(status)}",
             data: { toggle: 'tooltip', placement: tooltip_placement }
   end
diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb
index 493505e0c95..474041eccbb 100644
--- a/app/helpers/commits_helper.rb
+++ b/app/helpers/commits_helper.rb
@@ -16,6 +16,16 @@ module CommitsHelper
     commit_person_link(commit, options.merge(source: :committer))
   end
 
+  def commit_author_avatar(commit, options = {})
+    options = options.merge(source: :author)
+    user = commit.send(options[:source])
+
+    source_email = clean(commit.send "#{options[:source]}_email".to_sym)
+    person_email = user.try(:email) || source_email
+
+    image_tag(avatar_icon(person_email, options[:size]), class: "avatar #{"s#{options[:size]}" if options[:size]} hidden-xs", width: options[:size], alt: "")
+  end
+
   def image_diff_class(diff)
     if diff.deleted_file
       "deleted"
@@ -102,24 +112,24 @@ module CommitsHelper
     if current_controller?(:projects, :commits)
       if @repo.blob_at(commit.id, @path)
         return link_to(
-          "Browse File »",
+          "Browse File",
           namespace_project_blob_path(project.namespace, project,
                                       tree_join(commit.id, @path)),
-          class: "pull-right"
+          class: "btn btn-default"
         )
       elsif @path.present?
         return link_to(
-          "Browse Directory »",
+          "Browse Directory",
           namespace_project_tree_path(project.namespace, project,
                                       tree_join(commit.id, @path)),
-          class: "pull-right"
+          class: "btn btn-default"
         )
       end
     end
     link_to(
       "Browse Files",
       namespace_project_tree_path(project.namespace, project, commit),
-      class: "pull-right"
+      class: "btn btn-default"
     )
   end
 
@@ -187,12 +197,10 @@ module CommitsHelper
     source_email = clean(commit.send "#{options[:source]}_email".to_sym)
 
     person_name = user.try(:name) || source_name
-    person_email = user.try(:email) || source_email
 
     text =
       if options[:avatar]
-        avatar = image_tag(avatar_icon(person_email, options[:size]), class: "avatar #{"s#{options[:size]}" if options[:size]}", width: options[:size], alt: "")
-        %Q{#{avatar} <span class="commit-#{options[:source]}-name">#{person_name}</span>}
+        %Q{<span class="commit-#{options[:source]}-name">#{person_name}</span>}
       else
         person_name
       end
diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml
index 367027182b6..a959b34a539 100644
--- a/app/views/projects/commits/_commit.html.haml
+++ b/app/views/projects/commits/_commit.html.haml
@@ -9,26 +9,30 @@
 
 = cache(cache_key) do
   %li.commit.js-toggle-container{ id: "commit-#{commit.short_id}" }
+    = commit_author_avatar(commit, size: 36)
     .commit-row-title
       %span.item-title
         = link_to_gfm commit.title, namespace_project_commit_path(project.namespace, project, commit.id), class: "commit-row-message"
+        %span.commit-row-message.visible-xs-inline
+          &middot;
+          = commit.short_id
+        - if commit.status
+          = render_commit_status(commit, cssclass: 'visible-xs-inline')
         - if commit.description?
-          %a.text-expander.js-toggle-button ...
+          %a.text-expander.hidden-xs.js-toggle-button ...
 
-      .pull-right
+      .commit-actions.hidden-xs
         - if commit.status
-          = render_commit_status(commit)
-        = clipboard_button(clipboard_text: commit.id)
-        = link_to commit.short_id, namespace_project_commit_path(project.namespace, project, commit), class: "commit_short_id"
+          = render_commit_status(commit, cssclass: 'btn btn-transparent')
+        = clipboard_button_with_class({ clipboard_text: commit.id }, css_class: 'btn-transparent')
+        = link_to commit.short_id, namespace_project_commit_path(project.namespace, project, commit), class: "commit-short-id btn btn-transparent"
+        = link_to_browse_code(project, commit)
 
     - if commit.description?
-      .commit-row-description.js-toggle-content
-        %pre
-          = preserve(markdown(escape_once(commit.description), pipeline: :single_line, author: commit.author))
+      %pre.commit-row-description.js-toggle-content
+        = preserve(markdown(escape_once(commit.description), pipeline: :single_line, author: commit.author))
 
     .commit-row-info
-      by
-      = commit_author_link(commit, avatar: true, size: 24)
-      .committed_ago
-        #{time_ago_with_tooltip(commit.committed_date)} &nbsp;
-      = link_to_browse_code(project, commit)
+      = commit_author_link(commit, avatar: false, size: 24)
+      authored
+      #{time_ago_with_tooltip(commit.committed_date)}
diff --git a/app/views/projects/commits/_commits.html.haml b/app/views/projects/commits/_commits.html.haml
index 7283a78a64e..dd12eae8f7e 100644
--- a/app/views/projects/commits/_commits.html.haml
+++ b/app/views/projects/commits/_commits.html.haml
@@ -4,18 +4,11 @@
 - commits, hidden = limited_commits(@commits)
 
 - commits.chunk { |c| c.committed_date.in_time_zone.to_date }.each do |day, commits|
-  .row.commits-row
-    .col-md-2.hidden-xs.hidden-sm
-      %h5.commits-row-date
-        %i.fa.fa-calendar
-        %span= day.strftime('%d %b, %Y')
-      .light
-        = pluralize(commits.count, 'commit')
-    .col-md-10.col-sm-12
-      %ul.content-list
-        = render commits, project: project
-  %hr.lists-separator
+  %li.commit-header= "#{day.strftime('%d %b, %Y')} #{pluralize(commits.count, 'commit')}"
+  %li.commits-row
+    %ul.list-unstyled.commit-list
+      = render commits, project: project
 
 - if hidden > 0
-  .alert.alert-warning
+  %li.alert.alert-warning
     #{number_with_delimiter(hidden)} additional commits have been omitted to prevent performance issues.
diff --git a/app/views/projects/commits/show.html.haml b/app/views/projects/commits/show.html.haml
index 76ba0bea36d..51ca4eb903e 100644
--- a/app/views/projects/commits/show.html.haml
+++ b/app/views/projects/commits/show.html.haml
@@ -23,21 +23,18 @@
             Create Merge Request
 
       .control
-        = form_tag(namespace_project_commits_path(@project.namespace, @project, @id), method: :get, class: 'pull-left commits-search-form') do
-          = search_field_tag :search, params[:search], { placeholder: 'Filter by commit message', id: 'commits-search', class: 'form-control search-text-input', spellcheck: false }
-
+        = form_tag(namespace_project_commits_path(@project.namespace, @project, @id), method: :get, class: 'commits-search-form') do
+          = search_field_tag :search, params[:search], { placeholder: 'Filter by commit message', id: 'commits-search', class: 'form-control search-text-input input-short', spellcheck: false }
       - if current_user && current_user.private_token
         .control
           = link_to namespace_project_commits_path(@project.namespace, @project, @ref, {format: :atom, private_token: current_user.private_token}), title: "Commits Feed", class: 'btn' do
             = icon("rss")
-
-
     %ul.breadcrumb.repo-breadcrumb
       = commits_breadcrumbs
 
   %div{id: dom_id(@project)}
-    #commits-list.content_list= render "commits", project: @project
-  .clear
+    %ol#commits-list.list-unstyled.content_list
+      = render "commits", project: @project
   = spinner
 
 :javascript
diff --git a/app/views/projects/merge_requests/show/_commits.html.haml b/app/views/projects/merge_requests/show/_commits.html.haml
index a8f09f855d4..0b05785430b 100644
--- a/app/views/projects/merge_requests/show/_commits.html.haml
+++ b/app/views/projects/merge_requests/show/_commits.html.haml
@@ -2,4 +2,5 @@
   = icon("sort-amount-desc")
   Most recent commits displayed first
 
-= render "projects/commits/commits", project: @merge_request.project
+%ol#commits-list.list-unstyled
+  = render "projects/commits/commits", project: @merge_request.project
diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb
index 2c0498de3b9..79a3ed8197e 100644
--- a/features/steps/project/source/browse_files.rb
+++ b/features/steps/project/source/browse_files.rb
@@ -202,8 +202,8 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
   end
 
   step 'I see Browse dir link' do
-    expect(page).to have_link 'Browse Directory »'
-    expect(page).not_to have_link 'Browse Code »'
+    expect(page).to have_link 'Browse Directory'
+    expect(page).not_to have_link 'Browse Code'
   end
 
   step 'I click on readme file' do
@@ -219,7 +219,7 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
 
   step 'I see Browse code link' do
     expect(page).to have_link 'Browse Files'
-    expect(page).not_to have_link 'Browse Directory »'
+    expect(page).not_to have_link 'Browse Directory'
   end
 
   step 'I click on Permalink' do