Performance Improvements to Vue MR page

5 files changed, 116 insertions, 80 deletions

diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue
index e0aecda54f6..eb0985e5603 100644
--- a/app/assets/javascripts/diffs/components/app.vue
+++ b/app/assets/javascripts/diffs/components/app.vue
@@ -145,7 +145,7 @@ export default {
 </script>
 
 <template>
-  <div v-if="shouldShow">
+  <div v-show="shouldShow">
     <div
       v-if="isLoading"
       class="loading"
diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js
index 7d6f1000ebe..53d7504de35 100644
--- a/app/assets/javascripts/merge_request_tabs.js
+++ b/app/assets/javascripts/merge_request_tabs.js
@@ -69,12 +69,23 @@ let { location } = window;
 
 export default class MergeRequestTabs {
   constructor({ action, setUrl, stubLocation } = {}) {
-    const mergeRequestTabs = document.querySelector('.js-tabs-affix');
+    this.mergeRequestTabs = document.querySelector('.merge-request-tabs-container');
+    this.mergeRequestTabsAll =
+      this.mergeRequestTabs && this.mergeRequestTabs.querySelectorAll
+        ? this.mergeRequestTabs.querySelectorAll('.merge-request-tabs li')
+        : null;
+    this.mergeRequestTabPanes = document.querySelector('#diff-notes-app');
+    this.mergeRequestTabPanesAll =
+      this.mergeRequestTabPanes && this.mergeRequestTabPanes.querySelectorAll
+        ? this.mergeRequestTabPanes.querySelectorAll('.tab-pane')
+        : null;
     const navbar = document.querySelector('.navbar-gitlab');
     const peek = document.getElementById('js-peek');
     const paddingTop = 16;
+
     this.commitsTab = document.querySelector('.tab-content .commits.tab-pane');
 
+    this.currentTab = null;
     this.diffsLoaded = false;
     this.pipelinesLoaded = false;
     this.commitsLoaded = false;
@@ -84,15 +95,15 @@ export default class MergeRequestTabs {
     this.setUrl = setUrl !== undefined ? setUrl : true;
     this.setCurrentAction = this.setCurrentAction.bind(this);
     this.tabShown = this.tabShown.bind(this);
-    this.showTab = this.showTab.bind(this);
+    this.clickTab = this.clickTab.bind(this);
     this.stickyTop = navbar ? navbar.offsetHeight - paddingTop : 0;
 
     if (peek) {
       this.stickyTop += peek.offsetHeight;
     }
 
-    if (mergeRequestTabs) {
-      this.stickyTop += mergeRequestTabs.offsetHeight;
+    if (this.mergeRequestTabs) {
+      this.stickyTop += this.mergeRequestTabs.offsetHeight;
     }
 
     if (stubLocation) {
@@ -100,25 +111,22 @@ export default class MergeRequestTabs {
     }
 
     this.bindEvents();
-    this.activateTab(action);
+    if (
+      this.mergeRequestTabs &&
+      this.mergeRequestTabs.querySelector(`a[data-action='${action}']`) &&
+      this.mergeRequestTabs.querySelector(`a[data-action='${action}']`).click
+    )
+      this.mergeRequestTabs.querySelector(`a[data-action='${action}']`).click();
     this.initAffix();
   }
 
   bindEvents() {
-    $(document)
-      .on('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown)
-      .on('click', '.js-show-tab', this.showTab);
-
-    $('.merge-request-tabs a[data-toggle="tab"]').on('click', this.clickTab);
+    $('.merge-request-tabs a[data-toggle="tabvue"]').on('click', this.clickTab);
   }
 
   // Used in tests
   unbindEvents() {
-    $(document)
-      .off('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown)
-      .off('click', '.js-show-tab', this.showTab);
-
-    $('.merge-request-tabs a[data-toggle="tab"]').off('click', this.clickTab);
+    $('.merge-request-tabs a[data-toggle="tabvue"]').off('click', this.clickTab);
   }
 
   destroyPipelinesView() {
@@ -130,58 +138,87 @@ export default class MergeRequestTabs {
     }
   }
 
-  showTab(e) {
-    e.preventDefault();
-    this.activateTab($(e.target).data('action'));
-  }
-
   clickTab(e) {
-    if (e.currentTarget && isMetaClick(e)) {
-      const targetLink = e.currentTarget.getAttribute('href');
+    if (e.currentTarget) {
       e.stopImmediatePropagation();
       e.preventDefault();
-      window.open(targetLink, '_blank');
+
+      const { action } = e.currentTarget.dataset;
+
+      if (action) {
+        const href = e.currentTarget.getAttribute('href');
+        this.tabShown(action, href);
+      } else if (isMetaClick(e)) {
+        const targetLink = e.currentTarget.getAttribute('href');
+        window.open(targetLink, '_blank');
+      }
     }
   }
 
-  tabShown(e) {
-    const $target = $(e.target);
-    const action = $target.data('action');
-
-    if (action === 'commits') {
-      this.loadCommits($target.attr('href'));
-      this.expandView();
-      this.resetViewContainer();
-      this.destroyPipelinesView();
-    } else if (this.isDiffAction(action)) {
-      if (!isInVueNoteablePage()) {
-        this.loadDiff($target.attr('href'));
-      }
-      if (bp.getBreakpointSize() !== 'lg') {
-        this.shrinkView();
+  tabShown(action, href) {
+    if (action !== this.currentTab && this.mergeRequestTabs) {
+      this.currentTab = action;
+
+      if (this.mergeRequestTabPanesAll) {
+        this.mergeRequestTabPanesAll.forEach(el => {
+          const tabPane = el;
+          tabPane.style.display = 'none';
+        });
       }
-      if (this.diffViewType() === 'parallel') {
-        this.expandViewContainer();
+
+      if (this.mergeRequestTabsAll) {
+        this.mergeRequestTabsAll.forEach(el => {
+          el.classList.remove('active');
+        });
       }
-      this.destroyPipelinesView();
-      this.commitsTab.classList.remove('active');
-    } else if (action === 'pipelines') {
-      this.resetViewContainer();
-      this.mountPipelinesView();
-    } else {
-      if (bp.getBreakpointSize() !== 'xs') {
+
+      const tabPane = this.mergeRequestTabPanes.querySelector(`#${action}`);
+      if (tabPane) tabPane.style.display = 'block';
+      const tab = this.mergeRequestTabs.querySelector(`.${action}-tab`);
+      if (tab) tab.classList.add('active');
+
+      if (action === 'commits') {
+        this.loadCommits(href);
+        this.expandView();
+        this.resetViewContainer();
+        this.destroyPipelinesView();
+      } else if (action === 'new') {
         this.expandView();
+        this.resetViewContainer();
+        this.destroyPipelinesView();
+      } else if (this.isDiffAction(action)) {
+        if (!isInVueNoteablePage()) {
+          this.loadDiff(href);
+        }
+        if (bp.getBreakpointSize() !== 'lg') {
+          this.shrinkView();
+        }
+        if (this.diffViewType() === 'parallel') {
+          this.expandViewContainer();
+        }
+        this.destroyPipelinesView();
+        this.commitsTab.classList.remove('active');
+      } else if (action === 'pipelines') {
+        this.resetViewContainer();
+        this.mountPipelinesView();
+      } else {
+        this.mergeRequestTabPanes.querySelector('#notes').style.display = 'block';
+        this.mergeRequestTabs.querySelector('.notes-tab').classList.add('active');
+
+        if (bp.getBreakpointSize() !== 'xs') {
+          this.expandView();
+        }
+        this.resetViewContainer();
+        this.destroyPipelinesView();
+
+        initDiscussionTab();
+      }
+      if (this.setUrl) {
+        this.setCurrentAction(action);
       }
-      this.resetViewContainer();
-      this.destroyPipelinesView();
 
-      initDiscussionTab();
-    }
-    if (this.setUrl) {
-      this.setCurrentAction(action);
+      this.eventHub.$emit('MergeRequestTabChange', this.getCurrentAction());
     }
-
-    this.eventHub.$emit('MergeRequestTabChange', this.getCurrentAction());
   }
 
   scrollToElement(container) {
@@ -194,12 +231,6 @@ export default class MergeRequestTabs {
     }
   }
 
-  // Activate a tab based on the current action
-  activateTab(action) {
-    // important note: the .tab('show') method triggers 'shown.bs.tab' event itself
-    $(`.merge-request-tabs a[data-action='${action}']`).tab('show');
-  }
-
   // Replaces the current Merge Request-specific action in the URL with a new one
   //
   // If the action is "notes", the URL is reset to the standard
diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue
index 7853847fc37..a8995021699 100644
--- a/app/assets/javascripts/notes/components/notes_app.vue
+++ b/app/assets/javascripts/notes/components/notes_app.vue
@@ -175,7 +175,7 @@ export default {
 
 <template>
   <div
-    v-if="shouldShow"
+    v-show="shouldShow"
     id="notes"
   >
     <ul
diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
index 82a7931c557..097be8a0643 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -108,7 +108,7 @@ module MergeRequestsHelper
     data_attrs = {
       action: tab.to_s,
       target: "##{tab}",
-      toggle: options.fetch(:force_link, false) ? '' : 'tab'
+      toggle: options.fetch(:force_link, false) ? '' : 'tabvue'
     }
 
     url = case tab
diff --git a/app/views/projects/merge_requests/creations/_new_submit.html.haml b/app/views/projects/merge_requests/creations/_new_submit.html.haml
index b2eacabc21a..f7a5d85500f 100644
--- a/app/views/projects/merge_requests/creations/_new_submit.html.haml
+++ b/app/views/projects/merge_requests/creations/_new_submit.html.haml
@@ -24,23 +24,28 @@
         There are no commits yet.
       = custom_icon ('illustration_no_commits')
   - else
-    %ul.merge-request-tabs.nav.nav-tabs.nav-links.no-top.no-bottom
-      %li.commits-tab
-        = link_to url_for(safe_params), data: {target: 'div#commits', action: 'new', toggle: 'tab'} do
-          Commits
-          %span.badge.badge-pill= @commits.size
-      - if @pipelines.any?
-        %li.builds-tab
-          = link_to url_for(safe_params.merge(action: 'pipelines')), data: {target: 'div#pipelines', action: 'pipelines', toggle: 'tab'} do
-            Pipelines
-            %span.badge.badge-pill= @pipelines.size
-      %li.diffs-tab
-        = link_to url_for(safe_params.merge(action: 'diffs')), data: {target: 'div#diffs', action: 'diffs', toggle: 'tab'} do
-          Changes
-          %span.badge.badge-pill= @merge_request.diff_size
+    .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.nav-tabs.nav-links.no-top.no-bottom.js-tabs-affix
+            %li.commits-tab.new-tab
+              = link_to url_for(safe_params), data: {target: 'div#commits', action: 'new', toggle: 'tabvue'} do
+                Commits
+                %span.badge.badge-pill= @commits.size
+            - if @pipelines.any?
+              %li.builds-tab
+                = link_to url_for(safe_params.merge(action: 'pipelines')), data: {target: 'div#pipelines', action: 'pipelines', toggle: 'tabvue'} do
+                  Pipelines
+                  %span.badge.badge-pill= @pipelines.size
+            %li.diffs-tab
+              = link_to url_for(safe_params.merge(action: 'diffs')), data: {target: 'div#diffs', action: 'diffs', toggle: 'tabvue'} do
+                Changes
+                %span.badge.badge-pill= @merge_request.diff_size
 
-    .tab-content
-      #commits.commits.tab-pane.active
+    #diff-notes-app.tab-content
+      #new.commits.tab-pane.active
         = render "projects/merge_requests/commits"
       #diffs.diffs.tab-pane
         -# This tab is always loaded via AJAX