Fix collapsing behaviour at 1200px

3 files changed, 27 insertions, 29 deletions

diff --git a/app/assets/javascripts/contextual_sidebar.js b/app/assets/javascripts/contextual_sidebar.js
index 8561383c574..8e2ff314434 100644
--- a/app/assets/javascripts/contextual_sidebar.js
+++ b/app/assets/javascripts/contextual_sidebar.js
@@ -47,9 +47,9 @@ export default class ContextualSidebar {
 
   // 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.windowWidth() > 1200;
+  static isDesktopBreakpoint = () => bp.windowWidth() >= 1200;
   static setCollapsedCookie(value) {
-    if (bp.getBreakpointSize() !== 'lg') {
+    if (!ContextualSidebar.isDesktopBreakpoint()) {
       return;
     }
     Cookies.set('sidebar_collapsed', value, { expires: 365 * 10 });
@@ -92,12 +92,11 @@ export default class ContextualSidebar {
   render() {
     if (!this.$sidebar.length) return;
 
-    const breakpoint = bp.getBreakpointSize();
     if (!ContextualSidebar.isDesktopBreakpoint()) {
       this.toggleSidebarNav(false);
-    } else if (breakpoint === 'lg') {
+    } else {
       const collapse = parseBoolean(Cookies.get('sidebar_collapsed'));
-      this.toggleCollapsedSidebar(collapse, false);
+      this.toggleCollapsedSidebar(collapse, true);
     }
   }
 }
diff --git a/app/assets/stylesheets/framework/contextual_sidebar.scss b/app/assets/stylesheets/framework/contextual_sidebar.scss
index 7601cb671e2..58adda3a40b 100644
--- a/app/assets/stylesheets/framework/contextual_sidebar.scss
+++ b/app/assets/stylesheets/framework/contextual_sidebar.scss
@@ -5,12 +5,9 @@
     padding-left: $contextual-sidebar-collapsed-width;
   }
 
-  // NOTE: at 1200px nav sidebar should be in 'desktop mode' (not overlap the content)
+  // At 1200px nav sidebar should not overlap the content
   // https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/24555#note_134136110
-  // But setting 'desktop mode' at 1200px will break spec/support/features/reportable_note_shared_examples.rb
-  $desktop-nav-sidebar-breakpoint: 1201px;
-
-  @media (min-width: $desktop-nav-sidebar-breakpoint) {
+  @include media-breakpoint-up(xl) {
     padding-left: $contextual-sidebar-width;
   }
 
@@ -215,7 +212,7 @@
     width: 16px;
   }
 
-  @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) - 1px) {
     &: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 43fe477fb29..736a574e937 100644
--- a/spec/features/projects/user_sees_sidebar_spec.rb
+++ b/spec/features/projects/user_sees_sidebar_spec.rb
@@ -11,8 +11,6 @@ describe 'Projects > User sees sidebar' 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')
@@ -26,6 +24,21 @@ describe 'Projects > User sees sidebar' do
       end
     end
 
+    shared_examples 'has a desktop nav sidebar' do
+      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
+
     context 'with xs size' do
       before do
         resize_screen_xs
@@ -56,9 +69,9 @@ describe 'Projects > User sees sidebar' do
       it_behaves_like 'has a collapsible mobile nav sidebar'
     end
 
-    context 'at 1200px exactly' do
+    context 'with size 1199px' do
       before do
-        resize_window(1200, 800)
+        resize_window(1199, 800)
         visit project_path(project)
         expect(page).to have_selector('.nav-sidebar')
       end
@@ -66,25 +79,14 @@ describe 'Projects > User sees sidebar' do
       it_behaves_like 'has a collapsible mobile nav sidebar'
     end
 
-    context 'at 1201px exactly' do
+    context 'with a larger screen' do
       before do
-        resize_window(1201, 800)
+        resize_window(1200, 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
+      it_behaves_like 'has a desktop nav sidebar'
     end
   end