Merge branch 'move-router-to-component' into 'master'

Move fullscreen logic from router to component

See merge request gitlab-org/gitlab!40818

app/assets/javascripts/design_management/pages/design/index.vue

@@ -21,6 +21,7 @@ import {
   updateImageDiffNoteOptimisticResponse,
   toDiffNoteGid,
   extractDesignNoteId,
+  getPageLayoutElement,
 } from '../../utils/design_management_utils';
 import {
   updateStoreAfterAddImageDiffNote,
@@ -38,7 +39,7 @@ import {
 } from '../../utils/error_messages';
 import { trackDesignDetailView } from '../../utils/tracking';
 import { DESIGNS_ROUTE_NAME } from '../../router/constants';
-import { ACTIVE_DISCUSSION_SOURCE_TYPES } from '../../constants';
+import { ACTIVE_DISCUSSION_SOURCE_TYPES, DESIGN_DETAIL_LAYOUT_CLASSLIST } from '../../constants';
 
 const DEFAULT_SCALE = 1;
 
@@ -300,6 +301,22 @@ export default {
       this.resolvedDiscussionsExpanded = !this.resolvedDiscussionsExpanded;
     },
   },
+  beforeRouteEnter(to, from, next) {
+    const pageEl = getPageLayoutElement();
+    if (pageEl) {
+      pageEl.classList.add(...DESIGN_DETAIL_LAYOUT_CLASSLIST);
+    }
+
+    next();
+  },
+  beforeRouteLeave(to, from, next) {
+    const pageEl = getPageLayoutElement();
+    if (pageEl) {
+      pageEl.classList.remove(...DESIGN_DETAIL_LAYOUT_CLASSLIST);
+    }
+
+    next();
+  },
   createImageDiffNoteMutation,
   DESIGNS_ROUTE_NAME,
 };

app/assets/javascripts/design_management/router/index.js

 import Vue from 'vue';
 import VueRouter from 'vue-router';
 import routes from './routes';
-import { DESIGN_ROUTE_NAME } from './constants';
-import { getPageLayoutElement } from '~/design_management/utils/design_management_utils';
-import { DESIGN_DETAIL_LAYOUT_CLASSLIST } from '../constants';
 
 Vue.use(VueRouter);
 
@@ -13,20 +10,6 @@ export default function createRouter(base) {
     mode: 'history',
     routes,
   });
-  const pageEl = getPageLayoutElement();
-
-  router.beforeEach(({ name }, _, next) => {
-    // apply a fullscreen layout style in Design View (a.k.a design detail)
-    if (pageEl) {
-      if (name === DESIGN_ROUTE_NAME) {
-        pageEl.classList.add(...DESIGN_DETAIL_LAYOUT_CLASSLIST);
-      } else {
-        pageEl.classList.remove(...DESIGN_DETAIL_LAYOUT_CLASSLIST);
-      }
-    }
-
-    next();
-  });
 
   return router;
 }

spec/frontend/design_management/pages/design/index_spec.js

@@ -24,7 +24,13 @@ import mockAllVersions from '../../mock_data/all_versions';
 jest.mock('~/flash');
 
 const focusInput = jest.fn();
-
+const mutate = jest.fn().mockResolvedValue();
+const mockPageLayoutElement = {
+  classList: {
+    add: jest.fn(),
+    remove: jest.fn(),
+  },
+};
 const DesignReplyForm = {
   template: '<div><textarea ref="textarea"></textarea></div>',
   methods: {
@@ -37,6 +43,32 @@ const mockDesignNoDiscussions = {
     nodes: [],
   },
 };
+const newComment = 'new comment';
+const annotationCoordinates = {
+  x: 10,
+  y: 10,
+  width: 100,
+  height: 100,
+};
+const createDiscussionMutationVariables = {
+  mutation: createImageDiffNoteMutation,
+  update: expect.anything(),
+  variables: {
+    input: {
+      body: newComment,
+      noteableId: design.id,
+      position: {
+        headSha: 'headSha',
+        baseSha: 'baseSha',
+        startSha: 'startSha',
+        paths: {
+          newPath: 'full-design-path',
+        },
+        ...annotationCoordinates,
+      },
+    },
+  },
+};
 
 const localVue = createLocalVue();
 localVue.use(VueRouter);
@@ -45,35 +77,6 @@ describe('Design management design index page', () => {
   let wrapper;
   let router;
 
-  const newComment = 'new comment';
-  const annotationCoordinates = {
-    x: 10,
-    y: 10,
-    width: 100,
-    height: 100,
-  };
-  const createDiscussionMutationVariables = {
-    mutation: createImageDiffNoteMutation,
-    update: expect.anything(),
-    variables: {
-      input: {
-        body: newComment,
-        noteableId: design.id,
-        position: {
-          headSha: 'headSha',
-          baseSha: 'baseSha',
-          startSha: 'startSha',
-          paths: {
-            newPath: 'full-design-path',
-          },
-          ...annotationCoordinates,
-        },
-      },
-    },
-  };
-
-  const mutate = jest.fn().mockResolvedValue();
-
   const findDiscussionForm = () => wrapper.find(DesignReplyForm);
   const findSidebar = () => wrapper.find(DesignSidebar);
   const findDesignPresentation = () => wrapper.find(DesignPresentation);
@@ -122,19 +125,44 @@ describe('Design management design index page', () => {
     wrapper.destroy();
   });
 
-  describe('when navigating', () => {
-    it('applies fullscreen layout', () => {
-      const mockEl = {
-        classList: {
-          add: jest.fn(),
-          remove: jest.fn(),
-        },
-      };
-      jest.spyOn(utils, 'getPageLayoutElement').mockReturnValue(mockEl);
+  describe('when navigating to component', () => {
+    it('applies fullscreen layout class', () => {
+      jest.spyOn(utils, 'getPageLayoutElement').mockReturnValue(mockPageLayoutElement);
       createComponent({ loading: true });
 
-      expect(mockEl.classList.add).toHaveBeenCalledTimes(1);
-      expect(mockEl.classList.add).toHaveBeenCalledWith(...DESIGN_DETAIL_LAYOUT_CLASSLIST);
+      expect(mockPageLayoutElement.classList.add).toHaveBeenCalledTimes(1);
+      expect(mockPageLayoutElement.classList.add).toHaveBeenCalledWith(
+        ...DESIGN_DETAIL_LAYOUT_CLASSLIST,
+      );
+    });
+  });
+
+  describe('when navigating within the component', () => {
+    it('`scale` prop of DesignPresentation component is 1', async () => {
+      jest.spyOn(utils, 'getPageLayoutElement').mockReturnValue(mockPageLayoutElement);
+      createComponent({ loading: false }, { data: { design, scale: 2 } });
+
+      await wrapper.vm.$nextTick();
+      expect(findDesignPresentation().props('scale')).toBe(2);
+
+      DesignIndex.beforeRouteUpdate.call(wrapper.vm, {}, {}, jest.fn());
+      await wrapper.vm.$nextTick();
+
+      expect(findDesignPresentation().props('scale')).toBe(1);
+    });
+  });
+
+  describe('when navigating away from component', () => {
+    it('removes fullscreen layout class', async () => {
+      jest.spyOn(utils, 'getPageLayoutElement').mockReturnValue(mockPageLayoutElement);
+      createComponent({ loading: true });
+
+      wrapper.vm.$options.beforeRouteLeave[0].call(wrapper.vm, {}, {}, jest.fn());
+
+      expect(mockPageLayoutElement.classList.remove).toHaveBeenCalledTimes(1);
+      expect(mockPageLayoutElement.classList.remove).toHaveBeenCalledWith(
+        ...DESIGN_DETAIL_LAYOUT_CLASSLIST,
+      );
     });
   });
 

spec/frontend/design_management/pages/index_spec.js

@@ -19,7 +19,6 @@ import {
 import { deprecatedCreateFlash as createFlash } from '~/flash';
 import createRouter from '~/design_management/router';
 import * as utils from '~/design_management/utils/design_management_utils';
-import { DESIGN_DETAIL_LAYOUT_CLASSLIST } from '~/design_management/constants';
 import {
   designListQueryResponse,
   designUploadMutationCreatedResponse,
@@ -682,13 +681,6 @@ describe('Design management index page', () => {
   });
 
   describe('when navigating', () => {
-    it('ensures fullscreen layout is not applied', () => {
-      createComponent({ loading: true });
-
-      expect(mockPageEl.classList.remove).toHaveBeenCalledTimes(1);
-      expect(mockPageEl.classList.remove).toHaveBeenCalledWith(...DESIGN_DETAIL_LAYOUT_CLASSLIST);
-    });
-
     it('should trigger a scrollIntoView method if designs route is detected', () => {
       router.replace({
         path: '/designs',