From a58413f98db6ee90f8da3f3c2cd80b266472202e Mon Sep 17 00:00:00 2001 From: Fatih Acet Date: Mon, 19 Mar 2018 15:29:56 +0300 Subject: MR Diffs Refactor Part 02: Minor changes for refactor. --- app/assets/javascripts/autosave.js | 4 +- app/assets/javascripts/awards_handler.js | 8 +-- app/assets/javascripts/lib/utils/common_utils.js | 16 ++++-- app/assets/javascripts/merge_request.js | 1 + app/assets/javascripts/merge_request_tabs.js | 13 ++++- spec/javascripts/lib/utils/common_utils_spec.js | 70 ++++++++++++++++++++++++ 6 files changed, 100 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/autosave.js b/app/assets/javascripts/autosave.js index 0da872db7e5..3153f5156db 100644 --- a/app/assets/javascripts/autosave.js +++ b/app/assets/javascripts/autosave.js @@ -31,7 +31,9 @@ export default class Autosave { // https://github.com/vuejs/vue/issues/2804#issuecomment-216968137 const event = new Event('change', { bubbles: true, cancelable: false }); const field = this.field.get(0); - field.dispatchEvent(event); + if (field) { + field.dispatchEvent(event); + } } save() { diff --git a/app/assets/javascripts/awards_handler.js b/app/assets/javascripts/awards_handler.js index 6da33a26e58..06e2ac66152 100644 --- a/app/assets/javascripts/awards_handler.js +++ b/app/assets/javascripts/awards_handler.js @@ -4,7 +4,7 @@ import $ from 'jquery'; import _ from 'underscore'; import Cookies from 'js-cookie'; import { __ } from './locale'; -import { isInIssuePage, isInMRPage, hasVueMRDiscussionsCookie, updateTooltipTitle } from './lib/utils/common_utils'; +import { isInIssuePage, isInMRPage, updateTooltipTitle } from './lib/utils/common_utils'; import flash from './flash'; import axios from './lib/utils/axios_utils'; @@ -295,12 +295,8 @@ class AwardsHandler { } } - isVueMRDiscussions() { - return isInMRPage() && hasVueMRDiscussionsCookie() && !$('#diffs').is(':visible'); - } - isInVueNoteablePage() { - return isInIssuePage() || this.isVueMRDiscussions(); + return isInIssuePage() || isInMRPage(); } getVotesBlock() { diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index 0830ebe9e4e..081c8c48365 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -3,6 +3,7 @@ import Cookies from 'js-cookie'; import axios from './axios_utils'; import { getLocationHash } from './url_utility'; import { convertToCamelCase } from './text_utility'; +import { isObject } from './type_utility'; export const getPagePath = (index = 0) => $('body').attr('data-page').split(':')[index]; @@ -78,7 +79,7 @@ export const handleLocationHash = () => { const target = document.getElementById(hash) || document.getElementById(`user-content-${hash}`); const fixedTabs = document.querySelector('.js-tabs-affix'); - const fixedDiffStats = document.querySelector('.js-diff-files-changed.is-stuck'); + const fixedDiffStats = document.querySelector('.js-diff-files-changed'); const fixedNav = document.querySelector('.navbar-gitlab'); let adjustment = 0; @@ -422,17 +423,24 @@ export const spriteIcon = (icon, className = '') => { * Reasoning for this method is to ensure consistent property * naming conventions across JS code. */ -export const convertObjectPropsToCamelCase = (obj = {}) => { +export const convertObjectPropsToCamelCase = (obj = {}, options = {}) => { if (obj === null) { return {}; } + const initial = Array.isArray(obj) ? [] : {}; + return Object.keys(obj).reduce((acc, prop) => { const result = acc; + const val = obj[prop]; - result[convertToCamelCase(prop)] = obj[prop]; + if (options.deep && (isObject(val) || Array.isArray(val))) { + result[convertToCamelCase(prop)] = convertObjectPropsToCamelCase(val, options); + } else { + result[convertToCamelCase(prop)] = obj[prop]; + } return acc; - }, {}); + }, initial); }; export const imagePath = imgUrl => `${gon.asset_host || ''}${gon.relative_url_root || ''}/assets/${imgUrl}`; diff --git a/app/assets/javascripts/merge_request.js b/app/assets/javascripts/merge_request.js index d8222ebec63..d8960c212b4 100644 --- a/app/assets/javascripts/merge_request.js +++ b/app/assets/javascripts/merge_request.js @@ -49,6 +49,7 @@ MergeRequest.prototype.initTabs = function() { if (window.mrTabs) { window.mrTabs.unbindEvents(); } + window.mrTabs = new MergeRequestTabs(this.opts); }; diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index e77318fef46..a98b0b1c0b1 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -1,6 +1,7 @@ /* eslint-disable no-new, class-methods-use-this */ import $ from 'jquery'; +import Vue from 'vue'; import Cookies from 'js-cookie'; import axios from './lib/utils/axios_utils'; import flash from './flash'; @@ -11,6 +12,7 @@ import { parseUrlPathname, handleLocationHash, isMetaClick, + hasVueMRDiscussionsCookie, } from './lib/utils/common_utils'; import { getLocationHash } from './lib/utils/url_utility'; import initDiscussionTab from './image_diff/init_discussion_tab'; @@ -80,6 +82,7 @@ export default class MergeRequestTabs { this.pipelinesLoaded = false; this.commitsLoaded = false; this.fixedLayoutPref = null; + this.eventHub = new Vue(); this.setUrl = setUrl !== undefined ? setUrl : true; this.setCurrentAction = this.setCurrentAction.bind(this); @@ -156,7 +159,10 @@ export default class MergeRequestTabs { this.resetViewContainer(); this.destroyPipelinesView(); } else if (this.isDiffAction(action)) { - this.loadDiff($target.attr('href')); + if (!hasVueMRDiscussionsCookie()) { + this.loadDiff($target.attr('href')); + } + if (bp.getBreakpointSize() !== 'lg') { this.shrinkView(); } @@ -164,6 +170,7 @@ export default class MergeRequestTabs { this.expandViewContainer(); } this.destroyPipelinesView(); + $('.tab-content .commits.tab-pane').removeClass('active'); } else if (action === 'pipelines') { this.resetViewContainer(); this.mountPipelinesView(); @@ -179,6 +186,8 @@ export default class MergeRequestTabs { if (this.setUrl) { this.setCurrentAction(action); } + + this.eventHub.$emit('MergeRequestTabChange', this.getCurrentAction()); } scrollToElement(container) { @@ -291,6 +300,8 @@ export default class MergeRequestTabs { pipelineTableViewEl.appendChild(this.commitPipelinesTable.$el); } + // TODO: @fatihacet + // Delete this method later. It's not being called anymore but here for reference for refactor. loadDiff(source) { if (this.diffsLoaded) { document.dispatchEvent(new CustomEvent('scroll')); diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js index 27f06573432..bf18d9eff69 100644 --- a/spec/javascripts/lib/utils/common_utils_spec.js +++ b/spec/javascripts/lib/utils/common_utils_spec.js @@ -523,5 +523,75 @@ describe('common_utils', () => { expect(Object.keys(commonUtils.convertObjectPropsToCamelCase()).length).toBe(0); expect(Object.keys(commonUtils.convertObjectPropsToCamelCase({})).length).toBe(0); }); + + it('does not deep-convert by default', () => { + const obj = { + snake_key: { + child_snake_key: 'value', + }, + }; + + expect( + commonUtils.convertObjectPropsToCamelCase(obj), + ).toEqual({ + snakeKey: { + child_snake_key: 'value', + }, + }); + }); + + describe('deep: true', () => { + it('converts object with child objects', () => { + const obj = { + snake_key: { + child_snake_key: 'value', + }, + }; + + expect( + commonUtils.convertObjectPropsToCamelCase(obj, { deep: true }), + ).toEqual({ + snakeKey: { + childSnakeKey: 'value', + }, + }); + }); + + it('converts array with child objects', () => { + const arr = [ + { + child_snake_key: 'value', + }, + ]; + + expect( + commonUtils.convertObjectPropsToCamelCase(arr, { deep: true }), + ).toEqual([ + { + childSnakeKey: 'value', + }, + ]); + }); + + it('converts array with child arrays', () => { + const arr = [ + [ + { + child_snake_key: 'value', + }, + ], + ]; + + expect( + commonUtils.convertObjectPropsToCamelCase(arr, { deep: true }), + ).toEqual([ + [ + { + childSnakeKey: 'value', + }, + ], + ]); + }); + }); }); }); -- cgit v1.2.1