Merge branch '29857-move-source-target-branch-to-be-above-merge-request-description' into 'master'

Reduce redundancy and move source/target branch selectors to top of MR view

See merge request gitlab-org/gitlab!17559

app/assets/stylesheets/framework/variables.scss

@@ -20,6 +20,27 @@ $spacing-scale: (
   5: #{4 * $grid-size}
 );
 
+/*
+ * Why another sizing scale???
+ * Great question, friend!
+ * This size scale is a "backport" of the equivalent set of "named" sizes
+ * (e.g. `xl` versus `70`) that came from the following design document as of 2019-10-23:
+ *
+ * https://gitlab-org.gitlab.io/gitlab-design/hosted/design-gitlab-specs/forms-spec-previews/
+ *
+ * (See `input-` items at the bottom)
+ *
+ * The presumption here is that these sizes will be standardized in GitLab UI and thus will be
+ * broadly useful here in the GitLab product when not using the GitLab UI components.
+ */
+$size-scale: (
+  'xs': #{10 * $grid-size},
+  's': #{20 * $grid-size},
+  'm': #{30 * $grid-size},
+  'l': #{40 * $grid-size},
+  'xl': #{70 * $grid-size}
+);
+
 /*
  * Color schema
  */

app/assets/stylesheets/utilities.scss

@@ -16,6 +16,12 @@
   }
 }
 
+@each $index, $size in $size-scale {
+  #{'.mw-#{$index}'} {
+    max-width: $size;
+  }
+}
+
 .border-width-1px { border-width: 1px; }
 .border-style-dashed { border-style: dashed; }
 .border-style-solid { border-style: solid; }

app/views/projects/merge_requests/creations/_new_submit.html.haml

 %h3.page-title
   New Merge Request
-%p.slead
-  - source_title, target_title = format_mr_branch_names(@merge_request)
-  From
-  %strong.ref-name= source_title
-  %span into
-  %strong.ref-name= target_title
-
-  %span.float-right
-    = link_to 'Change branches', mr_change_branches_path(@merge_request)
-%hr
 = form_for [@project.namespace.becomes(Namespace), @project, @merge_request], html: { class: 'merge-request-form common-note-form js-requires-input js-quick-submit' } do |f|
   = render 'shared/issuable/form', f: f, issuable: @merge_request, commits: @commits, presenter: @mr_presenter
   = f.hidden_field :source_project_id

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

@@ -2,5 +2,4 @@
 
 %h3.page-title
   Edit Merge Request #{@merge_request.to_reference}
-%hr
 = render 'form'

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

@@ -12,6 +12,9 @@
     = link_to "the #{issuable.class.model_name.human.downcase}", polymorphic_path([@project.namespace.becomes(Namespace), @project, issuable]), target: "_blank", rel: 'noopener noreferrer'
     and make sure your changes will not unintentionally remove theirs
 
+= render 'shared/issuable/form/branch_chooser', issuable: issuable, form: form
+
+%hr
 .form-group.row
   = form.label :title, class: 'col-form-label col-sm-2'
 
@@ -34,8 +37,6 @@
 
 = render_if_exists 'shared/issuable/approvals', issuable: issuable, presenter: presenter, form: form
 
-= render 'shared/issuable/form/branch_chooser', issuable: issuable, form: form
-
 = render 'shared/issuable/form/merge_params', issuable: issuable
 
 = render 'shared/issuable/form/contribution', issuable: issuable, form: form

app/views/shared/issuable/form/_branch_chooser.html.haml

@@ -4,21 +4,19 @@
 - return unless issuable.is_a?(MergeRequest)
 - return if issuable.closed_without_fork?
 
-%hr
-- if issuable.new_record?
-  .form-group.row
-    = form.label :source_branch, class: 'col-form-label col-sm-2'
-    .col-sm-10
-      .issuable-form-select-holder
-        = form.select(:source_branch, [issuable.source_branch], {}, { class: 'source_branch select2 ref-name', disabled: true })
-.form-group.row
-  = form.label :target_branch, class: 'col-form-label col-sm-2'
-  .col-sm-10.target-branch-select-dropdown-container
-    .issuable-form-select-holder
-      = form.hidden_field(:target_branch,
-        { class: 'target_branch js-target-branch-select ref-name',
-          disabled: issuable.new_record?,
-          data: { placeholder: "Select branch", endpoint: refs_project_path(@project, sort: 'updated_desc', find: 'branches') }})
+- source_title, target_title = format_mr_branch_names(@merge_request)
+
+.form-group.row.d-flex.gl-pl-3.gl-pr-3.branch-selector
+  .align-self-center
+    %span= s_('From %{source_title} into').html_safe % { source_title: "<code>#{source_title}</code>".html_safe }
     - if issuable.new_record?
+      %code= target_title
       &nbsp;
-      = link_to 'Change branches', mr_change_branches_path(issuable)
+      = link_to _('Change branches'), mr_change_branches_path(issuable)
+    - elsif issuable.for_fork?
+      %code= issuable.target_project_path + ":"
+  - unless issuable.new_record?
+    %span.dropdown.prepend-left-5.d-inline-block
+      = form.hidden_field(:target_branch,
+        { class: 'target_branch js-target-branch-select ref-name mw-xl',
+          data: { placeholder: _('Select branch'), endpoint: refs_project_path(@project, sort: 'updated_desc', find: 'branches') }})

app/views/shared/issuable/form/_merge_params.html.haml

@@ -3,17 +3,17 @@
 - return unless issuable.is_a?(MergeRequest)
 - return if issuable.closed_without_fork?
 
-- if issuable.can_remove_source_branch?(current_user)
-  .form-group.row
-    .col-sm-10.offset-sm-2
-      .form-check
+.form-group.row
+  .col-sm-2.col-form-label.pt-sm-0
+    %label
+      = _('Merge options')
+  .col-sm-10
+    - if issuable.can_remove_source_branch?(current_user)
+      .form-check.append-bottom-default
         = hidden_field_tag 'merge_request[force_remove_source_branch]', '0', id: nil
         = check_box_tag 'merge_request[force_remove_source_branch]', '1', issuable.force_remove_source_branch?, class: 'form-check-input'
         = label_tag 'merge_request[force_remove_source_branch]', class: 'form-check-label' do
           Delete source branch when merge request is accepted.
-
-.form-group.row
-  .col-sm-10.offset-sm-2
     .form-check
       = hidden_field_tag 'merge_request[squash]', '0', id: nil
       = check_box_tag 'merge_request[squash]', '1', issuable.squash, class: 'form-check-input'

changelogs/unreleased/maintenance-move-branch-selector-to-top.yml

+---
+title: Reduce new MR page redundancy by moving the source/target branch selector to
+  the top
+merge_request: 17559
+author:
+type: changed

locale/gitlab.pot

@@ -2898,6 +2898,9 @@ msgstr ""
 msgid "Change assignee(s)."
 msgstr ""
 
+msgid "Change branches"
+msgstr ""
+
 msgid "Change label"
 msgstr ""
 
@@ -7397,6 +7400,9 @@ msgstr ""
 msgid "From %{providerTitle}"
 msgstr ""
 
+msgid "From %{source_title} into"
+msgstr ""
+
 msgid "From Bitbucket"
 msgstr ""
 
@@ -10287,6 +10293,9 @@ msgstr ""
 msgid "Merge in progress"
 msgstr ""
 
+msgid "Merge options"
+msgstr ""
+
 msgid "Merge request"
 msgstr ""
 
@@ -14871,6 +14880,9 @@ msgstr ""
 msgid "Select an existing Kubernetes cluster or create a new one"
 msgstr ""
 
+msgid "Select branch"
+msgstr ""
+
 msgid "Select branch/tag"
 msgstr ""
 

spec/features/dashboard/projects_spec.rb

@@ -216,8 +216,7 @@ describe 'Dashboard Projects' do
       expect(page).to have_selector('.merge-request-form')
       expect(current_path).to eq project_new_merge_request_path(project)
       expect(find('#merge_request_target_project_id', visible: false).value).to eq project.id.to_s
-      expect(find('input#merge_request_source_branch', visible: false).value).to eq 'feature'
-      expect(find('input#merge_request_target_branch', visible: false).value).to eq 'master'
+      expect(page).to have_content "From feature into master"
     end
   end
 

spec/features/merge_request/user_edits_merge_request_spec.rb

@@ -17,7 +17,7 @@ describe 'User edits a merge request', :js do
   end
 
   it 'changes the target branch' do
-    expect(page).to have_content('Target branch')
+    expect(page).to have_content('From master into feature')
 
     select2('merge-test', from: '#merge_request_target_branch')
     click_button('Save changes')

spec/features/projects/commit/cherry_pick_spec.rb

@@ -55,12 +55,16 @@ describe 'Cherry-pick Commits' do
     end
   end
 
-  context "I cherry-pick a commit in a new merge request" do
+  context "I cherry-pick a commit in a new merge request", :js do
     it do
+      find('.header-action-buttons a.dropdown-toggle').click
       find("a[href='#modal-cherry-pick-commit']").click
       page.within('#modal-cherry-pick-commit') do
         click_button 'Cherry-pick'
       end
+
+      wait_for_requests
+
       expect(page).to have_content("The commit has been successfully cherry-picked into cherry-pick-#{master_pickable_commit.short_id}. You can now submit a merge request to get this change into the original branch.")
       expect(page).to have_content("From cherry-pick-#{master_pickable_commit.short_id} into master")
     end