Merge branch 'restyle/issue' into 'master'

Improve UI for Issue#show, MR#show pages

Merge branch 'restyle/issue' into 'master'
Improve UI for Issue#show, MR#show pages
Dmitriy Zaporozhets
2 parents b09610b5 ededfd2d
Showing 11 changed files with 112 additions and 88 deletions Show diff stats
app/assets/stylesheets/generic/common.scss
app/assets/stylesheets/generic/issue_box.scss
app/helpers/issues_helper.rb
app/helpers/merge_requests_helper.rb
app/views/projects/issues/_issue_context.html.haml
app/views/projects/issues/show.html.haml
app/views/projects/merge_requests/show/_context.html.haml
app/views/projects/merge_requests/show/_mr_box.html.haml
app/views/projects/merge_requests/show/_mr_title.html.haml
features/steps/project/project_merge_requests.rb
spec/features/issues_spec.rb
@@ -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;
@@ -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;
     }
@@ -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
@@ -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
 = form_for [@project, @issue], remote: true, html: {class: 'edit-issue inline-update'} do |f|
-  Created by #{link_to_member(@project, issue.author)}&nbsp;
-  - 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
 %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)
 = 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)}&nbsp;
-  - 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
 .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)
 %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)
@@ -55,7 +55,9 @@ class ProjectMergeRequests &lt; 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 &lt; 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
@@ -186,7 +186,7 @@ describe &quot;Issues&quot; 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 &quot;Issues&quot; 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 &quot;Issues&quot; 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 &quot;Issues&quot; 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 &quot;Issues&quot; do
   def last_issue
     all("ul.issues-list li").last.text
   end
-end
 \ No newline at end of file
+end
	@@ -84,4 +84,12 @@ module IssuesHelper		@@ -84,4 +84,12 @@ module IssuesHelper
84	def milestone_options object	84	def milestone_options object
85	options_from_collection_for_select(@project.milestones.active, 'id', 'title', object.milestone_id)	85	options_from_collection_for_select(@project.milestones.active, 'id', 'title', object.milestone_id)
86	end	86	end
		87	+
		88	+ def issue_alert_class(issue)
		89	+ if issue.closed?
		90	+ 'alert-danger'
		91	+ else
		92	+ 'alert-success'
		93	+ end
		94	+ end
87	end	95	end
	@@ -41,4 +41,14 @@ module MergeRequestsHelper		@@ -41,4 +41,14 @@ module MergeRequestsHelper
41	"Branches: #{@merge_request.source_branch} #{separator} #{@merge_request.target_branch}"	41	"Branches: #{@merge_request.source_branch} #{separator} #{@merge_request.target_branch}"
42	end	42	end
43	end	43	end
		44	+
		45	+ def merge_request_alert_class(merge_request)
		46	+ if merge_request.merged?
		47	+ 'alert-info'
		48	+ elsif merge_request.closed?
		49	+ 'alert-danger'
		50	+ else
		51	+ 'alert-success'
		52	+ end
		53	+ end
44	end	54	end
1	.issue-box	1	.issue-box
		2	+ .state{ class: merge_request_alert_class(@merge_request) }
		3	+ - if @merge_request.merged?
		4	+ %span.state-label.state-label-blue
		5	+ Merged
		6	+ - elsif @merge_request.closed?
		7	+ %span.state-label.state-label-red
		8	+ Closed
		9	+ - else
		10	+ %span.state-label.state-label-green
		11	+ Open
		12	+ %span.creator
		13	+ Created by #{link_to_member(@project, @merge_request.author)} #{time_ago_with_tooltip(@merge_request.created_at)}
		14	+
2	%h4.title	15	%h4.title
3	= gfm escape_once(@merge_request.title)	16	= gfm escape_once(@merge_request.title)
4		17
5	- .context
6	- %cite.cgray
7	- = render partial: 'projects/merge_requests/show/context', locals: { merge_request: @merge_request }
8	-
9	- if @merge_request.description.present?	18	- if @merge_request.description.present?
10	.description	19	.description
11	.wiki	20	.wiki
12	= preserve do	21	= preserve do
13	= markdown @merge_request.description	22	= markdown @merge_request.description
14		23
15	- - if @merge_request.closed?
16	- .description.alert-danger
17	- %span
18	- %i.icon-remove
19	- Closed by #{link_to_member(@project, @merge_request.closed_event.author)}
20	- #{time_ago_with_tooltip(@merge_request.closed_event.created_at)}.
21	- - if @merge_request.merged?
22	- .description.alert-success
23	- %span
24	- %i.icon-ok
25	- Merged by #{link_to_member(@project, @merge_request.merge_event.author)}
26	- #{time_ago_with_tooltip(@merge_request.merge_event.created_at)}.
27	- - if !@closes_issues.empty? && @merge_request.opened?
28	- .description.alert-info
29	- %span
30	- %i.icon-ok
31	- Accepting this merge request will close #{@closes_issues.size == 1 ? 'issue' : 'issues'}
32	- = succeed '.' do
33	- != gfm(@closes_issues.map { \|i\| "##{i.iid}" }.to_sentence)	24	+ .context
		25	+ %cite.cgray
		26	+ = render partial: 'projects/merge_requests/show/context', locals: { merge_request: @merge_request }
		27	+
		28	+- if @merge_request.closed?
		29	+ .alert.alert-info
		30	+ %span
		31	+ %i.icon-remove
		32	+ Closed by #{link_to_member(@project, @merge_request.closed_event.author)}
		33	+ #{time_ago_with_tooltip(@merge_request.closed_event.created_at)}.
		34	+- if @merge_request.merged?
		35	+ .alert.alert-info
		36	+ %span
		37	+ %i.icon-ok
		38	+ Merged by #{link_to_member(@project, @merge_request.merge_event.author)}
		39	+ #{time_ago_with_tooltip(@merge_request.merge_event.created_at)}.
		40	+- if !@closes_issues.empty? && @merge_request.opened?
		41	+ .alert.alert-info.alert-info
		42	+ %span
		43	+ %i.icon-ok
		44	+ Accepting this merge request will close #{@closes_issues.size == 1 ? 'issue' : 'issues'}
		45	+ = succeed '.' do
		46	+ != gfm(@closes_issues.map { \|i\| "##{i.iid}" }.to_sentence)
1	%h3.page-title	1	%h3.page-title
2	= "Merge Request ##{@merge_request.iid}"	2	= "Merge Request ##{@merge_request.iid}"
3	- %small
4	- created #{time_ago_with_tooltip(@merge_request.created_at)}
5	-
6	- - if @merge_request.merged?
7	- %span.state-label.state-label-green
8	- %i.icon-ok
9	- Merged
10	- - elsif @merge_request.closed?
11	- %span.state-label.state-label-red
12	- Closed
13	- - else
14	- %span.state-label.state-label-green
15	- Open
16	-
17	-
18		3
19	%span.pull-right	4	%span.pull-right
20	- if can?(current_user, :modify_merge_request, @merge_request)	5	- if can?(current_user, :modify_merge_request, @merge_request)