Merge remote-tracking branch 'origin/ui/issuable-form'

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

18 files changed, 95 insertions, 81 deletions

diff --git a/app/assets/stylesheets/framework/selects.scss b/app/assets/stylesheets/framework/selects.scss
index 5e5ae3d0af8..c01e1e32e41 100644
--- a/app/assets/stylesheets/framework/selects.scss
+++ b/app/assets/stylesheets/framework/selects.scss
@@ -59,17 +59,38 @@
   color: #313236;
 }
 
+.select2-container-multi {
+  .select2-choices {
+    @include border-radius(2px);
+    border-color: $input-border;
+    background: white;
+    padding-left: $gl-padding / 2;
+
+    .select2-search-field input {
+      padding: $gl-padding / 2;
+      font-size: 13px;
+      height: auto;
+      font-family: inherit;
+      font-size: inherit;
+    }
 
-.select2-container-multi .select2-choices {
-  @include border-radius(2px);
-  border-color: #CCC;
-}
+    .select2-search-choice {
+      margin: 8px 0 0 8px;
+      background: white;
+      box-shadow: none;
+      border-color: $input-border;
+      color: $gl-text-color;
+      line-height: 15px;
 
-.select2-container-multi .select2-choices .select2-search-field input {
-  padding: 8px 14px;
-  font-size: 13px;
-  line-height: 18px;
-  height: auto;
+      .select2-search-choice-close {
+        top: 5px;
+      }
+
+      &.select2-search-choice-focus {
+        border-color: $gl-text-color;
+      }
+    }
+  }
 }
 
 .select2-drop-active {
diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss
index 9313fa4795a..957da5c182e 100644
--- a/app/assets/stylesheets/pages/issuable.scss
+++ b/app/assets/stylesheets/pages/issuable.scss
@@ -90,6 +90,17 @@
   }
 }
 
+.issuable-show-labels {
+  a {
+    margin-right: 5px;
+    margin-bottom: 5px;
+    display: inline-block;
+    .color-label {
+      padding: 6px 10px;
+    }
+  }
+}
+
 .cross-project-reference {
   text-align: center;
   width: 100%;
diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss
index 41c069f0ad3..f5548c5b88b 100644
--- a/app/assets/stylesheets/pages/issues.scss
+++ b/app/assets/stylesheets/pages/issues.scss
@@ -56,17 +56,6 @@
   }
 }
 
-.issue-show-labels {
-  a {
-    margin-right: 5px;
-    margin-bottom: 5px;
-    display: inline-block;
-    .color-label {
-      padding: 6px 10px;
-    }
-  }
-}
-
 form.edit-issue {
   margin: 0;
 }
diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index d382fb59613..f21ad694d06 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -179,17 +179,6 @@
   display: none;
 }
 
-.merge-request-show-labels {
-  a {
-    margin-right: 5px;
-    margin-bottom: 5px;
-    display: inline-block;
-    .color-label {
-      padding: 6px 10px;
-    }
-  }
-}
-
 .merge-request-form .select2-container {
   width: 250px !important;
 }
diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index da2e0db0653..931d52055f8 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -51,8 +51,10 @@ module IssuesHelper
   end
 
   def milestone_options(object)
-    options_from_collection_for_select(object.project.milestones.active,
-                                       'id', 'title', object.milestone_id)
+    milestones = object.project.milestones.active.to_a
+    milestones.unshift(Milestone::None)
+
+    options_from_collection_for_select(milestones, 'id', 'title', object.milestone_id)
   end
 
   def issue_box_class(item)
diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
index b804d4f4e3b..7f3a61a5e38 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -49,18 +49,21 @@ module MergeRequestsHelper
         source_project_id: @merge_request.source_project_id,
         target_project_id: @merge_request.target_project_id,
         source_branch: @merge_request.source_branch,
-        target_branch: nil
-      }
+        target_branch: @merge_request.target_branch,
+      },
+      change_branches: true
     )
   end
 
   def source_branch_with_namespace(merge_request)
+    branch = link_to(merge_request.source_branch, namespace_project_commits_path(merge_request.source_project.namespace, merge_request.source_project, merge_request.source_branch))
+
     if merge_request.for_fork?
       namespace = link_to(merge_request.source_project_namespace,
         project_path(merge_request.source_project))
-      namespace + ":#{merge_request.source_branch}"
+      namespace + ":" + branch
     else
-      merge_request.source_branch
+      branch
     end
   end
 
diff --git a/app/views/projects/issues/_discussion.html.haml b/app/views/projects/issues/_discussion.html.haml
index 8f0a1ed9be2..b5f522f2079 100644
--- a/app/views/projects/issues/_discussion.html.haml
+++ b/app/views/projects/issues/_discussion.html.haml
@@ -29,10 +29,3 @@
     .issuable-affix
       .context
         = render 'shared/issuable/context', issuable: @issue
-
-      - if @issue.labels.any?
-        .issuable-context-title
-          %label Labels
-        .issue-show-labels
-          - @issue.labels.each do |label|
-            = link_to_label(label)
diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml
index f01bf2505da..e2de17cee6d 100644
--- a/app/views/projects/issues/show.html.haml
+++ b/app/views/projects/issues/show.html.haml
@@ -11,7 +11,8 @@
           Open
       %span.issue-id Issue ##{@issue.iid}
       %span.creator
-        · created by #{link_to_member(@project, @issue.author, size: 24)}
+        ·
+        opened by #{link_to_member(@project, @issue.author, size: 24)}
         ·
         = time_ago_with_tooltip(@issue.created_at, placement: 'bottom', html_class: 'issue_created_ago')
         - if @issue.updated_at != @issue.created_at
diff --git a/app/views/projects/merge_requests/_discussion.html.haml b/app/views/projects/merge_requests/_discussion.html.haml
index 4a192aeb2cd..ea462561668 100644
--- a/app/views/projects/merge_requests/_discussion.html.haml
+++ b/app/views/projects/merge_requests/_discussion.html.haml
@@ -26,10 +26,3 @@
     .issuable-affix
       .context
         = render 'shared/issuable/context', issuable: @merge_request
-
-      - if @merge_request.labels.any?
-        .issuable-context-title
-          %label Labels
-        .merge-request-show-labels
-          - @merge_request.labels.each do |label|
-            = link_to_label(label)
diff --git a/app/views/projects/merge_requests/_new_compare.html.haml b/app/views/projects/merge_requests/_new_compare.html.haml
index f57ce9b4a6c..236a545c840 100644
--- a/app/views/projects/merge_requests/_new_compare.html.haml
+++ b/app/views/projects/merge_requests/_new_compare.html.haml
@@ -11,7 +11,7 @@
         .panel-body
           = f.select(:source_project_id, [[@merge_request.source_project_path,@merge_request.source_project.id]] , {}, { class: 'source_project select2 span3', disabled: @merge_request.persisted?, required: true })
            
-          = f.select(:source_branch, @merge_request.source_branches, { include_blank: "Select branch" }, {class: 'source_branch select2 span2', required: true})
+          = f.select(:source_branch, @merge_request.source_branches, { include_blank: true }, { class: 'source_branch select2 span2', required: true, data: { placeholder: "Select source branch" } })
         .panel-footer
           .mr_source_commit
 
@@ -23,7 +23,7 @@
           - projects =  @project.forked_from_project.nil? ? [@project] : [@project, @project.forked_from_project]
           = f.select(:target_project_id, options_from_collection_for_select(projects, 'id', 'path_with_namespace', f.object.target_project_id), {}, { class: 'target_project select2 span3', disabled: @merge_request.persisted?, required: true })
            
-          = f.select(:target_branch, @merge_request.target_branches, { include_blank: "Select branch" }, {class: 'target_branch select2 span2', required: true})
+          = f.select(:target_branch, @merge_request.target_branches, { include_blank: true }, { class: 'target_branch select2 span2', required: true, data: { placeholder: "Select target branch" } })
         .panel-footer
           .mr_target_commit
 
diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml
index e7eb0066594..f5aff0877e7 100644
--- a/app/views/projects/merge_requests/_show.html.haml
+++ b/app/views/projects/merge_requests/_show.html.haml
@@ -26,15 +26,17 @@
               %li= link_to "Plain Diff",    merge_request_path(@merge_request, format: :diff)
       .normal
         %span Request to merge
-        %span.label-branch #{source_branch_with_namespace(@merge_request)}
+        %span.label-branch
+          = source_branch_with_namespace(@merge_request)
         %span into
-        %span.label-branch #{@merge_request.target_branch}
+        = link_to namespace_project_commits_path(@project.namespace, @project, @merge_request.target_branch), class: "label-branch" do
+          = @merge_request.target_branch
 
     = render "projects/merge_requests/show/how_to_merge"
     = render "projects/merge_requests/widget/show.html.haml"
 
-    - if @merge_request.open? && @merge_request.can_be_merged?
-      .light.append-bottom-20
+    - if @merge_request.open? && @merge_request.source_branch_exists? && @merge_request.can_be_merged? && @merge_request.can_be_merged_by?(current_user)
+      .light.prepend-top-default
         You can also accept this merge request manually using the
         = succeed '.' do
           = link_to "command line", "#modal_merge_info", class: "how_to_merge_link vlink", title: "How To Merge", "data-toggle" => "modal"
diff --git a/app/views/projects/merge_requests/new.html.haml b/app/views/projects/merge_requests/new.html.haml
index 9fdde80c6d9..d259968030e 100644
--- a/app/views/projects/merge_requests/new.html.haml
+++ b/app/views/projects/merge_requests/new.html.haml
@@ -1,7 +1,7 @@
 - page_title "New Merge Request"
 = render "header_title"
 
-- if @merge_request.can_be_created
+- if @merge_request.can_be_created && !params[:change_branches]
   = render 'new_submit'
 - else
   = render 'new_compare'
diff --git a/app/views/projects/merge_requests/show/_mr_title.html.haml b/app/views/projects/merge_requests/show/_mr_title.html.haml
index 2bf9cd597a4..4dfe46e2b86 100644
--- a/app/views/projects/merge_requests/show/_mr_title.html.haml
+++ b/app/views/projects/merge_requests/show/_mr_title.html.haml
@@ -4,7 +4,7 @@
   %span.issue-id Merge Request ##{@merge_request.iid}
   %span.creator
     ·
-    created by #{link_to_member(@project, @merge_request.author, size: 24)}
+    opened by #{link_to_member(@project, @merge_request.author, size: 24)}
     ·
     = time_ago_with_tooltip(@merge_request.created_at)
     - if @merge_request.updated_at != @merge_request.created_at
diff --git a/app/views/projects/merge_requests/widget/_merged.html.haml b/app/views/projects/merge_requests/widget/_merged.html.haml
index a788fcea23f..ac08e0b498a 100644
--- a/app/views/projects/merge_requests/widget/_merged.html.haml
+++ b/app/views/projects/merge_requests/widget/_merged.html.haml
@@ -10,7 +10,8 @@
       - if !@merge_request.source_branch_exists?
         = succeed '.' do
           The changes were merged into
-          %span.label-branch= @merge_request.target_branch
+          = link_to namespace_project_commits_path(@project.namespace, @project, @merge_request.target_branch), class: "label-branch" do
+            = @merge_request.target_branch
         The source branch has been removed.
 
       - elsif can_remove_branch?(@merge_request.source_project, @merge_request.source_branch)
@@ -18,7 +19,8 @@
           %p
             = succeed '.' do
               The changes were merged into
-              %span.label-branch= @merge_request.target_branch
+              = link_to namespace_project_commits_path(@project.namespace, @project, @merge_request.target_branch), class: "label-branch" do
+                = @merge_request.target_branch
             You can remove the source branch now.
           = link_to namespace_project_branch_path(@merge_request.source_project.namespace, @merge_request.source_project, @merge_request.source_branch), remote: true, method: :delete, class: "btn btn-primary btn-sm remove_source_branch" do
             %i.fa.fa-times
diff --git a/app/views/shared/issuable/_context.html.haml b/app/views/shared/issuable/_context.html.haml
index f1646b4ce64..f44b439a843 100644
--- a/app/views/shared/issuable/_context.html.haml
+++ b/app/views/shared/issuable/_context.html.haml
@@ -25,25 +25,32 @@
         none
     .issuable-context-selectbox
       - if can?(current_user, :"admin_#{issuable.to_ability_name}", @project)
-        = f.select(:milestone_id, milestone_options(issuable), { include_blank: 'Select milestone' }, {class: 'select2 select2-compact js-select2 js-milestone'})
+        = f.select(:milestone_id, milestone_options(issuable), { include_blank: true }, { class: 'select2 select2-compact js-select2 js-milestone', data: { placeholder: 'Select milestone' }})
         = hidden_field_tag :issuable_context
         = f.submit class: 'btn hide'
 
+  - if issuable.labels.any?
+    %div.prepend-top-default.clearfix
+      .issuable-context-title
+        %label Labels
+      .issuable-show-labels
+        - issuable.labels.each do |label|
+          = link_to_label(label)
+
   - if current_user
     - subscribed = issuable.subscribed?(current_user)
     %div.prepend-top-default.clearfix
       .issuable-context-title
-        %label
-          Subscription:
-      %button.btn.btn-block.subscribe-button{:type => 'button'}
-        = icon('eye')
-        %span= subscribed ? 'Unsubscribe' : 'Subscribe'
+        %label Subscription
       - subscribtion_status = subscribed ? 'subscribed' : 'unsubscribed'
       .subscription-status{data: {status: subscribtion_status}}
         .description-block.unsubscribed{class: ( 'hidden' if subscribed )}
           You're not receiving notifications from this thread.
         .description-block.subscribed{class: ( 'hidden' unless subscribed )}
           You're receiving notifications because you're subscribed to this thread.
+      %button.btn.btn-block.subscribe-button{:type => 'button'}
+        = icon('eye')
+        %span= subscribed ? 'Unsubscribe' : 'Subscribe'
 
 :javascript
   new Subscription("#{toggle_subscription_path(issuable)}");
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index 7793ae2449d..91ccd1ef660 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -36,9 +36,9 @@
       = f.label :assignee_id, "Assignee", class: 'control-label'
       .col-sm-10
         = users_select_tag("#{issuable.class.model_name.param_key}[assignee_id]",
-            placeholder: 'Select a user', class: 'custom-form-control', null_user: true,
+            placeholder: 'Select assignee', class: 'custom-form-control', null_user: true,
             selected: issuable.assignee_id, project: @target_project || @project,
-            first_user: true, current_user: true)
+            first_user: true, current_user: true, include_blank: true)
          
         = link_to 'Assign to me', '#', class: 'btn assign-to-me-link'
   .form-group
@@ -47,7 +47,7 @@
       .col-sm-10
         - if milestone_options(issuable).present?
           = f.select(:milestone_id, milestone_options(issuable),
-            { include_blank: 'Select milestone' }, { class: 'select2' })
+            { include_blank: true }, { class: 'select2', data: { placeholder: 'Select milestone' } })
         - else
           .prepend-top-10
           %span.light No open milestones available.
@@ -59,7 +59,7 @@
     .col-sm-10
       - if issuable.project.labels.any?
         = f.collection_select :label_ids, issuable.project.labels.all, :id, :name,
-          { selected: issuable.label_ids }, multiple: true, class: 'select2'
+          { selected: issuable.label_ids }, multiple: true, class: 'select2', data: { placeholder: "Select labels" }
       - else
         .prepend-top-10
         %span.light No labels yet.
@@ -77,22 +77,24 @@
   .form-group
     = f.label :target_branch, class: 'control-label'
     .col-sm-10
-      = f.select(:target_branch, @merge_request.target_branches, { include_blank: "Select branch" }, { class: 'target_branch select2 span2', disabled: @merge_request.new_record? })
+      = f.select(:target_branch, @merge_request.target_branches, { include_blank: true }, { class: 'target_branch select2 span2', disabled: @merge_request.new_record?, data: {placeholder: "Select branch"} })
       - if @merge_request.new_record?
         %p.help-block
         = link_to 'Change branches', mr_change_branches_path(@merge_request)
 
 - is_footer = !(issuable.is_a?(MergeRequest) && issuable.new_record?)
 .gray-content-block{class: (is_footer ? "footer-block" : "middle-block")}
-  - if !issuable.project.empty_repo? && (guide_url = contribution_guide_path(issuable.project)) && !issuable.persisted?
-    %p
-      Please review the
-      %strong #{link_to 'guidelines for contribution', guide_url}
-      to this repository.
   - if issuable.new_record?
     = f.submit "Submit #{issuable.class.model_name.human.downcase}", class: 'btn btn-create'
   - else
     = f.submit 'Save changes', class: 'btn btn-save'
+
+  - if !issuable.persisted? && !issuable.project.empty_repo? && (guide_url = contribution_guide_path(issuable.project))
+    .inline.prepend-left-10
+      Please review the
+      %strong #{link_to 'contribution guidelines', guide_url}
+      for this project.
+
   - if issuable.new_record?
     - cancel_project = issuable.source_project
   - else
diff --git a/features/steps/project/forked_merge_requests.rb b/features/steps/project/forked_merge_requests.rb
index de1dbfdfa93..d3675060994 100644
--- a/features/steps/project/forked_merge_requests.rb
+++ b/features/steps/project/forked_merge_requests.rb
@@ -112,11 +112,10 @@ class Spinach::Features::ProjectForkedMergeRequests < Spinach::FeatureSteps
   end
 
   step 'I fill out an invalid "Merge Request On Forked Project" merge request' do
-    select "Select branch", from: "merge_request_target_branch"
     expect(find(:select, "merge_request_source_project_id", {}).value).to eq @forked_project.id.to_s
     expect(find(:select, "merge_request_target_project_id", {}).value).to eq @project.id.to_s
     expect(find(:select, "merge_request_source_branch", {}).value).to eq ""
-    expect(find(:select, "merge_request_target_branch", {}).value).to eq ""
+    expect(find(:select, "merge_request_target_branch", {}).value).to eq "master"
     click_button "Compare branches"
   end
 
diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb
index 536a62e7d14..a13044c3ae1 100644
--- a/features/steps/project/issues/issues.rb
+++ b/features/steps/project/issues/issues.rb
@@ -86,7 +86,7 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps
   end
 
   step 'I should see label \'bug\' with issue' do
-    page.within '.issue-show-labels' do
+    page.within '.issuable-show-labels' do
       expect(page).to have_content 'bug'
     end
   end