Merge branch 'restyle/issue' into 'master'

Improve UI for Issue#show, MR#show pages

11 files changed, 112 insertions, 88 deletions

diff --git a/app/assets/stylesheets/generic/common.scss b/app/assets/stylesheets/generic/common.scss
index 91618688081..7afa74400c9 100644
--- a/app/assets/stylesheets/generic/common.scss
+++ b/app/assets/stylesheets/generic/common.scss
@@ -88,11 +88,15 @@ pre.well-pre {
 /** Big Labels **/
 .state-label {
   font-size: 14px;
-  padding: 6px 25px;
+  padding: 9px 25px;
   text-align: center;
-  @include border-radius(4px);
   text-shadow: none;
-  margin-left: 10px;
+  margin-right: 20px;
+
+  &.state-label-blue {
+    background: #31708f;
+    color: #FFF;
+  }
 
   &.state-label-green {
     background: #4A4;
diff --git a/app/assets/stylesheets/generic/issue_box.scss b/app/assets/stylesheets/generic/issue_box.scss
index 033b4b20f5b..0e0a365a82d 100644
--- a/app/assets/stylesheets/generic/issue_box.scss
+++ b/app/assets/stylesheets/generic/issue_box.scss
@@ -17,26 +17,33 @@
     margin-bottom: 0;
   }
 
+  .state {
+    height: 34px;
+    border-bottom: 1px solid #DDD;
+    line-height: 32px;
+  }
+
   .title {
     font-size: 22px;
     font-weight: 500;
     line-height: 1.5;
     margin: 0;
     color: #333;
+    padding-bottom: 0;
+    padding: 15px 25px;
   }
 
   .context {
     border: none;
     border-top: 1px solid #eee;
+    padding: 15px 25px;
   }
 
   .description {
-    border-top: 1px solid #eee;
+    padding: 0 25px 15px 25px;
   }
 
   .title, .context, .description {
-    padding: 15px 25px;
-
     .clearfix {
       margin: 0;
     }
diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index 16981edd980..0374992ebfc 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -84,4 +84,12 @@ module IssuesHelper
   def milestone_options object
     options_from_collection_for_select(@project.milestones.active, 'id', 'title', object.milestone_id)
   end
+
+  def issue_alert_class(issue)
+    if issue.closed?
+      'alert-danger'
+    else
+      'alert-success'
+    end
+  end
 end
diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
index 5e3f82fe9ce..62f061bb079 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -41,4 +41,14 @@ module MergeRequestsHelper
       "Branches: #{@merge_request.source_branch} #{separator} #{@merge_request.target_branch}"
     end
   end
+
+  def merge_request_alert_class(merge_request)
+    if merge_request.merged?
+      'alert-info'
+    elsif merge_request.closed?
+      'alert-danger'
+    else
+      'alert-success'
+    end
+  end
 end
diff --git a/app/views/projects/issues/_issue_context.html.haml b/app/views/projects/issues/_issue_context.html.haml
index e5418c25b23..aae101cf40f 100644
--- a/app/views/projects/issues/_issue_context.html.haml
+++ b/app/views/projects/issues/_issue_context.html.haml
@@ -1,23 +1,22 @@
 = form_for [@project, @issue], remote: true, html: {class: 'edit-issue inline-update'} do |f|
-  Created by #{link_to_member(@project, issue.author)} 
-  - if issue.assignee
-    \ and currently assigned to
+  %strong.append-right-10
+    Assignee:
 
   - if can?(current_user, :modify_issue, @issue)
     = project_users_select_tag('issue[assignee_id]', placeholder: 'Select assignee', class: 'custom-form-control', selected: @issue.assignee_id)
   - elsif issue.assignee
     = link_to_member(@project, @issue.assignee)
+  - else
+    None
 
-
-  .pull-right.hidden-sm.hidden-xs
-    - if issue.milestone
-      - milestone = issue.milestone
-      %cite.cgray Attached to milestone
-
+  .pull-right
+    %strong.append-right-10
+      Milestone:
     - if can?(current_user, :modify_issue, @issue)
       = f.select(:milestone_id, milestone_options(@issue), { include_blank: "Select milestone (none):" }, {class: 'select2 select2-compact'})
-
       = hidden_field_tag :issue_context
       = f.submit class: 'btn'
     - elsif issue.milestone
       = link_to issue.milestone.title, project_milestone_path
+    - else
+      None
diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml
index c1bd1147912..7aa37eda965 100644
--- a/app/views/projects/issues/show.html.haml
+++ b/app/views/projects/issues/show.html.haml
@@ -1,14 +1,6 @@
 %h3.page-title
   Issue ##{@issue.iid}
 
-  %small
-    created #{time_ago_with_tooltip(@issue.created_at)}
-
-  - if @issue.closed?
-    %span.state-label.state-label-red Closed
-  - else
-    %span.state-label.state-label-green Open
-
   %span.pull-right
     - if can?(current_user, :write_issue, @project)
       = link_to new_project_issue_path(@project), class: "btn grouped", title: "New Issue", id: "new_issue_link" do
@@ -38,18 +30,27 @@
         = @issue.milestone.title
 
 .issue-box
+  .state{ class: issue_alert_class(@issue) }
+    - if @issue.closed?
+      %span.state-label.state-label-red Closed
+    - else
+      %span.state-label.state-label-green Open
+
+    %span.creator
+      Created by #{link_to_member(@project, @issue.author)} #{time_ago_with_tooltip(@issue.created_at)}
+
   %h4.title
     = gfm escape_once(@issue.title)
 
-  .context
-    %cite.cgray
-      = render partial: 'issue_context', locals: { issue: @issue }
-
   - if @issue.description.present?
     .description
       .wiki
         = preserve do
           = markdown @issue.description
+  .context
+    %cite.cgray
+      = render partial: 'issue_context', locals: { issue: @issue }
+
 
 - content_for :note_actions do
   - if can?(current_user, :modify_issue, @issue)
diff --git a/app/views/projects/merge_requests/show/_context.html.haml b/app/views/projects/merge_requests/show/_context.html.haml
index 705eeb9894a..2bd850426a9 100644
--- a/app/views/projects/merge_requests/show/_context.html.haml
+++ b/app/views/projects/merge_requests/show/_context.html.haml
@@ -1,23 +1,22 @@
 = form_for [@project, @merge_request], remote: true, html: {class: 'edit-merge_request inline-update'} do |f|
-  Created by #{link_to_member(@project, merge_request.author)} 
-  - if merge_request.assignee
-    \ and currently assigned to
+  %strong.append-right-10
+    Assignee:
 
   - if can?(current_user, :modify_merge_request, @merge_request)
     = project_users_select_tag('merge_request[assignee_id]', placeholder: 'Select assignee', class: 'custom-form-control', selected: @merge_request.assignee_id)
   - elsif merge_request.assignee
     = link_to_member(@project, @merge_request.assignee)
+  - else
+    None
 
-
-  .pull-right.hidden-sm.hidden-xs
-    - if merge_request.milestone
-      - milestone = merge_request.milestone
-      %cite.cgray Attached to milestone
-
+  .pull-right
+    %strong.append-right-10
+      Milestone:
     - if can?(current_user, :modify_merge_request, @merge_request)
       = f.select(:milestone_id, milestone_options(@merge_request), { include_blank: "Select milestone (none):" }, {class: 'select2 select2-compact'})
-
       = hidden_field_tag :merge_request_context
       = f.submit class: 'btn'
     - elsif merge_request.milestone
       = link_to merge_request.milestone.title, project_milestone_path
+    - else
+      None
diff --git a/app/views/projects/merge_requests/show/_mr_box.html.haml b/app/views/projects/merge_requests/show/_mr_box.html.haml
index 7c3f9b93444..803e00292b0 100644
--- a/app/views/projects/merge_requests/show/_mr_box.html.haml
+++ b/app/views/projects/merge_requests/show/_mr_box.html.haml
@@ -1,33 +1,46 @@
 .issue-box
+  .state{ class: merge_request_alert_class(@merge_request) }
+    - if @merge_request.merged?
+      %span.state-label.state-label-blue
+        Merged
+    - elsif @merge_request.closed?
+      %span.state-label.state-label-red
+        Closed
+    - else
+      %span.state-label.state-label-green
+        Open
+    %span.creator
+      Created by #{link_to_member(@project, @merge_request.author)} #{time_ago_with_tooltip(@merge_request.created_at)}
+
   %h4.title
     = gfm escape_once(@merge_request.title)
 
-  .context
-    %cite.cgray
-      = render partial: 'projects/merge_requests/show/context', locals: { merge_request: @merge_request }
-
   - if @merge_request.description.present?
     .description
       .wiki
         = preserve do
           = markdown @merge_request.description
 
-  - if @merge_request.closed?
-    .description.alert-danger
-      %span
-        %i.icon-remove
-        Closed by #{link_to_member(@project, @merge_request.closed_event.author)}
-        #{time_ago_with_tooltip(@merge_request.closed_event.created_at)}.
-  - if @merge_request.merged?
-    .description.alert-success
-      %span
-        %i.icon-ok
-        Merged by #{link_to_member(@project, @merge_request.merge_event.author)}
-        #{time_ago_with_tooltip(@merge_request.merge_event.created_at)}.
-  - if !@closes_issues.empty? && @merge_request.opened?
-    .description.alert-info
-      %span
-        %i.icon-ok
-        Accepting this merge request will close #{@closes_issues.size == 1 ? 'issue' : 'issues'}
-        = succeed '.' do
-          != gfm(@closes_issues.map { |i| "##{i.iid}" }.to_sentence)
+  .context
+    %cite.cgray
+      = render partial: 'projects/merge_requests/show/context', locals: { merge_request: @merge_request }
+
+- if @merge_request.closed?
+  .alert.alert-info
+    %span
+      %i.icon-remove
+      Closed by #{link_to_member(@project, @merge_request.closed_event.author)}
+      #{time_ago_with_tooltip(@merge_request.closed_event.created_at)}.
+- if @merge_request.merged?
+  .alert.alert-info
+    %span
+      %i.icon-ok
+      Merged by #{link_to_member(@project, @merge_request.merge_event.author)}
+      #{time_ago_with_tooltip(@merge_request.merge_event.created_at)}.
+- if !@closes_issues.empty? && @merge_request.opened?
+  .alert.alert-info.alert-info
+    %span
+      %i.icon-ok
+      Accepting this merge request will close #{@closes_issues.size == 1 ? 'issue' : 'issues'}
+      = succeed '.' do
+        != gfm(@closes_issues.map { |i| "##{i.iid}" }.to_sentence)
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 7540c8a640f..b27522e46a4 100644
--- a/app/views/projects/merge_requests/show/_mr_title.html.haml
+++ b/app/views/projects/merge_requests/show/_mr_title.html.haml
@@ -1,20 +1,5 @@
 %h3.page-title
   = "Merge Request ##{@merge_request.iid}"
-  %small
-    created #{time_ago_with_tooltip(@merge_request.created_at)}
-
-  - if @merge_request.merged?
-    %span.state-label.state-label-green
-      %i.icon-ok
-      Merged
-  - elsif @merge_request.closed?
-    %span.state-label.state-label-red
-      Closed
-  - else
-    %span.state-label.state-label-green
-      Open
-
-
 
   %span.pull-right
     - if can?(current_user, :modify_merge_request, @merge_request)
diff --git a/features/steps/project/project_merge_requests.rb b/features/steps/project/project_merge_requests.rb
index cc2648c6204..adf9e77e136 100644
--- a/features/steps/project/project_merge_requests.rb
+++ b/features/steps/project/project_merge_requests.rb
@@ -55,7 +55,9 @@ class ProjectMergeRequests < Spinach::FeatureSteps
   end
 
   step 'I click link "Close"' do
-    click_link "Close"
+    within '.page-title' do
+      click_link "Close"
+    end
   end
 
   step 'I submit new merge request "Wiki Feature"' do
@@ -163,7 +165,7 @@ class ProjectMergeRequests < Spinach::FeatureSteps
   end
 
   step 'I should see merged request' do
-    within '.page-title' do
+    within '.issue-box' do
       page.should have_content "Merged"
     end
   end
diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb
index 834ed73549b..ffe6d02d2f2 100644
--- a/spec/features/issues_spec.rb
+++ b/spec/features/issues_spec.rb
@@ -186,7 +186,7 @@ describe "Issues" do
         find('.edit-issue.inline-update #issue_assignee_id').set project.team.members.first.id
         click_button 'Update Issue'
 
-        page.should have_content "currently assigned to"
+        page.should have_content "Assignee:"
         page.has_select?('issue_assignee_id', :selected => project.team.members.first.name)
       end
     end
@@ -206,11 +206,9 @@ describe "Issues" do
         login_with guest
 
         visit project_issue_path(project, issue)
-        page.should have_content "currently assigned to #{issue.assignee.name}"
-
+        page.should have_content issue.assignee.name
       end
     end
-
   end
 
   describe 'update milestone from issue#show' do
@@ -225,17 +223,16 @@ describe "Issues" do
         find('.edit-issue.inline-update').select(milestone.title, from: 'issue_milestone_id')
         click_button 'Update Issue'
 
-        page.should have_content "Attached to milestone"
+        page.should have_content "Milestone"
         page.has_select?('issue_assignee_id', :selected => milestone.title)
       end
     end
 
     context 'by unauthorized user' do
-
       let(:guest) { create(:user) }
 
       before :each do
-        project.team << [[guest], :guest]
+        project.team << [guest, :guest]
         issue.milestone = milestone
         issue.save
       end
@@ -245,8 +242,7 @@ describe "Issues" do
         login_with guest
 
         visit project_issue_path(project, issue)
-
-        page.should have_content "Attached to milestone #{milestone.title}"
+        page.should have_content milestone.title
       end
     end
   end
@@ -258,4 +254,4 @@ describe "Issues" do
   def last_issue
     all("ul.issues-list li").last.text
   end
-end
\ No newline at end of file
+end