Added test for interacting with the nav-sidebar on smaller screens

Use mobile collapsing behaviour at width 1200px

3 files changed, 98 insertions, 13 deletions

diff --git a/app/assets/javascripts/contextual_sidebar.js b/app/assets/javascripts/contextual_sidebar.js
index a9013025bad..7780ac8a1d2 100644
--- a/app/assets/javascripts/contextual_sidebar.js
+++ b/app/assets/javascripts/contextual_sidebar.js
@@ -4,7 +4,6 @@ import _ from 'underscore';
 import bp from './breakpoints';
 import { parseBoolean } from '~/lib/utils/common_utils';
 
-const DESKTOP_BREAKPOINTS = ['xl', 'lg'];
 export default class ContextualSidebar {
   constructor() {
     this.initDomElements();
@@ -39,11 +38,9 @@ export default class ContextualSidebar {
     this.$closeSidebar.on('click', () => this.toggleSidebarNav(false));
     this.$overlay.on('click', () => this.toggleSidebarNav(false));
     this.$sidebarToggle.on('click', () => {
-      const breakpoint = bp.getBreakpointSize();
-
-      if (!ContextualSidebar.isDesktopBreakpoint(breakpoint)) {
+      if (!ContextualSidebar.isDesktopBreakpoint()) {
         this.toggleSidebarNav(!this.$sidebar.hasClass('sidebar-expanded-mobile'));
-      } else if (breakpoint === 'lg') {
+      } else {
         const value = !this.$sidebar.hasClass('sidebar-collapsed-desktop');
         this.toggleCollapsedSidebar(value, true);
       }
@@ -52,9 +49,13 @@ export default class ContextualSidebar {
     $(window).on('resize', () => _.debounce(this.render(), 100));
   }
 
+  // NOTE: at 1200px sidebar should be in 'desktop' mode
+  // https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/24555#note_134136110
+  // But, current behaviour starts desktop mode at 1201px
+
   // TODO: use the breakpoints from breakpoints.js once they have been updated for bootstrap 4
   // See related issue and discussion: https://gitlab.com/gitlab-org/gitlab-ce/issues/56745
-  static isDesktopBreakpoint = (_bp = '') => DESKTOP_BREAKPOINTS.indexOf(_bp) > -1;
+  static isDesktopBreakpoint = () => bp.windowWidth() > 1200;
   static setCollapsedCookie(value) {
     if (bp.getBreakpointSize() !== 'lg') {
       return;
@@ -64,12 +65,11 @@ export default class ContextualSidebar {
 
   toggleSidebarNav(show) {
     const breakpoint = bp.getBreakpointSize();
-    const dbp = ContextualSidebar.isDesktopBreakpoint(breakpoint);
+    const dbp = ContextualSidebar.isDesktopBreakpoint();
 
     this.$sidebar.toggleClass('sidebar-expanded-mobile', !dbp ? show : false);
     this.$overlay.toggleClass('mobile-nav-open', breakpoint === 'xs' ? show : false);
     this.$sidebar.removeClass('sidebar-collapsed-desktop');
-    this.$page.toggleClass('page-with-contextual-sidebar', true);
   }
 
   toggleCollapsedSidebar(collapsed, saveCookie) {
@@ -80,7 +80,6 @@ export default class ContextualSidebar {
       this.$sidebar.toggleClass('sidebar-collapsed-desktop', collapsed);
       this.$sidebar.toggleClass('sidebar-expanded-mobile', !dbp ? !collapsed : false);
       this.$page.toggleClass('page-with-icon-sidebar', breakpoint === 'sm' ? true : collapsed);
-      this.$page.toggleClass('page-with-contextual-sidebar', true);
     }
 
     if (saveCookie) {
@@ -102,7 +101,7 @@ export default class ContextualSidebar {
     if (!this.$sidebar.length) return;
 
     const breakpoint = bp.getBreakpointSize();
-    if (!ContextualSidebar.isDesktopBreakpoint(breakpoint)) {
+    if (!ContextualSidebar.isDesktopBreakpoint()) {
       this.toggleSidebarNav(false);
     } else if (breakpoint === 'lg') {
       const collapse = parseBoolean(Cookies.get('sidebar_collapsed'));
diff --git a/app/assets/stylesheets/framework/contextual_sidebar.scss b/app/assets/stylesheets/framework/contextual_sidebar.scss
index 4f4ce389d5d..79c770a3a35 100644
--- a/app/assets/stylesheets/framework/contextual_sidebar.scss
+++ b/app/assets/stylesheets/framework/contextual_sidebar.scss
@@ -5,7 +5,9 @@
     padding-left: $contextual-sidebar-collapsed-width;
   }
 
-  @include media-breakpoint-up(xl) {
+
+  $desktop-nav-sidebar-breakpoint: 1201px;
+  @media (min-width: $desktop-nav-sidebar-breakpoint) {
     padding-left: $contextual-sidebar-width;
   }
 
@@ -210,8 +212,8 @@
     width: 16px;
   }
 
-  &:not(.sidebar-expanded-mobile) {
-    @media (min-width: map-get($grid-breakpoints, sm)) and (max-width: map-get($grid-breakpoints, xl)) {
+  @media (min-width: map-get($grid-breakpoints, sm)) and (max-width: map-get($grid-breakpoints, xl)) {
+    &:not(.sidebar-expanded-mobile) {
       @include collapse-contextual-sidebar;
       @include collapse-contextual-sidebar-content;
     }
diff --git a/spec/features/projects/user_sees_sidebar_spec.rb b/spec/features/projects/user_sees_sidebar_spec.rb
index ee5734a9bf1..43fe477fb29 100644
--- a/spec/features/projects/user_sees_sidebar_spec.rb
+++ b/spec/features/projects/user_sees_sidebar_spec.rb
@@ -4,6 +4,90 @@ describe 'Projects > User sees sidebar' do
   let(:user) { create(:user) }
   let(:project) { create(:project, :private, public_builds: false, namespace: user.namespace) }
 
+  context 'on a smaller screen', :js do
+    include MobileHelpers
+
+    before do
+      sign_in(user)
+    end
+
+    # refactor behaviours into shared behaviours
+
+    shared_examples 'has a collapsible mobile nav sidebar' do
+      it 'has a collapsed desktop nav-sidebar on load' do
+        expect(page).not_to have_content('Collapse sidebar')
+        expect(page).not_to have_selector('.sidebar-expanded-mobile')
+      end
+
+      it 'can expand the nav-sidebar' do
+        page.find('.nav-sidebar .js-toggle-sidebar').click
+        expect(page).to have_selector('.sidebar-expanded-mobile')
+        expect(page).to have_content('Collapse sidebar')
+      end
+    end
+
+    context 'with xs size' do
+      before do
+        resize_screen_xs
+        visit project_path(project)
+        expect(page).to have_selector('.nav-sidebar')
+        expect(page).to have_selector('.toggle-mobile-nav')
+      end
+
+      it 'has a collapsed nav-sidebar on load' do
+        expect(page).not_to have_content('.mobile-nav-open')
+        expect(page).not_to have_selector('.sidebar-expanded-mobile')
+      end
+
+      it 'can expand the nav-sidebar' do
+        page.find('.toggle-mobile-nav').click
+        expect(page).to have_selector('.mobile-nav-open')
+        expect(page).to have_selector('.sidebar-expanded-mobile')
+      end
+    end
+
+    context 'with sm size' do
+      before do
+        resize_screen_sm
+        visit project_path(project)
+        expect(page).to have_selector('.nav-sidebar')
+      end
+
+      it_behaves_like 'has a collapsible mobile nav sidebar'
+    end
+
+    context 'at 1200px exactly' do
+      before do
+        resize_window(1200, 800)
+        visit project_path(project)
+        expect(page).to have_selector('.nav-sidebar')
+      end
+
+      it_behaves_like 'has a collapsible mobile nav sidebar'
+    end
+
+    context 'at 1201px exactly' do
+      before do
+        resize_window(1201, 800)
+        visit project_path(project)
+        expect(page).to have_selector('.nav-sidebar')
+      end
+
+      it 'has a expanded desktop nav-sidebar on load' do
+        expect(page).to have_content('Collapse sidebar')
+        expect(page).not_to have_selector('.sidebar-collapsed-desktop')
+        expect(page).not_to have_selector('.sidebar-expanded-mobile')
+      end
+
+      it 'can collapse the nav-sidebar' do
+        page.find('.nav-sidebar .js-toggle-sidebar').click
+        expect(page).to have_selector('.sidebar-collapsed-desktop')
+        expect(page).not_to have_content('Collapse sidebar')
+        expect(page).not_to have_selector('.sidebar-expanded-mobile')
+      end
+    end
+  end
+
   context 'as owner' do
     before do
       sign_in(user)