diff --git a/app/assets/javascripts/diff.js b/app/assets/javascripts/diff.js index 23eb470503e8a51ce429c75517fdafad8d335301..6581649543253c3fb71b5d25d7eb8d84d9a0dfb8 100644 --- a/app/assets/javascripts/diff.js +++ b/app/assets/javascripts/diff.js @@ -12,9 +12,13 @@ const UNFOLD_COUNT = 20; let isBound = false; export default class Diff { - constructor() { + constructor({ mergeRequestEventHub } = {}) { const $diffFile = $('.files .diff-file'); + if (mergeRequestEventHub) { + this.mrHub = mergeRequestEventHub; + } + $diffFile.each((index, file) => { if (!$.data(file, 'singleFileDiff')) { $.data(file, 'singleFileDiff', new SingleFileDiff(file)); @@ -34,7 +38,8 @@ export default class Diff { $(document) .on('click', '.js-unfold', this.handleClickUnfold.bind(this)) .on('click', '.diff-line-num a', this.handleClickLineNum.bind(this)) - .on('mousedown', 'td.line_content.parallel', this.handleParallelLineDown.bind(this)); + .on('mousedown', 'td.line_content.parallel', this.handleParallelLineDown.bind(this)) + .on('click', '.inline-parallel-buttons a', ($e) => this.viewTypeSwitch($e)); isBound = true; } @@ -135,6 +140,20 @@ export default class Diff { diffViewType() { return $('.inline-parallel-buttons a.active').data('viewType'); } + viewTypeSwitch(event) { + const click = event.originalEvent; + const diffSource = new URL(click.target.getAttribute('href'), document.location.href); + + if (this.mrHub) { + click.preventDefault(); + click.stopPropagation(); + + diffSource.pathname = `${diffSource.pathname}.json`; + + this.mrHub.$emit('diff:switch-view-type', { source: diffSource.toString() }); + } + } + // eslint-disable-next-line class-methods-use-this lineNumbers(line) { const children = line.find('.diff-line-num').toArray(); diff --git a/app/assets/javascripts/init_diff_stats_dropdown.js b/app/assets/javascripts/init_diff_stats_dropdown.js index 27df761a10308f5bb69cfd53be3f280e5bbb5a9b..8413fe92f8948f1888a7c7bc83d2bdcb2a378b52 100644 --- a/app/assets/javascripts/init_diff_stats_dropdown.js +++ b/app/assets/javascripts/init_diff_stats_dropdown.js @@ -4,7 +4,7 @@ import { stickyMonitor } from './lib/utils/sticky'; export const initDiffStatsDropdown = (stickyTop) => { if (stickyTop) { - stickyMonitor(document.querySelector('.js-diff-files-changed'), stickyTop); + stickyMonitor(document.querySelector('.js-diff-files-changed'), stickyTop, false); } const el = document.querySelector('.js-diff-stats-dropdown'); diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 5a1410ceeba328dd132a18154abb0f826906d98b..46ee8fecfc57361b4e89457a2219865df72241c1 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -1,4 +1,4 @@ -/* eslint-disable no-new, class-methods-use-this */ +/* eslint-disable class-methods-use-this */ import $ from 'jquery'; import Vue from 'vue'; import { createAlert } from '~/flash'; @@ -134,8 +134,8 @@ function destroyPipelines(app) { return null; } -function loadDiffs({ url, sticky }) { - return axios.get(`${url}.json${location.search}`).then(({ data }) => { +function loadDiffs({ url, sticky, tabs }) { + return axios.get(url).then(({ data }) => { const $container = $('#diffs'); $container.html(data.html); initDiffStatsDropdown(sticky); @@ -143,7 +143,9 @@ function loadDiffs({ url, sticky }) { localTimeAgo(document.querySelectorAll('#diffs .js-timeago')); syntaxHighlight($('#diffs .js-syntax-highlight')); - new Diff(); + tabs.createDiff(); + tabs.setHubToDiff(); + scrollToContainer('#diffs'); $('.diff-file').each((i, el) => { @@ -204,6 +206,7 @@ export default class MergeRequestTabs { this.currentTab = null; this.diffsLoaded = false; + this.diffsClass = null; this.commitsLoaded = false; this.fixedLayoutPref = null; this.eventHub = createEventHub(); @@ -211,6 +214,7 @@ export default class MergeRequestTabs { this.setUrl = setUrl !== undefined ? setUrl : true; this.setCurrentAction = this.setCurrentAction.bind(this); + this.switchViewType = this.switchViewType.bind(this); this.tabShown = this.tabShown.bind(this); this.clickTab = this.clickTab.bind(this); @@ -230,11 +234,13 @@ export default class MergeRequestTabs { this.tabShown(action, location.href); this.eventHub.$emit('MergeRequestTabChange', action); }); + this.eventHub.$on('diff:switch-view-type', this.switchViewType); } // Used in tests unbindEvents() { $('.merge-request-tabs a[data-toggle="tabvue"]').off('click', this.clickTab); + this.eventHub.$off('diff:switch-view-type', this.switchViewType); } storeScroll() { @@ -341,7 +347,7 @@ export default class MergeRequestTabs { in practice, this only occurs when comparing commits in the new merge request form page. */ - this.loadDiff(href); + this.loadDiff({ endpoint: href, strip: true }); } // this.hideSidebar(); this.expandViewContainer(); @@ -503,17 +509,20 @@ export default class MergeRequestTabs { } // load the diff tab content from the backend - loadDiff(source) { + loadDiff({ endpoint, strip = true }) { if (this.diffsLoaded) { document.dispatchEvent(new CustomEvent('scroll')); return; } + // We extract pathname for the current Changes tab anchor href + // some pages like MergeRequestsController#new has query parameters on that anchor + const diffUrl = strip ? `${parseUrlPathname(endpoint)}.json${location.search}` : endpoint; + loadDiffs({ - // We extract pathname for the current Changes tab anchor href - // some pages like MergeRequestsController#new has query parameters on that anchor - url: parseUrlPathname(source), + url: diffUrl, sticky: computeTopOffset(this.mergeRequestTabs), + tabs: this, }) .then(() => { if (this.isDiffAction(this.currentAction)) { @@ -528,6 +537,21 @@ export default class MergeRequestTabs { }); }); } + switchViewType({ source }) { + this.diffsLoaded = false; + + this.loadDiff({ endpoint: source, strip: false }); + } + createDiff() { + if (!this.diffsClass) { + this.diffsClass = new Diff({ mergeRequestEventHub: this.eventHub }); + } + } + setHubToDiff() { + if (this.diffsClass) { + this.diffsClass.mrHub = this.eventHub; + } + } diffViewType() { return $('.js-diff-view-buttons button.active').data('viewType'); diff --git a/app/assets/javascripts/pages/projects/merge_requests/init_merge_request.js b/app/assets/javascripts/pages/projects/merge_requests/init_merge_request.js index a4e3ddfc50623f98257cc6db3fca5ba86a763a91..d4734b8842dfb7ad0cf7eeb722154b6c8b77172c 100644 --- a/app/assets/javascripts/pages/projects/merge_requests/init_merge_request.js +++ b/app/assets/javascripts/pages/projects/merge_requests/init_merge_request.js @@ -4,14 +4,12 @@ import $ from 'jquery'; import IssuableForm from 'ee_else_ce/issuable/issuable_form'; import IssuableLabelSelector from '~/issuable/issuable_label_selector'; import ShortcutsNavigation from '~/behaviors/shortcuts/shortcuts_navigation'; -import Diff from '~/diff'; import GLForm from '~/gl_form'; import LabelsSelect from '~/labels/labels_select'; import IssuableTemplateSelectors from '~/issuable/issuable_template_selectors'; import { mountMilestoneDropdown } from '~/sidebar/mount_sidebar'; export default () => { - new Diff(); new ShortcutsNavigation(); new GLForm($('.merge-request-form')); new IssuableForm($('.merge-request-form')); diff --git a/spec/frontend/diff_spec.js b/spec/frontend/diff_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..759ae32ac514efc0a8aa88f6e0604955259749a6 --- /dev/null +++ b/spec/frontend/diff_spec.js @@ -0,0 +1,72 @@ +import createEventHub from '~/helpers/event_hub_factory'; + +import Diff from '~/diff'; + +describe('Diff', () => { + describe('diff <-> tabs interactions', () => { + let hub; + + beforeEach(() => { + hub = createEventHub(); + }); + + describe('constructor', () => { + it("takes in the `mergeRequestEventHub` when it's provided", () => { + const diff = new Diff({ mergeRequestEventHub: hub }); + + expect(diff.mrHub).toBe(hub); + }); + + it('does not fatal if no event hub is provided', () => { + expect(() => { + new Diff(); /* eslint-disable-line no-new */ + }).not.toThrow(); + }); + + it("doesn't set the mrHub property if none is provided by the construction arguments", () => { + const diff = new Diff(); + + expect(diff.mrHub).toBe(undefined); + }); + }); + + describe('viewTypeSwitch', () => { + const clickPath = '/path/somewhere?params=exist'; + const jsonPath = 'http://test.host/path/somewhere.json?params=exist'; + const simulatejQueryClick = { + originalEvent: { + target: { + getAttribute() { + return clickPath; + }, + }, + preventDefault: jest.fn(), + stopPropagation: jest.fn(), + }, + }; + + it('emits the correct switch view event when called and there is an `mrHub`', async () => { + const diff = new Diff({ mergeRequestEventHub: hub }); + const hubEmit = new Promise((resolve) => { + hub.$on('diff:switch-view-type', resolve); + }); + + diff.viewTypeSwitch(simulatejQueryClick); + const { source } = await hubEmit; + + expect(simulatejQueryClick.originalEvent.preventDefault).toHaveBeenCalled(); + expect(simulatejQueryClick.originalEvent.stopPropagation).toHaveBeenCalled(); + expect(source).toBe(jsonPath); + }); + + it('is effectively a noop when there is no `mrHub`', () => { + const diff = new Diff(); + + expect(diff.mrHub).toBe(undefined); + expect(() => { + diff.viewTypeSwitch(simulatejQueryClick); + }).not.toThrow(); + }); + }); + }); +}); diff --git a/spec/frontend/merge_request_tabs_spec.js b/spec/frontend/merge_request_tabs_spec.js index 69ff5e47689849064b91e1d6ae9eec38cdc39494..6d434d7e654d3e3f106ff38b9bbcb32c13300be2 100644 --- a/spec/frontend/merge_request_tabs_spec.js +++ b/spec/frontend/merge_request_tabs_spec.js @@ -5,6 +5,7 @@ import initMrPage from 'helpers/init_vue_mr_page_helper'; import { stubPerformanceWebAPI } from 'helpers/performance'; import axios from '~/lib/utils/axios_utils'; import MergeRequestTabs from '~/merge_request_tabs'; +import Diff from '~/diff'; import '~/lib/utils/common_utils'; import '~/lib/utils/url_utility'; @@ -389,4 +390,73 @@ describe('MergeRequestTabs', () => { }); }); }); + + describe('tabs <-> diff interactions', () => { + beforeEach(() => { + jest.spyOn(testContext.class, 'loadDiff').mockImplementation(() => {}); + }); + + describe('switchViewType', () => { + it('marks the class as having not loaded diffs already', () => { + testContext.class.diffsLoaded = true; + + testContext.class.switchViewType({}); + + expect(testContext.class.diffsLoaded).toBe(false); + }); + + it('reloads the diffs', () => { + testContext.class.switchViewType({ source: 'a new url' }); + + expect(testContext.class.loadDiff).toHaveBeenCalledWith({ + endpoint: 'a new url', + strip: false, + }); + }); + }); + + describe('createDiff', () => { + it("creates a Diff if there isn't one", () => { + expect(testContext.class.diffsClass).toBe(null); + + testContext.class.createDiff(); + + expect(testContext.class.diffsClass).toBeInstanceOf(Diff); + }); + + it("doesn't create a Diff if one already exists", () => { + testContext.class.diffsClass = 'truthy'; + + testContext.class.createDiff(); + + expect(testContext.class.diffsClass).toBe('truthy'); + }); + + it('sets the available MR Tabs event hub to the new Diff', () => { + expect(testContext.class.diffsClass).toBe(null); + + testContext.class.createDiff(); + + expect(testContext.class.diffsClass.mrHub).toBe(testContext.class.eventHub); + }); + }); + + describe('setHubToDiff', () => { + it('sets the MR Tabs event hub to the child Diff', () => { + testContext.class.diffsClass = {}; + + testContext.class.setHubToDiff(); + + expect(testContext.class.diffsClass.mrHub).toBe(testContext.class.eventHub); + }); + + it('does not fatal if theres no child Diff', () => { + testContext.class.diffsClass = null; + + expect(() => { + testContext.class.setHubToDiff(); + }).not.toThrow(); + }); + }); + }); });