Merge branch '49110-update-mr-widget-styles' into 'master'

Resolve "Follow up to "Styling of the MR widget's info and pipeline sections""

Closes #49110

See merge request gitlab-org/gitlab-ce!21206

11 files changed, 324 insertions, 75 deletions

diff --git a/app/assets/javascripts/vue_merge_request_widget/components/deployment.vue b/app/assets/javascripts/vue_merge_request_widget/components/deployment.vue
index 21f21232596..d530ab2767b 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/deployment.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/deployment.vue
@@ -1,5 +1,6 @@
 <script>
 import Icon from '~/vue_shared/components/icon.vue';
+import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue';
 import timeagoMixin from '../../vue_shared/mixins/timeago';
 import tooltip from '../../vue_shared/directives/tooltip';
 import LoadingButton from '../../vue_shared/components/loading_button.vue';
@@ -16,6 +17,7 @@ export default {
     MemoryUsage,
     StatusIcon,
     Icon,
+    TooltipOnTruncate,
   },
   directives: {
     tooltip,
@@ -88,14 +90,20 @@ export default {
               <span>
                 Deployed to
               </span>
-              <a
-                :href="deployment.url"
-                target="_blank"
-                rel="noopener noreferrer nofollow"
-                class="deploy-link js-deploy-meta"
+              <tooltip-on-truncate
+                :title="deployment.name"
+                truncate-target="child"
+                class="deploy-link label-truncate"
               >
-                {{ deployment.name }}
-              </a>
+                <a
+                  :href="deployment.url"
+                  target="_blank"
+                  rel="noopener noreferrer nofollow"
+                  class="js-deploy-meta"
+                >
+                  {{ deployment.name }}
+                </a>
+              </tooltip-on-truncate>
             </template>
             <span
               v-tooltip
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue
index a4c2289c590..72bd28ae03f 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue
@@ -1,18 +1,17 @@
 <script>
-import tooltip from '~/vue_shared/directives/tooltip';
-import { n__ } from '~/locale';
+import _ from 'underscore';
+import { n__, s__, sprintf } from '~/locale';
 import { mergeUrlParams, webIDEUrl } from '~/lib/utils/url_utility';
 import Icon from '~/vue_shared/components/icon.vue';
 import clipboardButton from '~/vue_shared/components/clipboard_button.vue';
+import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue';
 
 export default {
   name: 'MRWidgetHeader',
-  directives: {
-    tooltip,
-  },
   components: {
     Icon,
     clipboardButton,
+    TooltipOnTruncate,
   },
   props: {
     mr: {
@@ -24,8 +23,12 @@ export default {
     shouldShowCommitsBehindText() {
       return this.mr.divergedCommitsCount > 0;
     },
-    commitsText() {
-      return n__('%d commit behind', '%d commits behind', this.mr.divergedCommitsCount);
+    commitsBehindText() {
+      return sprintf(s__('mrWidget|The source branch is %{commitsBehindLinkStart}%{commitsBehind}%{commitsBehindLinkEnd} the target branch'), {
+        commitsBehindLinkStart: `<a href="${_.escape(this.mr.targetBranchPath)}">`,
+        commitsBehind: n__('%d commit behind', '%d commits behind', this.mr.divergedCommitsCount),
+        commitsBehindLinkEnd: '</a>',
+      }, false);
     },
     branchNameClipboardData() {
       // This supports code in app/assets/javascripts/copy_to_clipboard.js that
@@ -36,12 +39,6 @@ export default {
         gfm: `\`${this.mr.sourceBranch}\``,
       });
     },
-    isSourceBranchLong() {
-      return this.isBranchTitleLong(this.mr.sourceBranch);
-    },
-    isTargetBranchLong() {
-      return this.isBranchTitleLong(this.mr.targetBranch);
-    },
     webIdePath() {
       return mergeUrlParams({
         target_project: this.mr.sourceProjectFullPath !== this.mr.targetProjectFullPath ?
@@ -49,11 +46,6 @@ export default {
       }, webIDEUrl(`/${this.mr.sourceProjectFullPath}/merge_requests/${this.mr.iid}`));
     },
   },
-  methods: {
-    isBranchTitleLong(branchTitle) {
-      return branchTitle.length > 32;
-    },
-  },
 };
 </script>
 <template>
@@ -65,30 +57,21 @@ export default {
       <div class="normal">
         <strong>
           {{ s__("mrWidget|Request to merge") }}
-          <span
-            :class="{ 'label-truncated': isSourceBranchLong }"
-            :title="isSourceBranchLong ? mr.sourceBranch : ''"
-            :v-tooltip="isSourceBranchLong"
-            class="label-branch js-source-branch"
-            data-placement="bottom"
+          <tooltip-on-truncate
+            :title="mr.sourceBranch"
+            truncate-target="child"
+            class="label-branch label-truncate js-source-branch"
             v-html="mr.sourceBranchLink"
-          >
-          </span>
-
-          <clipboard-button
+          /><clipboard-button
             :text="branchNameClipboardData"
             :title="__('Copy branch name to clipboard')"
             css-class="btn-default btn-transparent btn-clipboard"
           />
-
           {{ s__("mrWidget|into") }}
-
-          <span
-            :v-tooltip="isTargetBranchLong"
-            :class="{ 'label-truncatedtooltip': isTargetBranchLong }"
-            :title="isTargetBranchLong ? mr.targetBranch : ''"
-            class="label-branch"
-            data-placement="bottom"
+          <tooltip-on-truncate
+            :title="mr.targetBranch"
+            truncate-target="child"
+            class="label-branch label-truncate"
           >
             <a
               :href="mr.targetBranchTreePath"
@@ -96,15 +79,13 @@ export default {
             >
               {{ mr.targetBranch }}
             </a>
-          </span>
+          </tooltip-on-truncate>
         </strong>
         <div
           v-if="shouldShowCommitsBehindText"
           class="diverged-commits-count"
+          v-html="commitsBehindText"
         >
-          <span class="monospace">{{ mr.sourceBranch }}</span>
-          is {{ commitsText }}
-          <span class="monospace">{{ mr.targetBranch }}</span>
         </div>
       </div>
 
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue
index 4a3fd01fa39..fee41b239e8 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue
@@ -3,6 +3,7 @@
 import PipelineStage from '~/pipelines/components/stage.vue';
 import CiIcon from '~/vue_shared/components/ci_icon.vue';
 import Icon from '~/vue_shared/components/icon.vue';
+import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue';
 
 export default {
   name: 'MRWidgetPipeline',
@@ -10,6 +11,7 @@ export default {
     PipelineStage,
     CiIcon,
     Icon,
+    TooltipOnTruncate,
   },
   props: {
     pipeline: {
@@ -30,6 +32,10 @@ export default {
       type: String,
       required: false,
     },
+    sourceBranch: {
+      type: String,
+      required: false,
+    },
   },
   computed: {
     hasPipeline() {
@@ -107,11 +113,12 @@ export default {
                   >
                     {{ pipeline.commit.short_id }}</a>
                   on
-                  <span
-                    class="label-branch"
+                  <tooltip-on-truncate
+                    :title="sourceBranch"
+                    truncate-target="child"
+                    class="label-branch label-truncate"
                     v-html="sourceBranchLink"
-                  >
-                  </span>
+                  />
                 </template>
               </div>
               <div
diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
index 80593d1f34a..dc6be025f11 100644
--- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
@@ -254,6 +254,7 @@ export default {
       :pipeline="mr.pipeline"
       :ci-status="mr.ciStatus"
       :has-ci="mr.hasCI"
+      :source-branch="mr.sourceBranch"
       :source-branch-link="mr.sourceBranchLink"
     />
     <deployment
diff --git a/app/assets/javascripts/vue_shared/components/tooltip_on_truncate.vue b/app/assets/javascripts/vue_shared/components/tooltip_on_truncate.vue
new file mode 100644
index 00000000000..125826da6c3
--- /dev/null
+++ b/app/assets/javascripts/vue_shared/components/tooltip_on_truncate.vue
@@ -0,0 +1,67 @@
+<script>
+import _ from 'underscore';
+import tooltip from '../directives/tooltip';
+
+export default {
+  directives: {
+    tooltip,
+  },
+  props: {
+    title: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    placement: {
+      type: String,
+      required: false,
+      default: 'top',
+    },
+    truncateTarget: {
+      type: [String, Function],
+      required: false,
+      default: '',
+    },
+  },
+  data() {
+    return {
+      showTooltip: false,
+    };
+  },
+  mounted() {
+    const target = this.selectTarget();
+
+    if (target && target.scrollWidth > target.offsetWidth) {
+      this.showTooltip = true;
+    }
+  },
+  methods: {
+    selectTarget() {
+      if (_.isFunction(this.truncateTarget)) {
+        return this.truncateTarget(this.$el);
+      } else if (this.truncateTarget === 'child') {
+        return this.$el.childNodes[0];
+      }
+
+      return this.$el;
+    },
+  },
+};
+</script>
+
+<template>
+  <span
+    v-tooltip
+    v-if="showTooltip"
+    :title="title"
+    :data-placement="placement"
+    class="js-show-tooltip"
+  >
+    <slot></slot>
+  </span>
+  <span
+    v-else
+  >
+    <slot></slot>
+  </span>
+</template>
diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index 5631d943984..7b8cad254c7 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -195,6 +195,7 @@
     .ci-widget-content {
       display: flex;
       align-items: center;
+      flex: 1;
     }
   }
 
@@ -222,6 +223,7 @@
 
   .normal {
     flex: 1;
+    flex-basis: auto;
   }
 
   .capitalize {
@@ -235,22 +237,23 @@
     font-weight: normal;
     overflow: hidden;
     word-break: break-all;
+  }
 
-    &.label-truncated {
-      position: relative;
-      display: inline-block;
-      width: 250px;
-      margin-bottom: -3px;
-      white-space: nowrap;
-      text-overflow: clip;
-      line-height: 14px;
-
-      &::after {
-        position: absolute;
-        content: '...';
-        right: 0;
-        font-family: $regular-font;
-        background-color: $gray-light;
+  .deploy-link,
+  .label-branch {
+    &.label-truncate {
+      // NOTE: This selector targets its children because some of the HTML comes from
+      // 'source_branch_link'. Once this external HTML is no longer used, we could
+      // simplify this.
+      > a,
+      > span {
+        display: inline-block;
+        max-width: 12.5em;
+        margin-bottom: -3px;
+        white-space: nowrap;
+        text-overflow: ellipsis;
+        line-height: 14px;
+        overflow: hidden;
       }
     }
   }
@@ -582,7 +585,7 @@
 
     @include media-breakpoint-down(md) {
       flex-direction: column;
-      align-items: flex-start;
+      align-items: stretch;
 
       .branch-actions {
         margin-top: 16px;
@@ -593,13 +596,13 @@
       .branch-actions {
         align-self: center;
         margin-left: $gl-padding;
+        white-space: nowrap;
       }
     }
   }
 
   .diverged-commits-count {
     color: $gl-text-color-secondary;
-    font-size: 12px;
   }
 }
 
@@ -918,7 +921,7 @@
   flex: 1;
   flex-direction: row;
 
-  @include media-breakpoint-down(md) {
+  @include media-breakpoint-down(sm) {
     flex-direction: column;
 
     .stage-cell .stage-container {
diff --git a/changelogs/unreleased/49110-update-mr-widget-styles.yml b/changelogs/unreleased/49110-update-mr-widget-styles.yml
new file mode 100644
index 00000000000..e54866a0908
--- /dev/null
+++ b/changelogs/unreleased/49110-update-mr-widget-styles.yml
@@ -0,0 +1,5 @@
+---
+title: Truncate branch names and update "commits behind" text in MR page
+merge_request: 21206
+author:
+type: changed
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index f348f66ae17..0bf552d6084 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -6706,6 +6706,9 @@ msgstr ""
 msgid "mrWidget|The source branch has been removed"
 msgstr ""
 
+msgid "mrWidget|The source branch is %{commitsBehindLinkStart}%{commitsBehind}%{commitsBehindLinkEnd} the target branch"
+msgstr ""
+
 msgid "mrWidget|The source branch is being removed"
 msgstr ""
 
diff --git a/spec/javascripts/helpers/vue_mount_component_helper.js b/spec/javascripts/helpers/vue_mount_component_helper.js
index 1057f0aca3e..6848c95d95d 100644
--- a/spec/javascripts/helpers/vue_mount_component_helper.js
+++ b/spec/javascripts/helpers/vue_mount_component_helper.js
@@ -1,3 +1,5 @@
+import Vue from 'vue';
+
 const mountComponent = (Component, props = {}, el = null) =>
   new Component({
     propsData: props,
@@ -25,4 +27,12 @@ export const mountComponentWithSlots = (Component, { props, slots }) => {
   return component.$mount();
 };
 
+/**
+ * Mount a component with the given render method.
+ *
+ * This helps with inserting slots that need to be compiled.
+ */
+export const mountComponentWithRender = (render, el = null) =>
+  mountComponent(Vue.extend({ render }), {}, el);
+
 export default mountComponent;
diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_header_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_header_spec.js
index 8ac2f26979b..0e42537faed 100644
--- a/spec/javascripts/vue_mr_widget/components/mr_widget_header_spec.js
+++ b/spec/javascripts/vue_mr_widget/components/mr_widget_header_spec.js
@@ -45,7 +45,7 @@ describe('MRWidgetHeader', () => {
       });
     });
 
-    describe('commitsText', () => {
+    describe('commitsBehindText', () => {
       it('returns singular when there is one commit', () => {
         vm = mountComponent(Component, {
           mr: {
@@ -53,11 +53,12 @@ describe('MRWidgetHeader', () => {
             sourceBranch: 'mr-widget-refactor',
             sourceBranchLink: '<a href="/foo/bar/mr-widget-refactor">Link</a>',
             targetBranch: 'master',
+            targetBranchPath: '/foo/bar/master',
             statusPath: 'abc',
           },
         });
 
-        expect(vm.commitsText).toEqual('1 commit behind');
+        expect(vm.commitsBehindText).toEqual('The source branch is <a href="/foo/bar/master">1 commit behind</a> the target branch');
       });
 
       it('returns plural when there is more than one commit', () => {
@@ -67,11 +68,12 @@ describe('MRWidgetHeader', () => {
             sourceBranch: 'mr-widget-refactor',
             sourceBranchLink: '<a href="/foo/bar/mr-widget-refactor">Link</a>',
             targetBranch: 'master',
+            targetBranchPath: '/foo/bar/master',
             statusPath: 'abc',
           },
         });
 
-        expect(vm.commitsText).toEqual('2 commits behind');
+        expect(vm.commitsBehindText).toEqual('The source branch is <a href="/foo/bar/master">2 commits behind</a> the target branch');
       });
     });
   });
@@ -280,9 +282,9 @@ describe('MRWidgetHeader', () => {
       });
 
       it('renders diverged commits info', () => {
-        expect(vm.$el.querySelector('.diverged-commits-count').textContent).toMatch(
-          /(mr-widget-refactor[\s\S]+?is 12 commits behind[\s\S]+?master)/,
-        );
+        expect(vm.$el.querySelector('.diverged-commits-count').textContent).toEqual('The source branch is 12 commits behind the target branch');
+        expect(vm.$el.querySelector('.diverged-commits-count a').textContent).toEqual('12 commits behind');
+        expect(vm.$el.querySelector('.diverged-commits-count a')).toHaveAttr('href', vm.mr.targetBranchPath);
       });
     });
   });
diff --git a/spec/javascripts/vue_shared/components/tooltip_on_truncate_spec.js b/spec/javascripts/vue_shared/components/tooltip_on_truncate_spec.js
new file mode 100644
index 00000000000..8465757deb6
--- /dev/null
+++ b/spec/javascripts/vue_shared/components/tooltip_on_truncate_spec.js
@@ -0,0 +1,162 @@
+import { mountComponentWithRender } from 'spec/helpers/vue_mount_component_helper';
+import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue';
+
+const TEST_TITLE = 'lorem-ipsum-dolar-sit-amit-consectur-adipiscing-elit-sed-do';
+const CLASS_SHOW_TOOLTIP = 'js-show-tooltip';
+const STYLE_TRUNCATED = {
+  display: 'inline-block',
+  'max-width': '20px',
+};
+const STYLE_NORMAL = {
+  display: 'inline-block',
+  'max-width': '1000px',
+};
+
+function mountTooltipOnTruncate(options, createChildren) {
+  return mountComponentWithRender(h => h(TooltipOnTruncate, options, createChildren(h)), '#app');
+}
+
+describe('TooltipOnTruncate component', () => {
+  let vm;
+
+  beforeEach(() => {
+    const el = document.createElement('div');
+    el.id = 'app';
+    document.body.appendChild(el);
+  });
+
+  afterEach(() => {
+    vm.$destroy();
+  });
+
+  describe('with default target', () => {
+    it('renders tooltip if truncated', done => {
+      const options = {
+        style: STYLE_TRUNCATED,
+        props: {
+          title: TEST_TITLE,
+        },
+      };
+
+      vm = mountTooltipOnTruncate(options, () => [TEST_TITLE]);
+
+      vm.$nextTick()
+        .then(() => {
+          expect(vm.$el).toHaveClass(CLASS_SHOW_TOOLTIP);
+          expect(vm.$el).toHaveData('original-title', TEST_TITLE);
+          expect(vm.$el).toHaveData('placement', 'top');
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    it('does not render tooltip if normal', done => {
+      const options = {
+        style: STYLE_NORMAL,
+        props: {
+          title: TEST_TITLE,
+        },
+      };
+
+      vm = mountTooltipOnTruncate(options, () => [TEST_TITLE]);
+
+      vm.$nextTick()
+        .then(() => {
+          expect(vm.$el).not.toHaveClass(CLASS_SHOW_TOOLTIP);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+  });
+
+  describe('with child target', () => {
+    it('renders tooltip if truncated', done => {
+      const options = {
+        style: STYLE_NORMAL,
+        props: {
+          title: TEST_TITLE,
+          truncateTarget: 'child',
+        },
+      };
+
+      vm = mountTooltipOnTruncate(options, (h) => [
+        h('a', { style: STYLE_TRUNCATED }, TEST_TITLE),
+      ]);
+
+      vm.$nextTick()
+        .then(() => {
+          expect(vm.$el).toHaveClass(CLASS_SHOW_TOOLTIP);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    it('does not render tooltip if normal', done => {
+      const options = {
+        props: {
+          title: TEST_TITLE,
+          truncateTarget: 'child',
+        },
+      };
+
+      vm = mountTooltipOnTruncate(options, (h) => [
+        h('a', { style: STYLE_NORMAL }, TEST_TITLE),
+      ]);
+
+      vm.$nextTick()
+        .then(() => {
+          expect(vm.$el).not.toHaveClass(CLASS_SHOW_TOOLTIP);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+  });
+
+  describe('with fn target', () => {
+    it('renders tooltip if truncated', done => {
+      const options = {
+        style: STYLE_NORMAL,
+        props: {
+          title: TEST_TITLE,
+          truncateTarget: (el) => el.childNodes[1],
+        },
+      };
+
+      vm = mountTooltipOnTruncate(options, (h) => [
+        h('a', { style: STYLE_NORMAL }, TEST_TITLE),
+        h('span', { style: STYLE_TRUNCATED }, TEST_TITLE),
+      ]);
+
+      vm.$nextTick()
+        .then(() => {
+          expect(vm.$el).toHaveClass(CLASS_SHOW_TOOLTIP);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+  });
+
+  describe('placement', () => {
+    it('sets data-placement when tooltip is rendered', done => {
+      const options = {
+        props: {
+          title: TEST_TITLE,
+          truncateTarget: 'child',
+          placement: 'bottom',
+        },
+      };
+
+      vm = mountTooltipOnTruncate(options, (h) => [
+        h('a', { style: STYLE_TRUNCATED }, TEST_TITLE),
+      ]);
+
+      vm.$nextTick()
+        .then(() => {
+          expect(vm.$el).toHaveClass(CLASS_SHOW_TOOLTIP);
+          expect(vm.$el).toHaveData('placement', options.props.placement);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+  });
+});