Merge branch '331778-improve-sidebar-toggle-logic' into 'master'

Create SSoT for issuable sidebar toggling

See merge request gitlab-org/gitlab!65400

app/assets/javascripts/issuable_show/components/issuable_show_root.vue

@@ -153,9 +153,9 @@ export default {
       </template>
     </issuable-discussion>
 
-    <issuable-sidebar @sidebar-toggle="$emit('sidebar-toggle', $event)">
-      <template #right-sidebar-items="sidebarProps">
-        <slot name="right-sidebar-items" v-bind="sidebarProps"></slot>
+    <issuable-sidebar>
+      <template #right-sidebar-items="{ sidebarExpanded, toggleSidebar }">
+        <slot name="right-sidebar-items" v-bind="{ sidebarExpanded, toggleSidebar }"></slot>
       </template>
     </issuable-sidebar>
   </div>

app/assets/javascripts/issuable_sidebar/components/issuable_sidebar_root.vue

@@ -2,15 +2,15 @@
 import { GlIcon } from '@gitlab/ui';
 import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils';
 import Cookies from 'js-cookie';
-
 import { parseBoolean } from '~/lib/utils/common_utils';
+import { USER_COLLAPSED_GUTTER_COOKIE } from '../constants';
 
 export default {
   components: {
     GlIcon,
   },
   data() {
-    const userExpanded = !parseBoolean(Cookies.get('collapsed_gutter'));
+    const userExpanded = !parseBoolean(Cookies.get(USER_COLLAPSED_GUTTER_COOKIE));
 
     // We're deliberately keeping two different props for sidebar status;
     // 1. userExpanded reflects value based on cookie `collapsed_gutter`.
@@ -20,13 +20,6 @@ export default {
       isExpanded: userExpanded ? bp.isDesktop() : userExpanded,
     };
   },
-  watch: {
-    isExpanded(expanded) {
-      this.$emit('sidebar-toggle', {
-        expanded,
-      });
-    },
-  },
   mounted() {
     window.addEventListener('resize', this.handleWindowResize);
     this.updatePageContainerClass();
@@ -49,11 +42,11 @@ export default {
         this.updatePageContainerClass();
       }
     },
-    handleToggleSidebarClick() {
+    toggleSidebar() {
       this.isExpanded = !this.isExpanded;
       this.userExpanded = this.isExpanded;
 
-      Cookies.set('collapsed_gutter', !this.userExpanded);
+      Cookies.set(USER_COLLAPSED_GUTTER_COOKIE, !this.userExpanded);
       this.updatePageContainerClass();
     },
   },
@@ -68,8 +61,9 @@ export default {
   >
     <button
       class="toggle-right-sidebar-button js-toggle-right-sidebar-button w-100 gl-text-decoration-none! gl-display-flex gl-outline-0!"
+      data-testid="toggle-right-sidebar-button"
       :title="__('Toggle sidebar')"
-      @click="handleToggleSidebarClick"
+      @click="toggleSidebar"
     >
       <span v-if="isExpanded" class="collapse-text gl-flex-grow-1 gl-text-left">{{
         __('Collapse sidebar')
@@ -83,7 +77,10 @@ export default {
       />
     </button>
     <div data-testid="sidebar-items" class="issuable-sidebar">
-      <slot name="right-sidebar-items" v-bind="{ sidebarExpanded: isExpanded }"></slot>
+      <slot
+        name="right-sidebar-items"
+        v-bind="{ sidebarExpanded: isExpanded, toggleSidebar }"
+      ></slot>
     </div>
   </aside>
 </template>

app/assets/javascripts/issuable_sidebar/constants.js

+export const USER_COLLAPSED_GUTTER_COOKIE = 'collapsed_gutter';

ee/app/assets/javascripts/integrations/jira/issues_show/components/jira_issues_show_root.vue

@@ -174,7 +174,7 @@ export default {
       >
         <template #status-badge>{{ statusBadgeText }}</template>
 
-        <template #right-sidebar-items="{ sidebarExpanded }">
+        <template #right-sidebar-items="{ sidebarExpanded, toggleSidebar }">
           <jira-issue-sidebar
             :sidebar-expanded="sidebarExpanded"
             :issue="issue"
@@ -185,6 +185,7 @@ export default {
             @issue-labels-updated="onIssueLabelsUpdated"
             @issue-status-fetch="onIssueStatusFetch"
             @issue-status-updated="onIssueStatusUpdated"
+            @sidebar-toggle="toggleSidebar"
           />
         </template>
 

ee/app/assets/javascripts/integrations/jira/issues_show/components/sidebar/jira_issues_sidebar_root.vue

@@ -85,11 +85,10 @@ export default {
   },
   mounted() {
     this.sidebarEl = document.querySelector('aside.right-sidebar');
-    this.sidebarToggleEl = document.querySelector('.js-toggle-right-sidebar-button');
   },
   methods: {
     toggleSidebar() {
-      this.sidebarToggleEl.dispatchEvent(new Event('click'));
+      this.$emit('sidebar-toggle');
     },
     afterSidebarTransitioned(callback) {
       // Wait for sidebar expand animation to complete

ee/app/assets/javascripts/test_case_show/components/test_case_show_root.vue

@@ -229,13 +229,14 @@ export default {
           {{ __('Cancel') }}
         </gl-button>
       </template>
-      <template #right-sidebar-items="{ sidebarExpanded }">
+      <template #right-sidebar-items="{ sidebarExpanded, toggleSidebar }">
         <test-case-sidebar
           :sidebar-expanded="sidebarExpanded"
           :selected-labels="selectedLabels"
           :todo="todo"
           :moved="testCase.moved"
           @test-case-updated="handleTestCaseUpdated"
+          @sidebar-toggle="toggleSidebar"
         />
       </template>
     </issuable-show>

ee/app/assets/javascripts/test_case_show/components/test_case_sidebar.vue

@@ -91,7 +91,7 @@ export default {
       }
     },
     toggleSidebar() {
-      document.querySelector('.js-toggle-right-sidebar-button').dispatchEvent(new Event('click'));
+      this.$emit('sidebar-toggle');
     },
     expandSidebarAndOpenDropdown(dropdownButtonSelector) {
       // Expand the sidebar if not already expanded.

ee/spec/frontend/integrations/jira/issues_show/components/jira_issues_show_root_spec.js

@@ -167,5 +167,15 @@ describe('JiraIssuesShow', () => {
 
       expect(findJiraIssueSidebar().props('isUpdatingStatus')).toBe(false);
     });
+
+    it('updates `sidebarExpanded` prop on `sidebar-toggle` event', async () => {
+      const jiraIssueSidebar = findJiraIssueSidebar();
+      expect(jiraIssueSidebar.props('sidebarExpanded')).toBe(true);
+
+      jiraIssueSidebar.vm.$emit('sidebar-toggle');
+      await wrapper.vm.$nextTick();
+
+      expect(jiraIssueSidebar.props('sidebarExpanded')).toBe(false);
+    });
   });
 });

ee/spec/frontend/test_case_show/components/test_case_show_root_spec.js

@@ -55,6 +55,8 @@ const createComponent = ({ testCase, testCaseQueryLoading = false } = {}) =>
 describe('TestCaseShowRoot', () => {
   let wrapper;
 
+  const findTestCaseSidebar = () => wrapper.findComponent(TestCaseSidebar);
+
   beforeEach(() => {
     wrapper = createComponent();
   });
@@ -361,7 +363,17 @@ describe('TestCaseShowRoot', () => {
     });
 
     it('renders test-case-sidebar', async () => {
-      expect(wrapper.find(TestCaseSidebar).exists()).toBe(true);
+      expect(findTestCaseSidebar().exists()).toBe(true);
+    });
+
+    it('updates `sidebarExpanded` prop on `sidebar-toggle` event', async () => {
+      const testCaseSidebar = findTestCaseSidebar();
+      expect(testCaseSidebar.props('sidebarExpanded')).toBe(true);
+
+      testCaseSidebar.vm.$emit('sidebar-toggle');
+      await wrapper.vm.$nextTick();
+
+      expect(testCaseSidebar.props('sidebarExpanded')).toBe(false);
     });
   });
 });

ee/spec/frontend/test_case_show/components/test_case_sidebar_spec.js

@@ -139,17 +139,9 @@ describe('TestCaseSidebar', () => {
       });
 
       it('dispatches click event on sidebar toggle button', () => {
-        const buttonEl = document.querySelector('.js-toggle-right-sidebar-button');
-
-        jest.spyOn(buttonEl, 'dispatchEvent');
-
         wrapper.vm.toggleSidebar();
 
-        expect(buttonEl.dispatchEvent).toHaveBeenCalledWith(
-          expect.objectContaining({
-            type: 'click',
-          }),
-        );
+        expect(wrapper.emitted('sidebar-toggle')).toBeDefined();
       });
     });
 

spec/frontend/issuable_show/components/issuable_show_root_spec.js

@@ -133,14 +133,6 @@ describe('IssuableShowRoot', () => {
         expect(wrapper.emitted('task-list-update-failure')).toBeTruthy();
       });
 
-      it('component emits `sidebar-toggle` event bubbled via issuable-sidebar', () => {
-        const issuableSidebar = wrapper.find(IssuableSidebar);
-
-        issuableSidebar.vm.$emit('sidebar-toggle', true);
-
-        expect(wrapper.emitted('sidebar-toggle')).toBeTruthy();
-      });
-
       it.each(['keydown-title', 'keydown-description'])(
         'component emits `%s` event with event object and issuableMeta params via issuable-body',
         (eventName) => {

spec/frontend/issuable_sidebar/components/issuable_sidebar_root_spec.js

 import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils';
-import { shallowMount } from '@vue/test-utils';
 import Cookies from 'js-cookie';
+import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
 
 import IssuableSidebarRoot from '~/issuable_sidebar/components/issuable_sidebar_root.vue';
+import { USER_COLLAPSED_GUTTER_COOKIE } from '~/issuable_sidebar/constants';
 
-const createComponent = (expanded = true) =>
-  shallowMount(IssuableSidebarRoot, {
-    propsData: {
-      expanded,
-    },
+const MOCK_LAYOUT_PAGE_CLASS = 'layout-page';
+
+const createComponent = () => {
+  setFixtures(`<div class="${MOCK_LAYOUT_PAGE_CLASS}"></div>`);
+
+  return shallowMountExtended(IssuableSidebarRoot, {
     slots: {
       'right-sidebar-items': `
         <button class="js-todo">Todo</button>
       `,
     },
   });
+};
 
 describe('IssuableSidebarRoot', () => {
   let wrapper;
 
-  beforeEach(() => {
-    wrapper = createComponent();
-  });
+  const findToggleSidebarButton = () => wrapper.findByTestId('toggle-right-sidebar-button');
+
+  const assertPageLayoutClasses = ({ isExpanded }) => {
+    const { classList } = document.querySelector(`.${MOCK_LAYOUT_PAGE_CLASS}`);
+    if (isExpanded) {
+      expect(classList).toContain('right-sidebar-expanded');
+      expect(classList).not.toContain('right-sidebar-collapsed');
+    } else {
+      expect(classList).toContain('right-sidebar-collapsed');
+      expect(classList).not.toContain('right-sidebar-expanded');
+    }
+  };
 
   afterEach(() => {
     wrapper.destroy();
   });
 
-  describe('watch', () => {
-    describe('isExpanded', () => {
-      it('emits `sidebar-toggle` event on component', async () => {
-        wrapper.setData({
-          isExpanded: false,
-        });
-
-        await wrapper.vm.$nextTick();
-
-        expect(wrapper.emitted('sidebar-toggle')).toBeTruthy();
-        expect(wrapper.emitted('sidebar-toggle')[0]).toEqual([
-          {
-            expanded: false,
-          },
-        ]);
-      });
-    });
-  });
+  describe('when sidebar is expanded', () => {
+    beforeEach(() => {
+      jest.spyOn(Cookies, 'set').mockImplementation(jest.fn());
+      jest.spyOn(Cookies, 'get').mockReturnValue(false);
+      jest.spyOn(bp, 'isDesktop').mockReturnValue(true);
 
-  describe('methods', () => {
-    describe('updatePageContainerClass', () => {
-      beforeEach(() => {
-        setFixtures('<div class="layout-page"></div>');
-      });
+      wrapper = createComponent();
+    });
 
-      it.each`
-        isExpanded | layoutPageClass
-        ${true}    | ${'right-sidebar-expanded'}
-        ${false}   | ${'right-sidebar-collapsed'}
-      `(
-        'set class $layoutPageClass to container element when `isExpanded` prop is $isExpanded',
-        async ({ isExpanded, layoutPageClass }) => {
-          wrapper.setData({
-            isExpanded,
-          });
+    it('renders component container element with class `right-sidebar-expanded`', () => {
+      expect(wrapper.classes()).toContain('right-sidebar-expanded');
+    });
 
-          await wrapper.vm.$nextTick();
+    it('sets layout class to reflect expanded state', () => {
+      assertPageLayoutClasses({ isExpanded: true });
+    });
 
-          wrapper.vm.updatePageContainerClass();
+    it('renders sidebar toggle button with text and icon', () => {
+      const buttonEl = findToggleSidebarButton();
 
-          expect(document.querySelector('.layout-page').classList.contains(layoutPageClass)).toBe(
-            true,
-          );
-        },
-      );
+      expect(buttonEl.exists()).toBe(true);
+      expect(buttonEl.attributes('title')).toBe('Toggle sidebar');
+      expect(buttonEl.find('span').text()).toBe('Collapse sidebar');
+      expect(wrapper.findByTestId('icon-collapse').isVisible()).toBe(true);
     });
 
-    describe('handleWindowResize', () => {
-      beforeEach(async () => {
-        wrapper.setData({
-          userExpanded: true,
-        });
+    describe('when collapsing the sidebar', () => {
+      it('updates "collapsed_gutter" cookie value and layout classes', async () => {
+        await findToggleSidebarButton().trigger('click');
 
-        await wrapper.vm.$nextTick();
+        expect(Cookies.set).toHaveBeenCalledWith(USER_COLLAPSED_GUTTER_COOKIE, true);
+        assertPageLayoutClasses({ isExpanded: false });
       });
+    });
 
+    describe('when window `resize` event is triggered', () => {
       it.each`
         breakpoint | isExpandedValue
         ${'xs'}    | ${false}
@@ -91,109 +83,49 @@ describe('IssuableSidebarRoot', () => {
         ${'lg'}    | ${true}
         ${'xl'}    | ${true}
       `(
-        'sets `isExpanded` prop to $isExpandedValue only when current screen size is `lg` or `xl`',
+        'sets page layout classes correctly when current screen size is `$breakpoint`',
         async ({ breakpoint, isExpandedValue }) => {
           jest.spyOn(bp, 'isDesktop').mockReturnValue(breakpoint === 'lg' || breakpoint === 'xl');
 
-          wrapper.vm.handleWindowResize();
+          window.dispatchEvent(new Event('resize'));
+          await wrapper.vm.$nextTick();
 
-          expect(wrapper.vm.isExpanded).toBe(isExpandedValue);
+          assertPageLayoutClasses({ isExpanded: isExpandedValue });
         },
       );
-
-      it('calls `updatePageContainerClass` method', () => {
-        jest.spyOn(wrapper.vm, 'updatePageContainerClass');
-
-        wrapper.vm.handleWindowResize();
-
-        expect(wrapper.vm.updatePageContainerClass).toHaveBeenCalled();
-      });
-    });
-
-    describe('handleToggleSidebarClick', () => {
-      beforeEach(async () => {
-        jest.spyOn(Cookies, 'set').mockImplementation(jest.fn());
-        wrapper.setData({
-          isExpanded: true,
-        });
-
-        await wrapper.vm.$nextTick();
-      });
-
-      it('flips value of `isExpanded`', () => {
-        wrapper.vm.handleToggleSidebarClick();
-
-        expect(wrapper.vm.isExpanded).toBe(false);
-        expect(wrapper.vm.userExpanded).toBe(false);
-      });
-
-      it('updates "collapsed_gutter" cookie value', () => {
-        wrapper.vm.handleToggleSidebarClick();
-
-        expect(Cookies.set).toHaveBeenCalledWith('collapsed_gutter', true);
-      });
-
-      it('calls `updatePageContainerClass` method', () => {
-        jest.spyOn(wrapper.vm, 'updatePageContainerClass');
-
-        wrapper.vm.handleWindowResize();
-
-        expect(wrapper.vm.updatePageContainerClass).toHaveBeenCalled();
-      });
     });
   });
 
-  describe('template', () => {
-    describe('sidebar expanded', () => {
-      beforeEach(async () => {
-        wrapper.setData({
-          isExpanded: true,
-        });
+  describe('when sidebar is collapsed', () => {
+    beforeEach(() => {
+      jest.spyOn(Cookies, 'get').mockReturnValue(true);
 
-        await wrapper.vm.$nextTick();
-      });
-
-      it('renders component container element with class `right-sidebar-expanded` when `isExpanded` prop is true', () => {
-        expect(wrapper.classes()).toContain('right-sidebar-expanded');
-      });
-
-      it('renders sidebar toggle button with text and icon', () => {
-        const buttonEl = wrapper.find('button');
-
-        expect(buttonEl.exists()).toBe(true);
-        expect(buttonEl.attributes('title')).toBe('Toggle sidebar');
-        expect(buttonEl.find('span').text()).toBe('Collapse sidebar');
-        expect(buttonEl.find('[data-testid="icon-collapse"]').isVisible()).toBe(true);
-      });
+      wrapper = createComponent();
     });
 
-    describe('sidebar collapsed', () => {
-      beforeEach(async () => {
-        wrapper.setData({
-          isExpanded: false,
-        });
-
-        await wrapper.vm.$nextTick();
-      });
+    it('renders component container element with class `right-sidebar-collapsed`', () => {
+      expect(wrapper.classes()).toContain('right-sidebar-collapsed');
+    });
 
-      it('renders component container element with class `right-sidebar-collapsed` when `isExpanded` prop is false', () => {
-        expect(wrapper.classes()).toContain('right-sidebar-collapsed');
-      });
+    it('sets layout class to reflect collapsed state', () => {
+      assertPageLayoutClasses({ isExpanded: false });
+    });
 
-      it('renders sidebar toggle button with text and icon', () => {
-        const buttonEl = wrapper.find('button');
+    it('renders sidebar toggle button with text and icon', () => {
+      const buttonEl = findToggleSidebarButton();
 
-        expect(buttonEl.exists()).toBe(true);
-        expect(buttonEl.attributes('title')).toBe('Toggle sidebar');
-        expect(buttonEl.find('[data-testid="icon-expand"]').isVisible()).toBe(true);
-      });
+      expect(buttonEl.exists()).toBe(true);
+      expect(buttonEl.attributes('title')).toBe('Toggle sidebar');
+      expect(wrapper.findByTestId('icon-expand').isVisible()).toBe(true);
     });
+  });
 
-    it('renders sidebar items', () => {
-      const sidebarItemsEl = wrapper.find('[data-testid="sidebar-items"]');
+  it('renders slotted sidebar items', () => {
+    wrapper = createComponent();
 
-      expect(sidebarItemsEl.exists()).toBe(true);
-      expect(sidebarItemsEl.find('button.js-todo').exists()).toBe(true);
-    });
+    const sidebarItemsEl = wrapper.findByTestId('sidebar-items');
+
+    expect(sidebarItemsEl.exists()).toBe(true);
+    expect(sidebarItemsEl.find('button.js-todo').exists()).toBe(true);
   });
 });