From fec96453e73ae715c6888c7369cbb1b020e873b5 Mon Sep 17 00:00:00 2001
From: Annabel Dunstone Gray <annabel.m.gray@gmail.com>
Date: Thu, 6 Jun 2019 14:44:13 +0000
Subject: Merge request tabs fixes

- Makes activity dropdown and resolved counter smaller
- Uses grid for spacing
- Removes scrolling on MR tabs
---
 .../notes/components/discussion_counter.vue        | 12 +++--
 .../notes/components/discussion_filter.vue         |  4 +-
 app/assets/stylesheets/framework/blocks.scss       |  9 +---
 app/assets/stylesheets/framework/common.scss       | 12 +++++
 .../framework/secondary_navigation_elements.scss   | 52 +++++++++++-----------
 app/assets/stylesheets/pages/groups.scss           |  3 --
 app/assets/stylesheets/pages/issuable.scss         | 10 ++++-
 app/assets/stylesheets/pages/merge_requests.scss   | 35 ++++++++++++---
 app/assets/stylesheets/pages/notes.scss            | 13 ++++--
 app/views/dashboard/todos/index.html.haml          |  2 +-
 app/views/groups/show.html.haml                    |  2 +-
 app/views/projects/issues/_new_branch.html.haml    |  8 ++--
 app/views/projects/merge_requests/show.html.haml   | 43 +++++++++---------
 app/views/projects/tags/index.html.haml            |  2 +-
 app/views/projects/tags/show.html.haml             |  2 +-
 app/views/users/show.html.haml                     |  2 +-
 changelogs/unreleased/55125-mr-tab-scrolling.yml   |  5 +++
 17 files changed, 131 insertions(+), 85 deletions(-)
 create mode 100644 changelogs/unreleased/55125-mr-tab-scrolling.yml

diff --git a/app/assets/javascripts/notes/components/discussion_counter.vue b/app/assets/javascripts/notes/components/discussion_counter.vue
index 307e56708e0..efd84f5722c 100644
--- a/app/assets/javascripts/notes/components/discussion_counter.vue
+++ b/app/assets/javascripts/notes/components/discussion_counter.vue
@@ -49,8 +49,8 @@ export default {
 </script>
 
 <template>
-  <div v-if="resolvableDiscussionsCount > 0" class="line-resolve-all-container prepend-top-8">
-    <div>
+  <div v-if="resolvableDiscussionsCount > 0" class="line-resolve-all-container full-width-mobile">
+    <div class="full-width-mobile d-flex d-sm-block">
       <div :class="{ 'has-next-btn': hasNextButton }" class="line-resolve-all">
         <span
           :class="{ 'is-active': allResolved }"
@@ -64,7 +64,11 @@ export default {
           {{ n__('discussion resolved', 'discussions resolved', resolvableDiscussionsCount) }}
         </span>
       </div>
-      <div v-if="resolveAllDiscussionsIssuePath && !allResolved" class="btn-group" role="group">
+      <div
+        v-if="resolveAllDiscussionsIssuePath && !allResolved"
+        class="btn-group btn-group-sm"
+        role="group"
+      >
         <a
           v-gl-tooltip
           :href="resolveAllDiscussionsIssuePath"
@@ -74,7 +78,7 @@ export default {
           <icon name="issue-new" />
         </a>
       </div>
-      <div v-if="isLoggedIn && !allResolved" class="btn-group" role="group">
+      <div v-if="isLoggedIn && !allResolved" class="btn-group btn-group-sm" role="group">
         <button
           v-gl-tooltip
           title="Jump to first unresolved discussion"
diff --git a/app/assets/javascripts/notes/components/discussion_filter.vue b/app/assets/javascripts/notes/components/discussion_filter.vue
index 47951591e82..eb3fbbe1385 100644
--- a/app/assets/javascripts/notes/components/discussion_filter.vue
+++ b/app/assets/javascripts/notes/components/discussion_filter.vue
@@ -105,12 +105,12 @@ export default {
 <template>
   <div
     v-if="displayFilters"
-    class="discussion-filter-container js-discussion-filter-container d-inline-block align-bottom"
+    class="discussion-filter-container js-discussion-filter-container d-inline-block align-bottom full-width-mobile"
   >
     <button
       id="discussion-filter-dropdown"
       ref="dropdownToggle"
-      class="btn btn-default qa-discussion-filter"
+      class="btn btn-sm qa-discussion-filter"
       data-toggle="dropdown"
       aria-expanded="false"
     >
diff --git a/app/assets/stylesheets/framework/blocks.scss b/app/assets/stylesheets/framework/blocks.scss
index 3aabb66f7a6..65c0ee74c60 100644
--- a/app/assets/stylesheets/framework/blocks.scss
+++ b/app/assets/stylesheets/framework/blocks.scss
@@ -199,6 +199,7 @@
 
   &.user-cover-block {
     padding: 24px 0 0;
+    border-bottom: 1px solid $border-color;
 
     .nav-links {
       width: 100%;
@@ -232,14 +233,6 @@
   margin-top: -1px;
 }
 
-.nav-block {
-  .controls {
-    float: right;
-    margin-top: 8px;
-    padding-bottom: 8px;
-  }
-}
-
 .content-block {
   padding: $gl-padding 0;
   border-bottom: 1px solid $white-dark;
diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss
index fc488b85138..17635d48461 100644
--- a/app/assets/stylesheets/framework/common.scss
+++ b/app/assets/stylesheets/framework/common.scss
@@ -489,3 +489,15 @@ img.emoji {
 .cursor-pointer {
   cursor: pointer;
 }
+
+// Make buttons/dropdowns full-width on mobile
+.full-width-mobile {
+  @include media-breakpoint-down(xs) {
+    width: 100%;
+
+    > .dropdown-menu,
+    > .btn {
+      width: 100%;
+    }
+  }
+}
diff --git a/app/assets/stylesheets/framework/secondary_navigation_elements.scss b/app/assets/stylesheets/framework/secondary_navigation_elements.scss
index 31297b9d20c..ada8f2fe1a6 100644
--- a/app/assets/stylesheets/framework/secondary_navigation_elements.scss
+++ b/app/assets/stylesheets/framework/secondary_navigation_elements.scss
@@ -13,8 +13,8 @@
 
     a,
     button {
-      padding: $gl-btn-padding;
-      padding-bottom: 11px;
+      padding: $gl-padding-8;
+      padding-bottom: $gl-padding-8 + 1;
       font-size: 14px;
       line-height: 28px;
       color: $gl-text-color-secondary;
@@ -58,8 +58,12 @@
 }
 
 .top-area {
-  @include clearfix;
   border-bottom: 1px solid $border-color;
+  display: flex;
+
+  @include media-breakpoint-down(md) {
+    flex-flow: column-reverse wrap;
+  }
 
   .nav-text {
     padding-top: 16px;
@@ -75,9 +79,8 @@
   }
 
   .nav-links {
-    margin-bottom: 0;
     border-bottom: 0;
-    float: left;
+    flex: 1;
 
     &.wide {
       width: 100%;
@@ -98,16 +101,23 @@
 
       &.mobile-separator {
         border-bottom: 1px solid $border-color;
+        margin-bottom: $gl-padding-8;
       }
     }
   }
 
   .nav-controls {
     display: inline-block;
-    float: right;
     text-align: right;
-    padding: $gl-padding-8 0;
-    margin-bottom: 0;
+
+    @include media-breakpoint-down(sm) {
+      margin-top: $gl-padding-8;
+    }
+
+    @include media-breakpoint-up(md) {
+      display: flex;
+      align-items: center;
+    }
 
     > .btn,
     > .btn-container,
@@ -115,8 +125,6 @@
     > input,
     > form {
       margin-right: $gl-padding-top;
-      display: inline-block;
-      vertical-align: top;
 
       &:last-child {
         margin-right: 0;
@@ -143,7 +151,7 @@
       @include media-breakpoint-up(lg) { width: 250px; }
     }
 
-    @include media-breakpoint-down(xs) {
+    @include media-breakpoint-down(sm) {
       padding-bottom: 0;
       width: 100%;
 
@@ -153,7 +161,7 @@
       .dropdown-toggle,
       .dropdown-menu-toggle,
       .form-control {
-        margin: 0 0 10px;
+        margin: 0 0 $gl-padding-8;
         display: block;
         width: 100%;
       }
@@ -165,7 +173,7 @@
       form {
         display: block;
         height: auto;
-        margin-bottom: 14px;
+        margin-bottom: $gl-padding-8;
 
         input {
           width: 100%;
@@ -236,20 +244,11 @@
     width: 100%;
   }
 
-  @include media-breakpoint-down(xs) {
-    flex-flow: row wrap;
-
+  @include media-breakpoint-down(md) {
     .nav-controls {
       $controls-margin: $btn-margin-5 - 2px;
       flex: 0 0 100%;
-
-      &.controls-flex {
-        display: flex;
-        flex-flow: row wrap;
-        align-items: center;
-        justify-content: center;
-        padding: 0 0 $gl-padding-top;
-      }
+      margin-top: $gl-padding-8;
 
       .controls-item,
       .controls-item-full,
@@ -326,8 +325,8 @@
 
   .fade-right,
   .fade-left {
-    top: 16px;
-    bottom: auto;
+    bottom: $gl-padding;
+    top: auto;
   }
 
   &.is-smaller {
@@ -367,6 +366,7 @@
     display: flex;
     border-bottom: 1px solid $border-color;
     overflow: hidden;
+    align-items: center;
 
     .nav-links {
       border-bottom: 0;
diff --git a/app/assets/stylesheets/pages/groups.scss b/app/assets/stylesheets/pages/groups.scss
index 0a07747e0d4..656202f4e58 100644
--- a/app/assets/stylesheets/pages/groups.scss
+++ b/app/assets/stylesheets/pages/groups.scss
@@ -35,9 +35,6 @@
 }
 
 .group-nav-container .nav-controls {
-  align-items: flex-start;
-  padding: $gl-padding-top 0 0;
-
   .group-filter-form {
     flex: 1 1 auto;
     margin-right: $gl-padding-8;
diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss
index 4ba74d34664..79282f9043c 100644
--- a/app/assets/stylesheets/pages/issuable.scss
+++ b/app/assets/stylesheets/pages/issuable.scss
@@ -69,7 +69,11 @@
   }
 
   .emoji-block {
-    padding: 10px 0;
+    padding: $gl-padding-4 0;
+
+    @include media-breakpoint-down(md) {
+      padding: $gl-padding-8 0;
+    }
   }
 }
 
@@ -132,6 +136,10 @@
   z-index: 200;
   overflow: hidden;
 
+  @include media-breakpoint-down(sm) {
+    z-index: 251;
+  }
+
   a:not(.btn) {
     color: inherit;
 
diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index 77b40fe2d30..8cb3fab74e0 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -846,15 +846,40 @@
   display: flex;
   justify-content: space-between;
 
-  @include media-breakpoint-down(sm) {
-    flex-direction: column-reverse;
+  @include media-breakpoint-down(xs) {
+    .discussion-filter-container,
+    .line-resolve-all-container {
+      margin-bottom: $gl-padding-4;
+    }
   }
 
   .discussion-filter-container {
-    margin-top: $gl-padding-8;
-
     &:not(:only-child) {
-      padding-right: $gl-padding-8;
+      margin: $gl-padding-4;
+    }
+  }
+
+  .merge-request-tabs {
+    height: $grid-size * 6;
+  }
+}
+
+// Wrap MR tabs/buttons so you don't have to scroll on desktop
+@include media-breakpoint-down(md) {
+  .merge-request-tabs-container,
+  .epic-tabs-container {
+    flex-direction: column-reverse;
+    padding-top: $gl-padding-8;
+  }
+}
+
+@include media-breakpoint-down(lg) {
+  .right-sidebar-expanded {
+    .merge-request-tabs-container,
+    .epic-tabs-container {
+      flex-direction: column-reverse;
+      align-items: flex-start;
+      padding-top: $gl-padding-8;
     }
   }
 }
diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss
index 52677b7df60..26626217043 100644
--- a/app/assets/stylesheets/pages/notes.scss
+++ b/app/assets/stylesheets/pages/notes.scss
@@ -768,9 +768,7 @@ $note-form-margin-left: 72px;
 }
 
 .line-resolve-all-container {
-  @include notes-media('min', map-get($grid-breakpoints, sm)) {
-    margin-right: 0;
-  }
+  margin: $gl-padding-4;
 
   > div {
     white-space: nowrap;
@@ -786,6 +784,8 @@ $note-form-margin-left: 72px;
   }
 
   .btn {
+    line-height: $gl-line-height;
+
     svg {
       fill: $gray-darkest;
     }
@@ -811,10 +811,11 @@ $note-form-margin-left: 72px;
 .line-resolve-all {
   vertical-align: middle;
   display: inline-block;
-  padding: 6px 10px;
+  padding: $gl-padding-4 10px;
   background-color: $gray-light;
   border: 1px solid $border-color;
   border-radius: $border-radius-default;
+  font-size: $gl-btn-small-font-size;
 
   &.has-next-btn {
     border-top-right-radius: 0;
@@ -830,6 +831,10 @@ $note-form-margin-left: 72px;
       vertical-align: middle;
     }
   }
+
+  @include media-breakpoint-down(xs) {
+    flex: 1;
+  }
 }
 
 .line-resolve-btn {
diff --git a/app/views/dashboard/todos/index.html.haml b/app/views/dashboard/todos/index.html.haml
index 214630d245a..8212fb8bb33 100644
--- a/app/views/dashboard/todos/index.html.haml
+++ b/app/views/dashboard/todos/index.html.haml
@@ -34,7 +34,7 @@
             = icon('spinner spin')
 
   .todos-filters
-    .row-content-block.second-block
+    .issues-details-filters.row-content-block.second-block
       = form_tag todos_filter_path(without: [:project_id, :author_id, :type, :action_id]), method: :get, class: 'filter-form d-sm-flex' do
         .filter-categories.flex-fill
           .filter-item.inline
diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml
index 77fe88dacb7..255a9ad038c 100644
--- a/app/views/groups/show.html.haml
+++ b/app/views/groups/show.html.haml
@@ -9,7 +9,7 @@
   = render 'groups/home_panel'
 
   .groups-listing{ data: { endpoints: { default: group_children_path(@group, format: :json), shared: group_shared_projects_path(@group, format: :json) } } }
-    .top-area.group-nav-container
+    .top-area.group-nav-container.justify-content-between
       .scrolling-tabs-container.inner-page-scroll-tabs
         .fade-left= icon('angle-left')
         .fade-right= icon('angle-right')
diff --git a/app/views/projects/issues/_new_branch.html.haml b/app/views/projects/issues/_new_branch.html.haml
index fbd70cd1906..457b2936278 100644
--- a/app/views/projects/issues/_new_branch.html.haml
+++ b/app/views/projects/issues/_new_branch.html.haml
@@ -8,18 +8,18 @@
   - create_branch_path = project_branches_path(@project, branch_name: @issue.to_branch_name, ref: @project.default_branch, issue_iid: @issue.iid)
   - refs_path = refs_namespace_project_path(@project.namespace, @project, search: '')
 
-  .create-mr-dropdown-wrap.d-inline-block{ data: { can_create_path: can_create_path, create_mr_path: create_mr_path, create_branch_path: create_branch_path, refs_path: refs_path } }
-    .btn-group.unavailable
+  .create-mr-dropdown-wrap.d-inline-block.full-width-mobile{ data: { can_create_path: can_create_path, create_mr_path: create_mr_path, create_branch_path: create_branch_path, refs_path: refs_path } }
+    .btn-group.btn-group-sm.unavailable
       %button.btn.btn-grouped{ type: 'button', disabled: 'disabled' }
         = icon('spinner', class: 'fa-spin')
         %span.text
           Checking branch availability…
 
-    .btn-group.available.hidden
+    .btn-group.btn-group-sm.available.hidden
       %button.btn.js-create-merge-request.btn-success.btn-inverted{ type: 'button', data: { action: data_action } }
         = value
 
-      %button.btn.create-merge-request-dropdown-toggle.dropdown-toggle.btn-success.btn-inverted.js-dropdown-toggle{ type: 'button', data: { dropdown: { trigger: '#create-merge-request-dropdown' }, display: 'static' } }
+      %button.btn.create-merge-request-dropdown-toggle.dropdown-toggle.btn-success.btn-inverted.js-dropdown-toggle.flex-grow-0{ type: 'button', data: { dropdown: { trigger: '#create-merge-request-dropdown' }, display: 'static' } }
         = icon('caret-down')
 
       .droplab-dropdown
diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml
index 05aeb5d972b..a201fafb949 100644
--- a/app/views/projects/merge_requests/show.html.haml
+++ b/app/views/projects/merge_requests/show.html.haml
@@ -31,29 +31,26 @@
 
     .merge-request-tabs-holder{ class: ("js-tabs-affix" unless ENV['RAILS_ENV'] == 'test') }
       .merge-request-tabs-container
-        .scrolling-tabs-container.inner-page-scroll-tabs.is-smaller
-          .fade-left= icon('angle-left')
-          .fade-right= icon('angle-right')
-          %ul.merge-request-tabs.nav-tabs.nav.nav-links.scrolling-tabs
-            %li.notes-tab.qa-notes-tab
-              = tab_link_for @merge_request, :show, force_link: @commit.present? do
-                = _("Discussion")
-                %span.badge.badge-pill= @merge_request.related_notes.user.count
-            - if @merge_request.source_project
-              %li.commits-tab
-                = tab_link_for @merge_request, :commits do
-                  = _("Commits")
-                  %span.badge.badge-pill= @commits_count
-            - if @pipelines.any?
-              %li.pipelines-tab
-                = tab_link_for @merge_request, :pipelines do
-                  = _("Pipelines")
-                  %span.badge.badge-pill.js-pipelines-mr-count= @pipelines.size
-            %li.diffs-tab.qa-diffs-tab
-              = tab_link_for @merge_request, :diffs do
-                = _("Changes")
-                %span.badge.badge-pill= @merge_request.diff_size
-        .d-inline-flex.flex-wrap
+        %ul.merge-request-tabs.nav-tabs.nav.nav-links
+          %li.notes-tab.qa-notes-tab
+            = tab_link_for @merge_request, :show, force_link: @commit.present? do
+              = _("Discussion")
+              %span.badge.badge-pill= @merge_request.related_notes.user.count
+          - if @merge_request.source_project
+            %li.commits-tab
+              = tab_link_for @merge_request, :commits do
+                = _("Commits")
+                %span.badge.badge-pill= @commits_count
+          - if @pipelines.any?
+            %li.pipelines-tab
+              = tab_link_for @merge_request, :pipelines do
+                = _("Pipelines")
+                %span.badge.badge-pill.js-pipelines-mr-count= @pipelines.size
+          %li.diffs-tab.qa-diffs-tab
+            = tab_link_for @merge_request, :diffs do
+              = _("Changes")
+              %span.badge.badge-pill= @merge_request.diff_size
+        .d-flex.flex-wrap.align-items-center.justify-content-lg-end
           #js-vue-discussion-filter{ data: { default_filter: current_user&.notes_filter_for(@merge_request),
             notes_filters: UserPreference.notes_filters.to_json } }
           #js-vue-discussion-counter
diff --git a/app/views/projects/tags/index.html.haml b/app/views/projects/tags/index.html.haml
index 458096f9dd6..2e78b0bff3e 100644
--- a/app/views/projects/tags/index.html.haml
+++ b/app/views/projects/tags/index.html.haml
@@ -9,7 +9,7 @@
     .nav-text.row-main-content
       = s_('TagsPage|Tags give the ability to mark specific points in history as being important')
 
-    .nav-controls.row-fixed-content
+    .nav-controls
       = form_tag(filter_tags_path, method: :get) do
         = search_field_tag :search, params[:search], { placeholder: s_('TagsPage|Filter by tag name'), id: 'tag-search', class: 'form-control search-text-input input-short', spellcheck: false }
 
diff --git a/app/views/projects/tags/show.html.haml b/app/views/projects/tags/show.html.haml
index 0be62bc5612..59232372150 100644
--- a/app/views/projects/tags/show.html.haml
+++ b/app/views/projects/tags/show.html.haml
@@ -18,7 +18,7 @@
       - else
         = s_("TagsPage|Can't find HEAD commit for this tag")
 
-    .nav-controls.controls-flex
+    .nav-controls
       - if can?(current_user, :push_code, @project)
         = link_to edit_project_tag_release_path(@project, @tag.name), class: 'btn btn-edit controls-item has-tooltip', title: s_('TagsPage|Edit release notes') do
           = icon("pencil")
diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml
index 6dc61088e65..a71bfd624e4 100644
--- a/app/views/users/show.html.haml
+++ b/app/views/users/show.html.haml
@@ -9,7 +9,7 @@
   = auto_discovery_link_tag(:atom, user_url(@user, format: :atom), title: "#{@user.name} activity")
 
 .user-profile
-  .cover-block.user-cover-block.top-area
+  .cover-block.user-cover-block
     .cover-controls
       - if @user == current_user
         = link_to profile_path, class: 'btn btn-default has-tooltip', title: s_('UserProfile|Edit profile'), 'aria-label': 'Edit profile' do
diff --git a/changelogs/unreleased/55125-mr-tab-scrolling.yml b/changelogs/unreleased/55125-mr-tab-scrolling.yml
new file mode 100644
index 00000000000..e03ff6c5060
--- /dev/null
+++ b/changelogs/unreleased/55125-mr-tab-scrolling.yml
@@ -0,0 +1,5 @@
+---
+title: Update merge request tabs so they no longer scroll
+merge_request:
+author:
+type: other
-- 
cgit v1.2.1