Merge branch 'ui-improvements' into 'master'

UI improvements

Set of UI changes mostly for issue and merge request

* return edit/close buttons to old position (right of title)
* make 'Issue #1' header smaller
* move mr commits to separate tab
* change inline/side diff switcher to buttons from tabs
* make issue sidebar start with dicsussion block

See merge request !1351

app/assets/javascripts/merge_request.js.coffee

@@ -89,6 +89,9 @@ class @MergeRequest
         this.$('.merge-request-tabs .diffs-tab').addClass 'active'
         this.loadDiff() unless @diffs_loaded
         this.$('.diffs').show()
+      when 'commits'
+        this.$('.merge-request-tabs .commits-tab').addClass 'active'
+        this.$('.commits').show()
       else
         this.$('.merge-request-tabs .notes-tab').addClass 'active'
         this.$('.notes').show()

app/assets/stylesheets/generic/issue_box.scss

@@ -6,7 +6,9 @@
 .issue-box {
   display: inline-block;
-  padding: 0 10px;
+  padding: 7px 13px;
+  font-weight: normal;
+  margin-right: 5px;
   &.issue-box-closed {
     background-color: $bg_danger;

app/assets/stylesheets/sections/merge_requests.scss

@@ -19,13 +19,15 @@
   }
 }
-.merge-request .merge-request-tabs{
-  margin: 20px 0;
-
-  li {
-    a {
-      padding: 15px 40px;
-      font-size: 14px;
+@media(min-width: $screen-sm-max) {
+  .merge-request .merge-request-tabs{
+    margin: 20px 0;
+
+    li {
+      a {
+        padding: 15px 40px;
+        font-size: 14px;
+      }
     }
   }
 }
@@ -102,6 +104,7 @@
 .mr-state-widget {
   background: $box_bg;
   margin-bottom: 20px;
+  color: #666;
   @include box-shadow(0 1px 1px rgba(0, 0, 0, 0.09));
   .ci_widget {
@@ -146,7 +149,6 @@
     padding: 10px 15px;
     h4 {
-      font-size: 20px;
       font-weight: normal;
     }

app/assets/stylesheets/sections/sidebar.scss

@@ -60,7 +60,7 @@
     font-size: 13px;
     line-height: 20px;
     text-shadow: 0 1px 2px #FFF;
-    padding-left: 67px;
+    padding-left: 20px;
     &:hover {
       text-decoration: none;
@@ -75,6 +75,7 @@
     i {
       width: 20px;
       color: #888;
+      margin-right: 23px;
     }
   }
 }
@@ -91,7 +92,7 @@
     a {
       padding: 5px 15px;
       font-size: 12px;
-      padding-left: 67px;
+      padding-left: 20px;
     }
   }
 }

app/helpers/diff_helper.rb

@@ -117,4 +117,22 @@ module DiffHelper
     [comments_left, comments_right]
   end
+
+  def inline_diff_btn
+    params_copy = params.dup
+    params_copy[:view] = 'inline'
+
+    link_to url_for(params_copy), id: "commit-diff-viewtype", class: (params[:view] != 'parallel' ? 'btn active' : 'btn') do
+      'Inline'
+    end
+  end
+
+  def parallel_diff_btn
+    params_copy = params.dup
+    params_copy[:view] = 'parallel'
+
+    link_to url_for(params_copy), id: "commit-diff-viewtype", class: (params[:view] == 'parallel' ? 'btn active' : 'btn') do
+      'Side-by-side'
+    end
+  end
 end

app/views/dashboard/projects.html.haml

@@ -38,17 +38,19 @@
             = link_to project_path(project), class: dom_class(project) do
               = project.name_with_namespace
+            - if project.forked_from_project
+               
+              %small
+                %i.fa.fa-code-fork
+                Forked from:
+                = link_to project.forked_from_project.name_with_namespace, project_path(project.forked_from_project)
+
             - if current_user.can_leave_project?(project)
               .pull-right
                 = link_to leave_project_team_members_path(project), data: { confirm: "Leave project?"}, method: :delete, remote: true, class: "btn-tiny btn remove-row", title: 'Leave project' do
                   %i.fa.fa-sign-out
                   Leave
-            - if project.forked_from_project
-              %small.pull-right
-                %i.fa.fa-code-fork
-                Forked from:
-                = link_to project.forked_from_project.name_with_namespace, project_path(project.forked_from_project)
           .project-info
             .pull-right
               - if project.archived?

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

@@ -2,15 +2,9 @@
   .col-md-8
     = render 'projects/diffs/stats', diffs: diffs
   .col-md-4
-    %ul.nav.nav-tabs
-      %li.pull-right{class: params[:view] == 'parallel' ? 'active' : ''}
-        - params_copy = params.dup
-        - params_copy[:view] = 'parallel'
-        = link_to "Side-by-side Diff", url_for(params_copy), {id: "commit-diff-viewtype"}
-      %li.pull-right{class: params[:view] != 'parallel' ? 'active' : ''}
-        - params_copy[:view] = 'inline'
-        = link_to "Inline Diff", url_for(params_copy), {id: "commit-diff-viewtype"}
-
+    .btn-group.pull-right
+      = inline_diff_btn
+      = parallel_diff_btn
 - if show_diff_size_warning?(diffs)
   = render 'projects/diffs/warning', diffs: diffs

app/views/projects/issues/_discussion.html.haml

+- content_for :note_actions do
+  - if can?(current_user, :modify_issue, @issue)
+    - if @issue.closed?
+      = link_to 'Reopen Issue', project_issue_path(@project, @issue, issue: {state_event: :reopen }, status_only: true), method: :put, class: "btn btn-grouped btn-reopen js-note-target-reopen", title: 'Reopen Issue'
+    - else
+      = link_to 'Close Issue', project_issue_path(@project, @issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn btn-grouped btn-close js-note-target-close", title: "Close Issue"
+.row
+  .col-md-9
+    .participants
+      %cite.cgray
+        = pluralize(@issue.participants.count, 'participant')
+      - @issue.participants.each do |participant|
+        = link_to_member(@project, participant, name: false, size: 24)
+
+    .voting_notes#notes= render "projects/notes/notes_with_form"
+  .col-md-3.hidden-sm.hidden-xs
+    %div
+    .clearfix
+      %span.slead.has_tooltip{:"data-original-title" => 'Cross-project reference'}
+        = cross_project_reference(@project, @issue)
+    %hr
+    .clearfix
+      .votes-holder
+        %h6 Votes
+        #votes= render 'votes/votes_block', votable: @issue
+    %hr
+    .context
+      %cite.cgray
+        = render partial: 'issue_context', locals: { issue: @issue }
+
+    - if @issue.labels.any?
+      %hr
+      %h6 Labels
+      .issue-show-labels
+        - @issue.labels.each do |label|
+          = link_to project_issues_path(@project, label_name: label.name) do
+            %p= render_colored_label(label)

app/views/projects/issues/show.html.haml

-%h3.page-title
+%h4.page-title
   .issue-box{ class: issue_box_class(@issue) }
     - if @issue.closed?
       Closed
     - else
       Open
   Issue ##{@issue.iid}
-  .pull-right.creator
-    %small Created by #{link_to_member(@project, @issue.author)} #{issue_timestamp(@issue)}
-%hr
-.row
-  .col-sm-9
-    %h3.issue-title
-      = gfm escape_once(@issue.title)
-    %div
-      - if @issue.description.present?
-        .description
-          .wiki
-            = preserve do
-              = markdown(@issue.description, parse_tasks: true)
-    %hr
-    - content_for :note_actions do
-      - if can?(current_user, :modify_issue, @issue)
-        - if @issue.closed?
-          = link_to 'Reopen Issue', project_issue_path(@project, @issue, issue: {state_event: :reopen }, status_only: true), method: :put, class: "btn btn-grouped btn-reopen js-note-target-reopen", title: 'Reopen Issue'
-        - else
-          = link_to 'Close Issue', project_issue_path(@project, @issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn btn-grouped btn-close js-note-target-close", title: "Close Issue"
-    .participants
-      %cite.cgray
-        = pluralize(@issue.participants.count, 'participant')
-      - @issue.participants.each do |participant|
-        = link_to_member(@project, participant, name: false, size: 24)
-      .issue-show-labels.pull-right
-        - @issue.labels.each do |label|
-          = link_to project_issues_path(@project, label_name: label.name) do
-            = render_colored_label(label)
-    .voting_notes#notes= render "projects/notes/notes_with_form"
-  .col-sm-3
-    %div
-      - if can?(current_user, :write_issue, @project)
-        = link_to new_project_issue_path(@project), class: "btn btn-block", title: "New Issue", id: "new_issue_link" do
-          %i.fa.fa-plus
-          New Issue
-      - if can?(current_user, :modify_issue, @issue)
-        - if @issue.closed?
-          = link_to 'Reopen', project_issue_path(@project, @issue, issue: {state_event: :reopen }, status_only: true), method: :put,  class: "btn btn-block btn-reopen"
-        - else
-          = link_to 'Close', project_issue_path(@project, @issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn btn-block btn-close", title: "Close Issue"
-        = link_to edit_project_issue_path(@project, @issue), class: "btn btn-block issuable-edit" do
-          %i.fa.fa-pencil-square-o
-          Edit
-    .clearfix
-      %span.slead.has_tooltip{:"data-original-title" => 'Cross-project reference'}
-        = cross_project_reference(@project, @issue)
-    %hr
-    .clearfix
-      .votes-holder
-        %h6 Votes
-        #votes= render 'votes/votes_block', votable: @issue
-    %hr
-    .context
-      %cite.cgray
-        = render partial: 'issue_context', locals: { issue: @issue }
+  %small.creator
+    · created by #{link_to_member(@project, @issue.author)} #{issue_timestamp(@issue)}
+
+  .pull-right
+    - if can?(current_user, :write_issue, @project)
+      = link_to new_project_issue_path(@project), class: "btn btn-grouped", title: "New Issue", id: "new_issue_link" do
+        %i.fa.fa-plus
+        New Issue
+    - if can?(current_user, :modify_issue, @issue)
+      - if @issue.closed?
+        = link_to 'Reopen', project_issue_path(@project, @issue, issue: {state_event: :reopen }, status_only: true), method: :put,  class: "btn btn-grouped btn-reopen"
+      - else
+        = link_to 'Close', project_issue_path(@project, @issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn btn-grouped btn-close", title: "Close Issue"
+      = link_to edit_project_issue_path(@project, @issue), class: "btn btn-grouped issuable-edit" do
+        %i.fa.fa-pencil-square-o
+        Edit
+%hr
+%h3.issue-title
+  = gfm escape_once(@issue.title)
+%div
+  - if @issue.description.present?
+    .description
+      .wiki
+        = preserve do
+          = markdown(@issue.description, parse_tasks: true)
+
+%hr
+= render "projects/issues/discussion"

app/views/projects/merge_requests/_discussion.html.haml

+- content_for :note_actions do
+  - if can?(current_user, :modify_merge_request, @merge_request)
+    - if @merge_request.open?
+      = link_to 'Close', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :close }), method: :put, class: "btn btn-grouped btn-close close-mr-link js-note-target-close", title: "Close merge request"
+    - if @merge_request.closed?
+      = link_to 'Reopen', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-grouped btn-reopen reopen-mr-link js-note-target-reopen", title: "Reopen merge request"
+
+.row
+  .col-md-9
+    = render "projects/merge_requests/show/participants"
+    = render "projects/notes/notes_with_form"
+  .col-md-3.hidden-sm.hidden-xs
+    .clearfix
+      %span.slead.has_tooltip{:"data-original-title" => 'Cross-project reference'}
+        = cross_project_reference(@project, @merge_request)
+    %hr
+    .votes-holder.hidden-sm.hidden-xs
+      %h6 Votes
+      #votes= render 'votes/votes_block', votable: @merge_request
+    %hr
+    .context
+      %cite.cgray
+        = render partial: 'projects/merge_requests/show/context', locals: { merge_request: @merge_request }
+
+    - if @merge_request.labels.any?
+      %hr
+      %h6 Labels
+      .merge-request-show-labels
+        - @merge_request.labels.each do |label|
+          = link_to project_merge_requests_path(@project, label_name: label.name) do
+            %p= render_colored_label(label)

app/views/projects/merge_requests/_show.html.haml

 .merge-request
   = render "projects/merge_requests/show/mr_title"
   %hr
-  .row
-    .col-sm-9
-      = render "projects/merge_requests/show/how_to_merge"
-      = render "projects/merge_requests/show/mr_box"
-      %hr
-      .append-bottom-20
-        %p.slead
-          %span From
-          - if @merge_request.for_fork?
-            %strong.label-branch<
-              - if @merge_request.source_project
-                = link_to @merge_request.source_project_namespace, project_path(@merge_request.source_project)
-              - else
-                \ #{@merge_request.source_project_namespace}
-              \:#{@merge_request.source_branch}
-            %span into
-            %strong.label-branch #{@merge_request.target_project_namespace}:#{@merge_request.target_branch}
+  = render "projects/merge_requests/show/mr_box"
+  %hr
+  .append-bottom-20
+    .slead
+      %span From
+      - if @merge_request.for_fork?
+        %strong.label-branch<
+          - if @merge_request.source_project
+            = link_to @merge_request.source_project_namespace, project_path(@merge_request.source_project)
           - else
-            %strong.label-branch #{@merge_request.source_branch}
-            %span into
-            %strong.label-branch #{@merge_request.target_branch}
-      = render "projects/merge_requests/show/state_widget"
-      = render "projects/merge_requests/show/commits"
-      = render "projects/merge_requests/show/participants"
-    .col-sm-3
-      .issue-btn-group
-        - if can?(current_user, :modify_merge_request, @merge_request)
-          - if @merge_request.open?
-            .btn-group-justified.append-bottom-20
-              .btn-group
-                %a.btn.dropdown-toggle{ data: {toggle: :dropdown} }
-                  %i.fa.fa-download
-                  Download as
-                  %span.caret
-                %ul.dropdown-menu
-                  %li= link_to "Email Patches", project_merge_request_path(@project, @merge_request, format: :patch)
-                  %li= link_to "Plain Diff",    project_merge_request_path(@project, @merge_request, format: :diff)
-            = link_to 'Close', project_merge_request_path(@project, @merge_request, merge_request: { state_event: :close }), method: :put, class: "btn btn-block btn-close", title: "Close merge request"
-            = link_to edit_project_merge_request_path(@project, @merge_request), class: "btn btn-block issuable-edit", id: "edit_merge_request" do
-              %i.fa.fa-pencil-square-o
-              Edit
-          - if @merge_request.closed?
-            = link_to 'Reopen', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-block btn-reopen reopen-mr-link", title: "Close merge request"
-      .clearfix
-        %span.slead.has_tooltip{:"data-original-title" => 'Cross-project reference'}
-          = cross_project_reference(@project, @merge_request)
-      %hr
-      .votes-holder.hidden-sm.hidden-xs
-        %h6 Votes
-        #votes= render 'votes/votes_block', votable: @merge_request
-      %hr
-      .context
-        %cite.cgray
-          = render partial: 'projects/merge_requests/show/context', locals: { merge_request: @merge_request }
+            \ #{@merge_request.source_project_namespace}
+          \:#{@merge_request.source_branch}
+        %span into
+        %strong.label-branch #{@merge_request.target_project_namespace}:#{@merge_request.target_branch}
+      - else
+        %strong.label-branch #{@merge_request.source_branch}
+        %span into
+        %strong.label-branch #{@merge_request.target_branch}
+      - if @merge_request.open?
+        %span.pull-right
+          .btn-group
+            %a.btn.dropdown-toggle{ data: {toggle: :dropdown} }
+              %i.fa.fa-download
+              Download as
+              %span.caret
+            %ul.dropdown-menu
+              %li= link_to "Email Patches", project_merge_request_path(@project, @merge_request, format: :patch)
+              %li= link_to "Plain Diff",    project_merge_request_path(@project, @merge_request, format: :diff)
+  = render "projects/merge_requests/show/how_to_merge"
+  = render "projects/merge_requests/show/state_widget"
   - if @commits.present?
     %ul.nav.nav-tabs.merge-request-tabs
       %li.notes-tab{data: {action: 'notes'}}
         = link_to project_merge_request_path(@project, @merge_request) do
-          %i.fa.fa-comment
+          %i.fa.fa-comments
           Discussion
           %span.badge= @merge_request.mr_and_commit_notes.count
+      %li.commits-tab{data: {action: 'commits'}}
+        = link_to project_merge_request_path(@project, @merge_request), title: 'Commits' do
+          %i.fa.fa-database
+          Commits
+          %span.badge= @commits.size
       %li.diffs-tab{data: {action: 'diffs'}}
         = link_to diffs_project_merge_request_path(@project, @merge_request) do
           %i.fa.fa-list-alt
           Changes
           %span.badge= @merge_request.diffs.size
-  - content_for :note_actions do
-    - if can?(current_user, :modify_merge_request, @merge_request)
-      - if @merge_request.open?
-        = link_to 'Close', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :close }), method: :put, class: "btn btn-grouped btn-close close-mr-link js-note-target-close", title: "Close merge request"
-      - if @merge_request.closed?
-        = link_to 'Reopen', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-grouped btn-reopen reopen-mr-link js-note-target-reopen", title: "Reopen merge request"
-
+  .notes.tab-content.voting_notes#notes{ class: (controller.action_name == 'show') ? "" : "hide" }
+    = render "projects/merge_requests/discussion"
+  .commits.tab-content
+    = render "projects/merge_requests/show/commits"
   .diffs.tab-content
     - if current_page?(action: 'diffs')
       = render "projects/merge_requests/show/diffs"
-  .notes.tab-content.voting_notes#notes{ class: (controller.action_name == 'show') ? "" : "hide" }
-    .row
-      .col-sm-9
-        = render "projects/notes/notes_with_form"
+
   .mr-loading-status
     = spinner

app/views/projects/merge_requests/show/_mr_title.html.haml

-%h3.page-title
+%h4.page-title
   .issue-box{ class: issue_box_class(@merge_request) }
     - if @merge_request.merged?
       Merged
@@ -7,5 +7,16 @@
     - else
       Open
   = "Merge Request ##{@merge_request.iid}"
-  .pull-right.creator
-    %small Created by #{link_to_member(@project, @merge_request.author)} #{time_ago_with_tooltip(@merge_request.created_at)}
+  %small.creator
+    ·
+    created by #{link_to_member(@project, @merge_request.author)} #{time_ago_with_tooltip(@merge_request.created_at)}
+
+  .issue-btn-group.pull-right
+    - if can?(current_user, :modify_merge_request, @merge_request)
+      - if @merge_request.open?
+        = link_to 'Close', project_merge_request_path(@project, @merge_request, merge_request: { state_event: :close }), method: :put, class: "btn btn-grouped btn-close", title: "Close merge request"
+        = link_to edit_project_merge_request_path(@project, @merge_request), class: "btn btn-grouped issuable-edit", id: "edit_merge_request" do
+          %i.fa.fa-pencil-square-o
+          Edit
+      - if @merge_request.closed?
+        = link_to 'Reopen', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-grouped btn-reopen reopen-mr-link", title: "Close merge request"

app/views/projects/merge_requests/show/_participants.html.haml

@@ -2,8 +2,3 @@
   %cite.cgray #{@merge_request.participants.count} participants
   - @merge_request.participants.each do |participant|
     = link_to_member(@project, participant, name: false, size: 24)
-
-  .merge-request-show-labels.pull-right
-    - @merge_request.labels.each do |label|
-      = link_to project_merge_requests_path(@project, label_name: label.name) do
-        = render_colored_label(label)

features/steps/project/commits/commits.rb

@@ -78,14 +78,14 @@ class Spinach::Features::ProjectCommits < Spinach::FeatureSteps
   end
   step 'I click side-by-side diff button' do
-    click_link "Side-by-side Diff"
+    click_link "Side-by-side"
   end
   step 'I see side-by-side diff button' do
-    page.should have_content "Side-by-side Diff"
+    page.should have_content "Side-by-side"
   end
   step 'I see inline diff button' do
-    page.should have_content "Inline Diff"
+    page.should have_content "Inline"
   end
 end

features/steps/project/merge_requests.rb

@@ -109,6 +109,10 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
   end
   step 'I click on the commit in the merge request' do
+    within '.merge-request-tabs' do
+      click_link 'Commits'
+    end
+
     within '.mr-commits' do
       click_link Commit.truncate_sha(sample_commit.id)
     end
@@ -261,7 +265,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
   end
   step 'I click Side-by-side Diff tab' do
-    click_link 'Side-by-side Diff'
+    click_link 'Side-by-side'
   end
   step 'I should see comments on the side-by-side diff page' do