Merge branch 'design-view-scrolling-issue-bug' into 'master'

Apply fullscreen-layout to Design View

See merge request gitlab-org/gitlab!29881

app/assets/javascripts/design_management/constants.js

@@ -12,3 +12,5 @@ export const ACTIVE_DISCUSSION_SOURCE_TYPES = {
   pin: 'pin',
   discussion: 'discussion',
 };
+
+export const DESIGN_DETAIL_LAYOUT_CLASSLIST = ['design-detail-layout', 'overflow-hidden', 'm-0'];

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

@@ -2,6 +2,9 @@ import $ from 'jquery';
 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);
 
@@ -11,10 +14,20 @@ export default function createRouter(base) {
     mode: 'history',
     routes,
   });
+  const pageEl = getPageLayoutElement();
 
-  router.beforeEach(({ meta: { el } }, from, next) => {
+  router.beforeEach(({ meta: { el }, name }, _, next) => {
     $(`#${el}`).tab('show');
 
+    // 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();
   });
 

app/assets/javascripts/design_management/utils/design_management_utils.js

@@ -123,3 +123,5 @@ const normalizeAuthor = author => ({
 });
 
 export const extractParticipants = users => users.edges.map(({ node }) => normalizeAuthor(node));
+
+export const getPageLayoutElement = () => document.querySelector('.layout-page');

app/assets/stylesheets/components/design_management/design.scss

+.layout-page.design-detail-layout {
+  max-height: 100vh;
+}
+
 .design-detail {
   background-color: rgba($black, 0.9);
 

changelogs/unreleased/design-view-scrolling-issue-bug.yml

+---
+title: Remove ability to scroll Issue while in Design View
+merge_request: 29881
+author:
+type: fixed

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

-import { shallowMount } from '@vue/test-utils';
+import { shallowMount, createLocalVue } from '@vue/test-utils';
+import VueRouter from 'vue-router';
 import { GlAlert } from '@gitlab/ui';
 import { ApolloMutation } from 'vue-apollo';
 import createFlash from '~/flash';
@@ -17,6 +18,9 @@ import {
   DESIGN_VERSION_NOT_EXIST_ERROR,
 } from '~/design_management/utils/error_messages';
 import { DESIGNS_ROUTE_NAME } from '~/design_management/router/constants';
+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';
 
 jest.mock('~/flash');
 jest.mock('mousetrap', () => ({
@@ -24,8 +28,13 @@ jest.mock('mousetrap', () => ({
   unbind: jest.fn(),
 }));
 
+const localVue = createLocalVue();
+localVue.use(VueRouter);
+
 describe('Design management design index page', () => {
   let wrapper;
+  let router;
+
   const newComment = 'new comment';
   const annotationCoordinates = {
     x: 10,
@@ -62,14 +71,13 @@ describe('Design management design index page', () => {
   };
 
   const mutate = jest.fn().mockResolvedValue();
-  const routerPush = jest.fn();
 
   const findDiscussions = () => wrapper.findAll(DesignDiscussion);
   const findDiscussionForm = () => wrapper.find(DesignReplyForm);
   const findParticipants = () => wrapper.find(Participants);
   const findDiscussionsWrapper = () => wrapper.find('.image-notes');
 
-  function createComponent(loading = false, data = {}, { routeQuery = {} } = {}) {
+  function createComponent(loading = false, data = {}) {
     const $apollo = {
       queries: {
         design: {
@@ -79,17 +87,11 @@ describe('Design management design index page', () => {
       mutate,
     };
 
-    const $router = {
-      push: routerPush,
-    };
-
-    const $route = {
-      query: routeQuery,
-    };
+    router = createRouter();
 
     wrapper = shallowMount(DesignIndex, {
       propsData: { id: '1' },
-      mocks: { $apollo, $router, $route },
+      mocks: { $apollo },
       stubs: {
         ApolloMutation,
         DesignDiscussion,
@@ -104,6 +106,8 @@ describe('Design management design index page', () => {
           ...data,
         };
       },
+      localVue,
+      router,
     });
   }
 
@@ -111,6 +115,23 @@ 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);
+      createComponent(true);
+
+      wrapper.vm.$router.push('/designs/test');
+      expect(mockEl.classList.add).toHaveBeenCalledTimes(1);
+      expect(mockEl.classList.add).toHaveBeenCalledWith(...DESIGN_DETAIL_LAYOUT_CLASSLIST);
+    });
+  });
+
   it('sets loading state', () => {
     createComponent(true);
 
@@ -269,31 +290,35 @@ describe('Design management design index page', () => {
     describe('with no designs', () => {
       it('redirects to /designs', () => {
         createComponent(true);
+        router.push = jest.fn();
 
         wrapper.vm.onDesignQueryResult({ data: mockResponseNoDesigns, loading: false });
         return wrapper.vm.$nextTick().then(() => {
           expect(createFlash).toHaveBeenCalledTimes(1);
           expect(createFlash).toHaveBeenCalledWith(DESIGN_NOT_FOUND_ERROR);
-          expect(routerPush).toHaveBeenCalledTimes(1);
-          expect(routerPush).toHaveBeenCalledWith({ name: DESIGNS_ROUTE_NAME });
+          expect(router.push).toHaveBeenCalledTimes(1);
+          expect(router.push).toHaveBeenCalledWith({ name: DESIGNS_ROUTE_NAME });
         });
       });
     });
 
     describe('when no design exists for given version', () => {
       it('redirects to /designs', () => {
-        // attempt to query for a version of the design that doesn't exist
-        createComponent(true, {}, { routeQuery: { version: '999' } });
+        createComponent(true);
         wrapper.setData({
           allVersions: mockAllVersions,
         });
 
+        // attempt to query for a version of the design that doesn't exist
+        router.push({ query: { version: '999' } });
+        router.push = jest.fn();
+
         wrapper.vm.onDesignQueryResult({ data: mockResponseWithDesigns, loading: false });
         return wrapper.vm.$nextTick().then(() => {
           expect(createFlash).toHaveBeenCalledTimes(1);
           expect(createFlash).toHaveBeenCalledWith(DESIGN_VERSION_NOT_EXIST_ERROR);
-          expect(routerPush).toHaveBeenCalledTimes(1);
-          expect(routerPush).toHaveBeenCalledWith({ name: DESIGNS_ROUTE_NAME });
+          expect(router.push).toHaveBeenCalledTimes(1);
+          expect(router.push).toHaveBeenCalledWith({ name: DESIGNS_ROUTE_NAME });
         });
       });
     });

spec/frontend/design_management/pages/index_spec.js

@@ -2,7 +2,6 @@ import { createLocalVue, shallowMount } from '@vue/test-utils';
 import { ApolloMutation } from 'vue-apollo';
 import VueRouter from 'vue-router';
 import { GlEmptyState } from '@gitlab/ui';
-
 import Index from '~/design_management/pages/index.vue';
 import uploadDesignQuery from '~/design_management/graphql/mutations/uploadDesign.mutation.graphql';
 import DesignDestroyer from '~/design_management/components/design_destroyer.vue';
@@ -14,20 +13,21 @@ import {
   EXISTING_DESIGN_DROP_INVALID_FILENAME_MESSAGE,
 } from '~/design_management/utils/error_messages';
 import 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';
 
-const localVue = createLocalVue();
-localVue.use(VueRouter);
-const router = new VueRouter({
-  routes: [
-    {
-      name: DESIGNS_ROUTE_NAME,
-      path: '/designs',
-      component: Index,
+jest.mock('~/flash.js');
+const mockPageEl = {
+  classList: {
+    remove: jest.fn(),
   },
-  ],
-});
+};
+jest.spyOn(utils, 'getPageLayoutElement').mockReturnValue(mockPageEl);
 
-jest.mock('~/flash.js');
+const localVue = createLocalVue();
+const router = createRouter();
+localVue.use(VueRouter);
 
 const mockDesigns = [
   {
@@ -530,4 +530,14 @@ describe('Design management index page', () => {
       expect(wrapper.vm.onUploadDesign).not.toHaveBeenCalled();
     });
   });
+
+  describe('when navigating', () => {
+    it('ensures fullscreen layout is not applied', () => {
+      createComponent(true);
+
+      wrapper.vm.$router.push('/designs');
+      expect(mockPageEl.classList.remove).toHaveBeenCalledTimes(1);
+      expect(mockPageEl.classList.remove).toHaveBeenCalledWith(...DESIGN_DETAIL_LAYOUT_CLASSLIST);
+    });
+  });
 });