Merge branch '19183-refactor-sidebar-js' into 'master'

Refactor sidebar logic for more predictable behavior

## What does this MR do?

Fixes a few bugs with the sidebar and "pin" functionality:

1. Pinned state would get reset when loading a page with a viewport smaller than 1024px (#19183)
2. Toggle buttons could occasionally end up in an invalid state in which the toggle is hidden from view at the same time the sidebar is collapsed.
    ![2016-09-03-09.39.07](/uploads/75d9a836ab58aae9f89f38cc29e90bbd/2016-09-03-09.39.07.gif)

3. Clicking outside the sidebar to trigger 'collapse' when below the 1024px breakpoint would work only if not pinned, even though pin status should only effect the sidebar above the 1024px breakpoint.
4. Code confusing with no single source of truth for the state of the sidebar.  Sometimes pinned state is inferred from the cookie, sometimes from the DOM.  Code to handle these functions was confusingly split across both `sidebar.js` and `application.js` for no apparent reason.

I've created a singleton ES6 class to handle the sidebar DOM manipulations, using the properties `isExpanded` and `isPinned` as the canonical state and providing a `renderState` method to sync the DOM with this state whenever it needs updating.  This avoids the possibility of invalid states caused by reliance on jQuery `toggleClass()` methods and makes the code much more readable/maintainable.

## Are there points in the code the reviewer needs to double check?

It is a substantial rewrite, so I could use another set of eyes to make sure nothing was left behind from the original implementation.

## Why was this MR needed?

I initially intended to just fix #19183 by modifying the code in place, but it proved to be a difficult mess and rather than add to the technical debt it made sense to write a more readable implementation of the sidebar functionality.

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- Tests
  - [ ] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

## What are the relevant issue numbers?

Closes #19183

See merge request !6169

10 files changed, 104 insertions, 148 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 1148ad10dca..84e12901023 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -14,6 +14,7 @@ v 8.12.0 (unreleased)
   - Instructions for enabling Git packfile bitmaps !6104
   - Fix pagination on user snippets page
   - Escape search term before passing it to Regexp.new !6241 (winniehell)
+  - Fix pinned sidebar behavior in smaller viewports !6169
   - Change merge_error column from string to text type
   - Reduce contributions calendar data payload (ClemMakesApps)
   - Add `web_url` field to issue, merge request, and snippet API objects (Ben Boeckel)
diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js
index 43a679501a7..fea6f41d5e9 100644
--- a/app/assets/javascripts/application.js
+++ b/app/assets/javascripts/application.js
@@ -174,9 +174,7 @@
     $body.tooltip({
       selector: '.has-tooltip, [data-toggle="tooltip"]',
       placement: function(_, el) {
-        var $el;
-        $el = $(el);
-        return $el.data('placement') || 'bottom';
+        return $(el).data('placement') || 'bottom';
       }
     });
     $('.trigger-submit').on('change', function() {
@@ -286,42 +284,9 @@
     gl.awardsHandler = new AwardsHandler();
     checkInitialSidebarSize();
     new Aside();
-    if ($window.width() < 1024 && $.cookie('pin_nav') === 'true') {
-      $.cookie('pin_nav', 'false', {
-        path: gon.relative_url_root || '/',
-        expires: 365 * 10
-      });
-      $('.page-with-sidebar').toggleClass('page-sidebar-collapsed page-sidebar-expanded').removeClass('page-sidebar-pinned');
-      $('.navbar-fixed-top').removeClass('header-pinned-nav');
-    }
-    $document.off('click', '.js-nav-pin').on('click', '.js-nav-pin', function(e) {
-      var $page, $pinBtn, $tooltip, $topNav, doPinNav, tooltipText;
-      e.preventDefault();
-      $pinBtn = $(e.currentTarget);
-      $page = $('.page-with-sidebar');
-      $topNav = $('.navbar-fixed-top');
-      $tooltip = $("#" + ($pinBtn.attr('aria-describedby')));
-      doPinNav = !$page.is('.page-sidebar-pinned');
-      tooltipText = 'Pin navigation';
-      $(this).toggleClass('is-active');
-      if (doPinNav) {
-        $page.addClass('page-sidebar-pinned');
-        $topNav.addClass('header-pinned-nav');
-      } else {
-        $tooltip.remove();
-        $page.removeClass('page-sidebar-pinned').toggleClass('page-sidebar-collapsed page-sidebar-expanded');
-        $topNav.removeClass('header-pinned-nav').toggleClass('header-collapsed header-expanded');
-      }
-      $.cookie('pin_nav', doPinNav, {
-        path: gon.relative_url_root || '/',
-        expires: 365 * 10
-      });
-      if ($.cookie('pin_nav') === 'true' || doPinNav) {
-        tooltipText = 'Unpin navigation';
-      }
-      $tooltip.find('.tooltip-inner').text(tooltipText);
-      return $pinBtn.attr('title', tooltipText).tooltip('fixTitle');
-    });
+
+    // bind sidebar events
+    new gl.Sidebar();
 
     // Custom time ago
     gl.utils.shortTimeAgo($('.js-short-timeago'));
diff --git a/app/assets/javascripts/sidebar.js b/app/assets/javascripts/sidebar.js
deleted file mode 100644
index bd0c1194b36..00000000000
--- a/app/assets/javascripts/sidebar.js
+++ /dev/null
@@ -1,41 +0,0 @@
-(function() {
-  var collapsed, expanded, toggleSidebar;
-
-  collapsed = 'page-sidebar-collapsed';
-
-  expanded = 'page-sidebar-expanded';
-
-  toggleSidebar = function() {
-    $('.page-with-sidebar').toggleClass(collapsed + " " + expanded);
-    $('.navbar-fixed-top').toggleClass("header-collapsed header-expanded");
-    if ($.cookie('pin_nav') === 'true') {
-      $('.navbar-fixed-top').toggleClass('header-pinned-nav');
-      $('.page-with-sidebar').toggleClass('page-sidebar-pinned');
-    }
-    return setTimeout((function() {
-      var niceScrollBars;
-      niceScrollBars = $('.nav-sidebar').niceScroll();
-      return niceScrollBars.updateScrollBar();
-    }), 300);
-  };
-
-  $(document).off('click', 'body').on('click', 'body', function(e) {
-    var $nav, $target, $toggle, pageExpanded;
-    if ($.cookie('pin_nav') !== 'true') {
-      $target = $(e.target);
-      $nav = $target.closest('.sidebar-wrapper');
-      pageExpanded = $('.page-with-sidebar').hasClass('page-sidebar-expanded');
-      $toggle = $target.closest('.toggle-nav-collapse, .side-nav-toggle');
-      if ($nav.length === 0 && pageExpanded && $toggle.length === 0) {
-        $('.page-with-sidebar').toggleClass('page-sidebar-collapsed page-sidebar-expanded');
-        return $('.navbar-fixed-top').toggleClass('header-collapsed header-expanded');
-      }
-    }
-  });
-
-  $(document).on("click", '.toggle-nav-collapse, .side-nav-toggle', function(e) {
-    e.preventDefault();
-    return toggleSidebar();
-  });
-
-}).call(this);
diff --git a/app/assets/javascripts/sidebar.js.es6 b/app/assets/javascripts/sidebar.js.es6
new file mode 100644
index 00000000000..755fac8107b
--- /dev/null
+++ b/app/assets/javascripts/sidebar.js.es6
@@ -0,0 +1,93 @@
+((global) => {
+  let singleton;
+
+  const pinnedStateCookie = 'pin_nav';
+  const sidebarBreakpoint = 1024;
+
+  const pageSelector = '.page-with-sidebar';
+  const navbarSelector = '.navbar-fixed-top';
+  const sidebarWrapperSelector = '.sidebar-wrapper';
+  const sidebarContentSelector = '.nav-sidebar';
+
+  const pinnedToggleSelector = '.js-nav-pin';
+  const sidebarToggleSelector = '.toggle-nav-collapse, .side-nav-toggle';
+
+  const pinnedPageClass = 'page-sidebar-pinned';
+  const expandedPageClass = 'page-sidebar-expanded';
+
+  const pinnedNavbarClass = 'header-sidebar-pinned';
+  const expandedNavbarClass = 'header-sidebar-expanded';
+
+  class Sidebar {
+    constructor() {
+      if (!singleton) {
+        singleton = this;
+        singleton.init();
+      }
+      return singleton;
+    }
+
+    init() {
+      this.isPinned = $.cookie(pinnedStateCookie) === 'true';
+      this.isExpanded = (
+        window.innerWidth >= sidebarBreakpoint &&
+        $(pageSelector).hasClass(expandedPageClass)
+      );
+      $(document)
+        .on('click', sidebarToggleSelector, () => this.toggleSidebar())
+        .on('click', pinnedToggleSelector, () => this.togglePinnedState())
+        .on('click', 'html, body', (e) => this.handleClickEvent(e))
+        .on('page:change', () => this.renderState());
+      this.renderState();
+    }
+
+    handleClickEvent(e) {
+      if (this.isExpanded && (!this.isPinned || window.innerWidth < sidebarBreakpoint)) {
+        const $target = $(e.target);
+        const targetIsToggle = $target.closest(sidebarToggleSelector).length > 0;
+        const targetIsSidebar = $target.closest(sidebarWrapperSelector).length > 0;
+        if (!targetIsToggle && (!targetIsSidebar || $target.closest('a'))) {
+          this.toggleSidebar();
+        }
+      }
+    }
+
+    toggleSidebar() {
+      this.isExpanded = !this.isExpanded;
+      this.renderState();
+    }
+
+    togglePinnedState() {
+      this.isPinned = !this.isPinned;
+      if (!this.isPinned) {
+        this.isExpanded = false;
+      }
+      $.cookie(pinnedStateCookie, this.isPinned ? 'true' : 'false', {
+        path: gon.relative_url_root || '/',
+        expires: 3650
+      });
+      this.renderState();
+    }
+
+    renderState() {
+      $(pageSelector)
+        .toggleClass(pinnedPageClass, this.isPinned && this.isExpanded)
+        .toggleClass(expandedPageClass, this.isExpanded);
+      $(navbarSelector)
+        .toggleClass(pinnedNavbarClass, this.isPinned && this.isExpanded)
+        .toggleClass(expandedNavbarClass, this.isExpanded);
+
+      const $pinnedToggle = $(pinnedToggleSelector);
+      const tooltipText = this.isPinned ? 'Unpin navigation' : 'Pin navigation';
+      const tooltipState = $pinnedToggle.attr('aria-describedby') && this.isExpanded ? 'show' : 'hide';
+      $pinnedToggle.attr('title', tooltipText).tooltip('fixTitle').tooltip(tooltipState);
+
+      if (this.isExpanded) {
+        setTimeout(() => $(sidebarContentSelector).niceScroll().updateScrollBar(), 200);
+      }
+    }
+  }
+
+  global.Sidebar = Sidebar;
+
+})(window.gl || (window.gl = {}));
diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss
index 1036219172e..d4a030f7f7a 100644
--- a/app/assets/stylesheets/framework/header.scss
+++ b/app/assets/stylesheets/framework/header.scss
@@ -77,10 +77,6 @@ header {
       }
     }
 
-    &.header-collapsed {
-      padding: 0 16px;
-    }
-
     .side-nav-toggle {
       position: absolute;
       left: -10px;
diff --git a/app/assets/stylesheets/framework/sidebar.scss b/app/assets/stylesheets/framework/sidebar.scss
index 015fe3debf9..3b7de4b57bb 100644
--- a/app/assets/stylesheets/framework/sidebar.scss
+++ b/app/assets/stylesheets/framework/sidebar.scss
@@ -1,6 +1,5 @@
 .page-with-sidebar {
-  padding-top: $header-height;
-  padding-bottom: 25px;
+  padding: $header-height 0 25px;
   transition: padding $sidebar-transition-duration;
 
   &.page-sidebar-pinned {
@@ -15,6 +14,7 @@
     bottom: 0;
     left: 0;
     height: 100%;
+    width: 0;
     overflow: hidden;
     transition: width $sidebar-transition-duration;
     @include box-shadow(2px 0 16px 0 $black-transparent);
@@ -128,10 +128,8 @@
 
     .fa {
       transition: transform .15s;
-    }
 
-    &.is-active {
-      .fa {
+      .page-sidebar-pinned & {
         transform: rotate(90deg);
       }
     }
@@ -152,14 +150,6 @@
   }
 }
 
-.page-sidebar-collapsed {
-  padding-left: 0;
-
-  .sidebar-wrapper {
-    width: 0;
-  }
-}
-
 .page-sidebar-expanded {
   .sidebar-wrapper {
     width: $sidebar_width;
@@ -175,7 +165,7 @@
   }
 }
 
-header.header-pinned-nav {
+header.header-sidebar-pinned {
   @media (min-width: $sidebar-breakpoint) {
     padding-left: ($sidebar_width + $gl-padding);
 
diff --git a/app/assets/stylesheets/pages/builds.scss b/app/assets/stylesheets/pages/builds.scss
index 614405aa5c1..c879074c7fe 100644
--- a/app/assets/stylesheets/pages/builds.scss
+++ b/app/assets/stylesheets/pages/builds.scss
@@ -48,12 +48,6 @@
       margin-bottom: 10px;
     }
   }
-
-  .page-sidebar-collapsed {
-    .scroll-controls {
-      left: 70px;
-    }
-  }
 }
 
 .build-header {
diff --git a/app/helpers/nav_helper.rb b/app/helpers/nav_helper.rb
index 2b0ff6c0d00..df87fac132d 100644
--- a/app/helpers/nav_helper.rb
+++ b/app/helpers/nav_helper.rb
@@ -1,21 +1,7 @@
 module NavHelper
-  def nav_menu_collapsed?
-    cookies[:collapsed_nav] == 'true'
-  end
-
-  def nav_sidebar_class
-    if nav_menu_collapsed?
-      "sidebar-collapsed"
-    else
-      "sidebar-expanded"
-    end
-  end
-
   def page_sidebar_class
     if pinned_nav?
       "page-sidebar-expanded page-sidebar-pinned"
-    else
-      "page-sidebar-collapsed"
     end
   end
 
@@ -26,7 +12,6 @@ module NavHelper
       current_path?('merge_requests#builds') ||
       current_path?('merge_requests#conflicts') ||
       current_path?('merge_requests#pipelines') ||
-      
       current_path?('issues#show')
       if cookies[:collapsed_gutter] == 'true'
         "page-gutter right-sidebar-collapsed"
@@ -43,9 +28,7 @@ module NavHelper
     class_name << " with-horizontal-nav" if defined?(nav) && nav
 
     if pinned_nav?
-      class_name << " header-expanded header-pinned-nav"
-    else
-      class_name << " header-collapsed"
+      class_name << " header-sidebar-expanded header-sidebar-pinned"
     end
 
     class_name
diff --git a/app/views/layouts/_page.html.haml b/app/views/layouts/_page.html.haml
index bf50633af24..4f7839a881f 100644
--- a/app/views/layouts/_page.html.haml
+++ b/app/views/layouts/_page.html.haml
@@ -1,5 +1,5 @@
 .page-with-sidebar{ class: "#{page_sidebar_class} #{page_gutter_class}" }
-  .sidebar-wrapper.nicescroll{ class: nav_sidebar_class }
+  .sidebar-wrapper.nicescroll
     .sidebar-action-buttons
       = link_to '#', class: 'nav-header-btn toggle-nav-collapse', title: "Open/Close" do
         %span.sr-only Toggle navigation
diff --git a/spec/helpers/nav_helper_spec.rb b/spec/helpers/nav_helper_spec.rb
deleted file mode 100644
index e4d18d8bfc6..00000000000
--- a/spec/helpers/nav_helper_spec.rb
+++ /dev/null
@@ -1,25 +0,0 @@
-require 'spec_helper'
-
-# Specs in this file have access to a helper object that includes
-# the NavHelper. For example:
-#
-# describe NavHelper do
-#   describe "string concat" do
-#     it "concats two strings with spaces" do
-#       expect(helper.concat_strings("this","that")).to eq("this that")
-#     end
-#   end
-# end
-describe NavHelper do
-  describe '#nav_menu_collapsed?' do
-    it 'returns true when the nav is collapsed in the cookie' do
-      helper.request.cookies[:collapsed_nav] = 'true'
-      expect(helper.nav_menu_collapsed?).to eq true
-    end
-
-    it 'returns false when the nav is not collapsed in the cookie' do
-      helper.request.cookies[:collapsed_nav] = 'false'
-      expect(helper.nav_menu_collapsed?).to eq false
-    end
-  end
-end