Merge branch 'ui-right-sidebar' into 'master'

Implement new sidebar for issue and merge request pages

This is first step in implementing #3981. We don't have new dropdowns yet so I used some workarounds. 
Also I did it from linux machine without having access to design materials. So some colors/paddings might not match - but I will improve it with next iteration. 

cc @creamzy @JobV 

See merge request !2058

CHANGELOG

@@ -47,6 +47,7 @@ v 8.3.0 (unreleased)
   - Fix award-emojis Flash alert's width
   - Fix deleting notes on a merge request diff
   - Display referenced merge request statuses in the issue description (Greg Smethells)
+  - Implement new sidebar for issue and merge request pages
 
 v 8.2.3
   - Fix application settings cache not expiring after changes (Stan Hu)

app/assets/javascripts/issuable_context.js.coffee

@@ -5,9 +5,9 @@ class @IssuableContext
     new UsersSelect()
     $('select.select2').select2({width: 'resolve', dropdownAutoWidth: true})
 
-    $(".context .inline-update").on "change", "select", ->
+    $(".issuable-sidebar .inline-update").on "change", "select", ->
       $(this).submit()
-    $(".context .inline-update").on "change", ".js-assignee", ->
+    $(".issuable-sidebar .inline-update").on "change", ".js-assignee", ->
       $(this).submit()
 
     $('.issuable-details').waitForImages ->
@@ -18,6 +18,12 @@ class @IssuableContext
 
       $('.issuable-affix').affix offset:
         top: ->
-          @top = ($('.issuable-affix').offset().top - 70)
+          @top = ($('.issuable-affix').offset().top - 60)
         bottom: ->
           @bottom = $('.footer').outerHeight(true)
+
+    $(".edit-link").click (e) ->
+      block = $(@).parents('.block')
+      block.find('.selectbox').show()
+      block.find('.value').hide()
+      block.find('.js-select2').select2("open")

app/assets/stylesheets/framework/common.scss

@@ -461,3 +461,9 @@ table {
     visibility: hidden;
   }
 }
+
+.content-separator {
+  margin-left: -$gl-padding;
+  margin-right: -$gl-padding;
+  border-top: 1px solid $border-color;
+}

app/assets/stylesheets/pages/awards.scss

+.awards {
+  @include clearfix;
+  line-height: 34px;
+
+  .award {
+    @include border-radius(5px);
+
+    border: 1px solid;
+    padding: 0px 10px;
+    float: left;
+    margin-right: 5px;
+    border-color: $border-color;
+    cursor: pointer;
+
+    &.active {
+      border-color: $border-gray-light;
+      background-color: $gray-light;
+
+      .counter {
+        font-weight: bold;
+      }
+    }
+
+    .icon {
+      float: left;
+      margin-right: 10px;
+    }
+
+    .counter {
+      float: left;
+    }
+  }
+
+  .awards-controls {
+    margin-left: 10px;
+    float: left;
+
+    .add-award {
+      font-size: 24px;
+      color: $gl-gray;
+      position: relative;
+      top: 2px;
+
+      &:hover,
+      &:link {
+        text-decoration: none;
+      }
+    }
+
+    .awards-menu {
+      padding: $gl-padding;
+      min-width: 214px;
+
+      > li {
+        cursor: pointer;
+        margin: 5px;
+      }
+    }
+  }
+
+  .awards-menu{
+    li {
+      float: left;
+      margin: 3px;
+    }
+  }
+}

app/assets/stylesheets/pages/issuable.scss

@@ -18,26 +18,12 @@
 
     &.affix {
       position: fixed;
-      top: 70px;
+      top: 60px;
       margin-right: 35px;
     }
   }
 }
 
-.issuable-context-title {
-  margin-bottom: 5px;
-
-  .avatar {
-    margin-left: 0;
-  }
-
-  label {
-    color: $gl-gray;
-    font-weight: normal;
-    margin-right: 4px;
-  }
-}
-
 .project-issuable-filter {
   .controls {
     float: right;
@@ -50,23 +36,6 @@
 }
 
 .issuable-details {
-  .page-title {
-    margin-top: -$gl-padding;
-    padding: 7px 0;
-    margin-bottom: 0;
-    color: #5c5d5e;
-    font-size: 16px;
-    line-height: 42px;
-
-    .author {
-      color: #5c5d5e;
-    }
-
-    .issue-id {
-      color: #5c5d5e;
-    }
-  }
-
   .issue-title {
     margin: 0;
     font-size: 23px;
@@ -80,6 +49,21 @@
       margin-bottom: 0;
     }
   }
+
+  section {
+    border-right: 1px solid #ECEEF1;
+
+    > .tab-content {
+      margin-right: 1px;
+    }
+
+    .issue-discussion > .gray-content-block,
+    > .gray-content-block {
+      margin-top: 0;
+      border-top: none;
+      margin-right: -15px;
+    }
+  }
 }
 
 .issuable-filter-count {
@@ -101,84 +85,72 @@
   }
 }
 
-.cross-project-reference {
-  text-align: center;
-  width: 100%;
+.issuable-sidebar {
+  .block {
+    @include clearfix;
+    padding:  $gl-padding 0;
+    border-bottom: 1px solid #F0F0F0;
 
-  .slead {
-    padding: 5px;
+    &:last-child {
+      border: none;
     }
-
-  span, button {
-    background-color: $background-color;
   }
-}
-
-.awards {
-  @include clearfix;
-  line-height: 34px;
-  margin: 2px 0;
 
-  .award {
-    @include border-radius(5px);
+  .title {
+    color: $gl-text-color;
+    margin-bottom: 8px;
 
-    border: 1px solid;
-    padding: 0px 10px;
-    float: left;
-    margin: 0 5px;
-    border-color: $border-color;
-    cursor: pointer;
+    .avatar {
+      margin-left: 0;
+    }
 
-    &.active {
-      border-color: $border-gray-light;
-      background-color: $gray-light;
+    label {
+      font-weight: normal;
+      margin-right: 4px;
+    }
 
-      .counter {
-        font-weight: bold;
+    .edit-link {
+      color: $gl-gray;
     }
   }
 
-    .icon {
-      float: left;
-      margin-right: 10px;
-    }
+  .cross-project-reference {
+    font-weight: bold;
+    color: $gl-link-color;
 
-    .counter {
-      float: left;
+    button {
+      float: right;
     }
   }
 
-  .awards-controls {
-    margin-left: 10px;
-    float: left;
+  .selectbox {
+    display: none
+  }
 
-    .add-award {
-      font-size: 24px;
+  .btn-clipboard {
     color: $gl-gray;
-      position: relative;
-      top: 2px;
-
-      &:hover,
-      &:link {
-        text-decoration: none;
   }
+
+  .participants .avatar {
+    margin-top: 6px;
+    margin-right: 2px;
   }
+}
 
-    .awards-menu {
-      padding: $gl-padding;
-      min-width: 214px;
+.issuable-title {
+  margin: -$gl-padding;
+  padding: 7px $gl-padding;
+  margin-bottom: 0px;
+  border-bottom: 1px solid $border-color;
+  color: #5c5d5e;
+  font-size: 16px;
+  line-height: 42px;
 
-      > li {
-        cursor: pointer;
-        margin: 5px;
-      }
-    }
+  .author {
+    color: #5c5d5e;
   }
 
-  .awards-menu{
-    li {
-      float: left;
-      margin: 3px;
-    }
+  .issuable-id {
+    color: #5c5d5e;
   }
 }

app/assets/stylesheets/pages/merge_requests.scss

@@ -121,10 +121,6 @@
   }
 }
 
-.merge-request-details {
-  margin-bottom: $gl-padding;
-}
-
 .mr_source_commit,
 .mr_target_commit {
   .commit {

app/controllers/projects/issues_controller.rb

@@ -58,7 +58,6 @@ class Projects::IssuesController < Projects::ApplicationController
   end
 
   def show
-    @participants = @issue.participants(current_user)
     @note = @project.notes.new(noteable: @issue)
     @notes = @issue.notes.nonawards.with_associations.fresh
     @noteable = @issue

app/controllers/projects/merge_requests_controller.rb

@@ -279,8 +279,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   end
 
   def define_show_vars
-    @participants = @merge_request.participants(current_user)
-
     # Build a note object for comment form
     @note = @project.notes.new(noteable: @merge_request)
     @notes = @merge_request.mr_and_commit_notes.nonawards.inc_author.fresh

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

@@ -5,24 +5,8 @@
     - else
       = link_to 'Close Issue', issue_path(@issue, issue: {state_event: :close}, status_only: true), method: :put, class: 'btn btn-grouped btn-close js-note-target-close', title: 'Close Issue'
 
-= render 'shared/show_aside'
-
 .gray-content-block.second-block.oneline-block
-  .row
-    .col-md-9
-      .votes-holder.pull-right
-        #votes= render 'votes/votes_block', votable: @issue
-      = render "shared/issuable/participants"
-    .col-md-3
-      .input-group.cross-project-reference
-        %span#cross-project-reference.slead.has_tooltip{title: 'Cross-project reference'}
-          = cross_project_reference(@project, @issue)
-        = clipboard_button(clipboard_target: 'span#cross-project-reference')
+  = render 'votes/votes_block', votable: @issue
 
-.row
-  %section.col-md-9
-    .voting_notes#notes= render 'projects/notes/notes_with_form'
-  %aside.col-md-3
-    .issuable-affix
-      .context
-        = render 'shared/issuable/context', issuable: @issue
+#notes
+  = render 'projects/notes/notes_with_form'

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

@@ -3,13 +3,13 @@
 
 .issue
   .issue-details.issuable-details
-    .page-title
+    .issuable-title
       .issue-box{ class: issue_box_class(@issue) }
         - if @issue.closed?
           Closed
         - else
           Open
-      %span.issue-id Issue ##{@issue.iid}
+      %span.issuable-id Issue ##{@issue.iid}
       %span.creator
         ·
         opened by #{link_to_member(@project, @issue.author, size: 24)}
@@ -36,7 +36,9 @@
             = icon('pencil-square-o')
             Edit
 
-    .gray-content-block.middle-block
+    .row
+      %section.col-md-9
+        .gray-content-block
           %h2.issue-title
             = markdown escape_once(@issue.title), pipeline: :single_line
           %div
@@ -55,3 +57,8 @@
           = render 'projects/issues/closed_by_box'
         .issue-discussion
           = render 'projects/issues/discussion'
+
+      %aside.col-md-3
+        = render 'shared/issuable/sidebar', issuable: @issue
+
+      = render 'shared/show_aside'

app/views/projects/issues/update.js.haml

-$('.context').html("#{escape_javascript(render 'shared/issuable/context', issuable: @issue)}");
-$('.context').effect('highlight')
+$('.issuable-sidebar').html("#{escape_javascript(render 'shared/issuable/sidebar', issuable: @issue)}");
+$('.issuable-sidebar').parent().effect('highlight')
 new Issue();

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

@@ -5,24 +5,7 @@
     - if @merge_request.closed?
       = link_to 'Reopen', merge_request_path(@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"
 
-= render 'shared/show_aside'
+.gray-content-block.second-block.oneline-block
+  = render 'votes/votes_block', votable: @merge_request
 
-.gray-content-block.middle-block.oneline-block
-  .row
-    .col-md-9
-      .votes-holder.pull-right
-        #votes= render 'votes/votes_block', votable: @merge_request
-      = render "shared/issuable/participants"
-    .col-md-3
-      .input-group.cross-project-reference
-        %span#cross-project-reference.slead.has_tooltip{title: 'Cross-project reference'}
-          = cross_project_reference(@project, @merge_request)
-        = clipboard_button(clipboard_target: 'span#cross-project-reference')
-
-.row
-  %section.col-md-9
-    .voting_notes#notes= render "projects/notes/notes_with_form"
-  %aside.col-md-3
-    .issuable-affix
-      .context
-        = render 'shared/issuable/context', issuable: @merge_request
+#notes= render "projects/notes/notes_with_form"

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

@@ -7,6 +7,8 @@
 .merge-request{'data-url' => merge_request_path(@merge_request)}
   .merge-request-details.issuable-details
     = render "projects/merge_requests/show/mr_title"
+    .row
+      %section.col-md-9
         = render "projects/merge_requests/show/mr_box"
         .append-bottom-default.mr-source-target.prepend-top-default
           - if @merge_request.open?
@@ -73,6 +75,12 @@
         .mr-loading-status
           = spinner
 
+      %aside.col-md-3
+        = render 'shared/issuable/sidebar', issuable: @merge_request
+
+      = render 'shared/show_aside'
+
+
 :javascript
   var merge_request;
 

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

-.page-title
+.issuable-title
   .issue-box{ class: issue_box_class(@merge_request) }
     = @merge_request.state_human_name
-  %span.issue-id Merge Request ##{@merge_request.iid}
+  %span.issuable-id Merge Request ##{@merge_request.iid}
   %span.creator
     ·
     opened by #{link_to_member(@project, @merge_request.author, size: 24)}

app/views/projects/merge_requests/update.js.haml

-$('.context').html("#{escape_javascript(render 'shared/issuable/context', issuable: @merge_request)}");
-$('.context').effect('highlight')
+$('.issuable-sidebar').html("#{escape_javascript(render 'shared/issuable/sidebar', issuable: @merge_request)}");
+$('.issuable-sidebar').parent().effect('highlight')
 merge_request = new MergeRequest();

app/views/shared/issuable/_context.html.haml

-= form_for [@project.namespace.becomes(Namespace), @project, issuable], remote: true, html: {class: 'issuable-context-form inline-update js-issuable-update'} do |f|
-  %div.prepend-top-default
-    .issuable-context-title
-      %label
-        Assignee:
-      - if issuable.assignee
-        %strong= link_to_member(@project, issuable.assignee, size: 24)
-      - else
-        none
-    .issuable-context-selectbox
-      - if can?(current_user, :"admin_#{issuable.to_ability_name}", @project)
-        = users_select_tag("#{issuable.class.table_name.singularize}[assignee_id]", placeholder: 'Select assignee', class: 'custom-form-control js-select2 js-assignee', selected: issuable.assignee_id, project: @target_project, null_user: true, current_user: true, first_user: true)
-
-  %div.prepend-top-default.clearfix
-    .issuable-context-title
-      %label
-        Milestone:
-      - if issuable.milestone
-        %span.back-to-milestone
-          = link_to namespace_project_milestone_path(@project.namespace, @project, issuable.milestone) do
-            %strong
-              = icon('clock-o')
-              = issuable.milestone.title
-      - else
-        none
-    .issuable-context-selectbox
-      - if can?(current_user, :"admin_#{issuable.to_ability_name}", @project)
-        = 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
-      - 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)}");
-  new IssuableContext();

app/views/shared/issuable/_participants.html.haml

-.participants
-  %span
-    = pluralize @participants.count, "participant"
-  - @participants.each do |participant|
+.block.participants
+  .title
+    = pluralize participants.count, "participant"
+  - participants.each do |participant|
     = link_to_member(@project, participant, name: false, size: 24)

app/views/shared/issuable/_sidebar.html.haml

+.issuable-sidebar.issuable-affix
+  = form_for [@project.namespace.becomes(Namespace), @project, issuable], remote: true, html: {class: 'issuable-context-form inline-update js-issuable-update'} do |f|
+    .block
+      .title
+        Cross-project reference
+      .cross-project-reference
+        %span#cross-project-reference
+          = cross_project_reference(@project, issuable)
+        = clipboard_button(clipboard_target: 'span#cross-project-reference')
+
+    .block.assignee
+      .title
+        %label
+          Assignee
+        - if can?(current_user, :"admin_#{issuable.to_ability_name}", @project)
+          .pull-right
+            = link_to 'Edit', '#', class: 'edit-link'
+      .value
+        - if issuable.assignee
+          %strong= link_to_member(@project, issuable.assignee, size: 24)
+        - else
+          .light None
+
+      .selectbox
+        = users_select_tag("#{issuable.class.table_name.singularize}[assignee_id]", placeholder: 'Select assignee', class: 'custom-form-control js-select2 js-assignee', selected: issuable.assignee_id, project: @target_project, null_user: true, current_user: true, first_user: true)
+
+    .block.milestone
+      .title
+        %label
+          Milestone
+        - if can?(current_user, :"admin_#{issuable.to_ability_name}", @project)
+          .pull-right
+            = link_to 'Edit', '#', class: 'edit-link'
+      .value
+        - if issuable.milestone
+          %span.back-to-milestone
+            = link_to namespace_project_milestone_path(@project.namespace, @project, issuable.milestone) do
+              %strong
+                = icon('clock-o')
+                = issuable.milestone.title
+        - else
+          .light None
+      .selectbox
+        = 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.project.labels.any?
+      .block
+        .title
+          %label Labels
+          - if can?(current_user, :"admin_#{issuable.to_ability_name}", @project)
+            .pull-right
+              = link_to 'Edit', '#', class: 'edit-link'
+        .value.issuable-show-labels
+          - if issuable.labels.any?
+            - issuable.labels.each do |label|
+              = link_to_label(label)
+          - else
+            .light None
+        .selectbox
+          = f.collection_select :label_ids, issuable.project.labels.all, :id, :name,
+            { selected: issuable.label_ids }, multiple: true, class: 'select2 js-select2', data: { placeholder: "Select labels" }
+
+    = render "shared/issuable/participants", participants: issuable.participants(current_user)
+
+    - if current_user
+      - subscribed = issuable.subscribed?(current_user)
+      .block.light
+        .title
+          %label.light Notifications
+        - subscribtion_status = subscribed ? 'subscribed' : 'unsubscribed'
+        %button.btn.btn-block.btn-gray.subscribe-button{:type => 'button'}
+          %span= subscribed ? 'Unsubscribe' : 'Subscribe'
+        .subscription-status{data: {status: subscribtion_status}}
+          .unsubscribed{class: ( 'hidden' if subscribed )}
+            You're not receiving notifications from this thread.
+          .subscribed{class: ( 'hidden' unless subscribed )}
+            You're receiving notifications because you're subscribed to this thread.
+
+  :javascript
+    new Subscription("#{toggle_subscription_path(issuable)}");
+    new IssuableContext();

spec/features/issues_spec.rb

@@ -69,7 +69,10 @@ describe 'Issues', feature: true do
 
       click_button 'Save changes'
 
-      expect(page).to have_content 'Assignee: none'
+      page.within('.assignee') do
+        expect(page).to have_content 'None'
+      end
+
       expect(issue.reload.assignee).to be_nil
     end
   end
@@ -202,11 +205,11 @@ describe 'Issues', feature: true do
       it 'with dropdown menu' do
         visit namespace_project_issue_path(project.namespace, project, issue)
 
-        find('.context #issue_assignee_id').
+        find('.issuable-sidebar #issue_assignee_id').
           set project.team.members.first.id
         click_button 'Update Issue'
 
-        expect(page).to have_content 'Assignee:'
+        expect(page).to have_content 'Assignee'
         has_select?('issue_assignee_id',
                     selected: project.team.members.first.name)
       end
@@ -241,12 +244,16 @@ describe 'Issues', feature: true do
       it 'with dropdown menu' do
         visit namespace_project_issue_path(project.namespace, project, issue)
 
-        find('.context').
+        find('.issuable-sidebar').
           select(milestone.title, from: 'issue_milestone_id')
         click_button 'Update Issue'
 
         expect(page).to have_content "Milestone changed to #{milestone.title}"
-        expect(page).to have_content "Milestone: #{milestone.title}"
+
+        page.within('.milestone') do
+          expect(page).to have_content milestone.title
+        end
+
         has_select?('issue_assignee_id', selected: milestone.title)
       end
     end
@@ -279,13 +286,19 @@ describe 'Issues', feature: true do
 
       it 'allows user to remove assignee', js: true do
         visit namespace_project_issue_path(project.namespace, project, issue)
-        expect(page).to have_content "Assignee: #{user2.name}"
 
-        first('#s2id_issue_assignee_id').click
+        page.within('.assignee') do
+          expect(page).to have_content user2.name
+        end
+
+        find('.assignee .edit-link').click
         sleep 2 # wait for ajax stuff to complete
         first('.user-result').click
 
-        expect(page).to have_content 'Assignee: none'
+        page.within('.assignee') do
+          expect(page).to have_content 'None'
+        end
+
         sleep 2 # wait for ajax stuff to complete
         expect(issue.reload.assignee).to be_nil
       end