+
diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue
index b08b9df13a43d8b5a1a1dab9c71546e0fe13e8e2..ea7112689c144005b73161ae0133460f5e0201ce 100644
--- a/app/assets/javascripts/diffs/components/diff_file_header.vue
+++ b/app/assets/javascripts/diffs/components/diff_file_header.vue
@@ -18,6 +18,7 @@ import { __, s__, sprintf } from '~/locale';
import { diffViewerModes } from '~/ide/constants';
import DiffStats from './diff_stats.vue';
import { scrollToElement } from '~/lib/utils/common_utils';
+import { isCollapsed } from '../diff_file';
import { DIFF_FILE_HEADER } from '../i18n';
export default {
@@ -125,6 +126,9 @@ export default {
isUsingLfs() {
return this.diffFile.stored_externally && this.diffFile.external_storage === 'lfs';
},
+ isCollapsed() {
+ return isCollapsed(this.diffFile, { fileByFile: this.viewDiffsFileByFile });
+ },
collapseIcon() {
return this.expanded ? 'chevron-down' : 'chevron-right';
},
@@ -334,7 +338,7 @@ export default {
-
+
diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js
index dc97d9993da21bb6577b74348ca3dcdaef1c4d95..0735470bfa2e27775dcac54199fdc19e51b340f5 100644
--- a/app/assets/javascripts/diffs/constants.js
+++ b/app/assets/javascripts/diffs/constants.js
@@ -73,6 +73,10 @@ export const ALERT_OVERFLOW_HIDDEN = 'overflow';
export const ALERT_MERGE_CONFLICT = 'merge-conflict';
export const ALERT_COLLAPSED_FILES = 'collapsed';
+// Diff File collapse types
+export const DIFF_FILE_AUTOMATIC_COLLAPSE = 'automatic';
+export const DIFF_FILE_MANUAL_COLLAPSE = 'manual';
+
// State machine states
export const STATE_IDLING = 'idle';
export const STATE_LOADING = 'loading';
diff --git a/app/assets/javascripts/diffs/diff_file.js b/app/assets/javascripts/diffs/diff_file.js
index 933197a2c7f9f94d924d4a3d7da96a2d4f9975d7..a14a30b41a9cbe29cb752936e3cc867e6ed6a2d7 100644
--- a/app/assets/javascripts/diffs/diff_file.js
+++ b/app/assets/javascripts/diffs/diff_file.js
@@ -1,4 +1,9 @@
-import { DIFF_FILE_SYMLINK_MODE, DIFF_FILE_DELETED_MODE } from './constants';
+import {
+ DIFF_FILE_SYMLINK_MODE,
+ DIFF_FILE_DELETED_MODE,
+ DIFF_FILE_MANUAL_COLLAPSE,
+ DIFF_FILE_AUTOMATIC_COLLAPSE,
+} from './constants';
function fileSymlinkInformation(file, fileList) {
const duplicates = fileList.filter(iteratedFile => iteratedFile.file_hash === file.file_hash);
@@ -23,6 +28,7 @@ function collapsed(file) {
return {
automaticallyCollapsed: viewer.automaticallyCollapsed || viewer.collapsed || false,
+ manuallyCollapsed: null,
};
}
@@ -37,3 +43,19 @@ export function prepareRawDiffFile({ file, allFiles }) {
return file;
}
+
+export function collapsedType(file) {
+ const isManual = typeof file.viewer?.manuallyCollapsed === 'boolean';
+
+ return isManual ? DIFF_FILE_MANUAL_COLLAPSE : DIFF_FILE_AUTOMATIC_COLLAPSE;
+}
+
+export function isCollapsed(file) {
+ const type = collapsedType(file);
+ const collapsedStates = {
+ [DIFF_FILE_AUTOMATIC_COLLAPSE]: file.viewer?.automaticallyCollapsed || false,
+ [DIFF_FILE_MANUAL_COLLAPSE]: file.viewer?.manuallyCollapsed,
+ };
+
+ return collapsedStates[type];
+}
diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js
index 966b706fc31cc5288a5b79a689591c9d3f83761d..322051cff05cbff8382dce95d504a0273a364e6a 100644
--- a/app/assets/javascripts/diffs/store/actions.js
+++ b/app/assets/javascripts/diffs/store/actions.js
@@ -40,8 +40,11 @@ import {
DIFF_WHITESPACE_COOKIE_NAME,
SHOW_WHITESPACE,
NO_SHOW_WHITESPACE,
+ DIFF_FILE_MANUAL_COLLAPSE,
+ DIFF_FILE_AUTOMATIC_COLLAPSE,
} from '../constants';
import { diffViewerModes } from '~/ide/constants';
+import { isCollapsed } from '../diff_file';
export const setBaseConfig = ({ commit }, options) => {
const {
@@ -239,6 +242,13 @@ export const renderFileForDiscussionId = ({ commit, rootState, state }, discussi
if (file.viewer.automaticallyCollapsed) {
eventHub.$emit(`loadCollapsedDiff/${file.file_hash}`);
scrollToElement(document.getElementById(file.file_hash));
+ } else if (file.viewer.manuallyCollapsed) {
+ commit(types.SET_FILE_COLLAPSED, {
+ filePath: file.file_path,
+ collapsed: false,
+ trigger: DIFF_FILE_AUTOMATIC_COLLAPSE,
+ });
+ eventHub.$emit('scrollToDiscussion');
} else {
eventHub.$emit('scrollToDiscussion');
}
@@ -252,8 +262,7 @@ export const startRenderDiffsQueue = ({ state, commit }) => {
const nextFile = state.diffFiles.find(
file =>
!file.renderIt &&
- (file.viewer &&
- (!file.viewer.automaticallyCollapsed || file.viewer.name !== diffViewerModes.text)),
+ (file.viewer && (!isCollapsed(file) || file.viewer.name !== diffViewerModes.text)),
);
if (nextFile) {
@@ -641,8 +650,9 @@ export function switchToFullDiffFromRenamedFile({ commit, dispatch, state }, { d
});
}
-export const setFileCollapsed = ({ commit }, { filePath, collapsed }) =>
- commit(types.SET_FILE_COLLAPSED, { filePath, collapsed });
+export const setFileCollapsedByUser = ({ commit }, { filePath, collapsed }) => {
+ commit(types.SET_FILE_COLLAPSED, { filePath, collapsed, trigger: DIFF_FILE_MANUAL_COLLAPSE });
+};
export const setSuggestPopoverDismissed = ({ commit, state }) =>
axios
diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js
index 91425c7825bb890a495d34da38049dca8fee5687..4900dcd473d76b3477d80077054fb72e9f866d20 100644
--- a/app/assets/javascripts/diffs/store/getters.js
+++ b/app/assets/javascripts/diffs/store/getters.js
@@ -8,8 +8,16 @@ export const isParallelView = state => state.diffViewType === PARALLEL_DIFF_VIEW
export const isInlineView = state => state.diffViewType === INLINE_DIFF_VIEW_TYPE;
-export const hasCollapsedFile = state =>
- state.diffFiles.some(file => file.viewer && file.viewer.automaticallyCollapsed);
+export const whichCollapsedTypes = state => {
+ const automatic = state.diffFiles.some(file => file.viewer?.automaticallyCollapsed);
+ const manual = state.diffFiles.some(file => file.viewer?.manuallyCollapsed);
+
+ return {
+ any: automatic || manual,
+ automatic,
+ manual,
+ };
+};
export const commitId = state => (state.commit && state.commit.id ? state.commit.id : null);
diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js
index 13ecf6a997d884ec993a431af092a34193e0ca80..5328845e4d94575dee09b4a41e3c2320c0a9d765 100644
--- a/app/assets/javascripts/diffs/store/mutations.js
+++ b/app/assets/javascripts/diffs/store/mutations.js
@@ -1,6 +1,10 @@
import Vue from 'vue';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
-import { INLINE_DIFF_VIEW_TYPE } from '../constants';
+import {
+ DIFF_FILE_MANUAL_COLLAPSE,
+ DIFF_FILE_AUTOMATIC_COLLAPSE,
+ INLINE_DIFF_VIEW_TYPE,
+} from '../constants';
import {
findDiffFile,
addLineReferences,
@@ -16,6 +20,12 @@ function updateDiffFilesInState(state, files) {
return Object.assign(state, { diffFiles: files });
}
+function renderFile(file) {
+ Object.assign(file, {
+ renderIt: true,
+ });
+}
+
export default {
[types.SET_BASE_CONFIG](state, options) {
const {
@@ -81,9 +91,7 @@ export default {
},
[types.RENDER_FILE](state, file) {
- Object.assign(file, {
- renderIt: true,
- });
+ renderFile(file);
},
[types.SET_MERGE_REQUEST_DIFFS](state, mergeRequestDiffs) {
@@ -173,6 +181,7 @@ export default {
Object.assign(file, {
viewer: Object.assign(file.viewer, {
automaticallyCollapsed: false,
+ manuallyCollapsed: false,
}),
});
});
@@ -351,11 +360,24 @@ export default {
file.isShowingFullFile = true;
file.isLoadingFullFile = false;
},
- [types.SET_FILE_COLLAPSED](state, { filePath, collapsed }) {
+ [types.SET_FILE_COLLAPSED](
+ state,
+ { filePath, collapsed, trigger = DIFF_FILE_AUTOMATIC_COLLAPSE },
+ ) {
const file = state.diffFiles.find(f => f.file_path === filePath);
if (file && file.viewer) {
- file.viewer.automaticallyCollapsed = collapsed;
+ if (trigger === DIFF_FILE_MANUAL_COLLAPSE) {
+ file.viewer.automaticallyCollapsed = false;
+ file.viewer.manuallyCollapsed = collapsed;
+ } else if (trigger === DIFF_FILE_AUTOMATIC_COLLAPSE) {
+ file.viewer.automaticallyCollapsed = collapsed;
+ file.viewer.manuallyCollapsed = null;
+ }
+ }
+
+ if (file && !collapsed) {
+ renderFile(file);
}
},
[types.SET_HIDDEN_VIEW_DIFF_FILE_LINES](state, { filePath, lines }) {
diff --git a/app/assets/javascripts/notes/components/diff_with_note.vue b/app/assets/javascripts/notes/components/diff_with_note.vue
index a4271852563423b96d25dd6df53d33f2941afb15..91cf682943e29fcb75455e76a360a677b32f3de8 100644
--- a/app/assets/javascripts/notes/components/diff_with_note.vue
+++ b/app/assets/javascripts/notes/components/diff_with_note.vue
@@ -7,6 +7,7 @@ import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue';
import ImageDiffOverlay from '~/diffs/components/image_diff_overlay.vue';
import { getDiffMode } from '~/diffs/store/utils';
import { diffViewerModes } from '~/ide/constants';
+import { isCollapsed } from '../../diffs/diff_file';
const FIRST_CHAR_REGEX = /^(\+|-| )/;
@@ -46,6 +47,9 @@ export default {
this.discussion.truncated_diff_lines && this.discussion.truncated_diff_lines.length !== 0
);
},
+ isCollapsed() {
+ return isCollapsed(this.discussion.diff_file);
+ },
},
mounted() {
if (this.isTextFile && !this.hasTruncatedDiffLines) {
@@ -76,7 +80,7 @@ export default {
:discussion-path="discussion.discussion_path"
:diff-file="discussion.diff_file"
:can-current-user-fork="false"
- :expanded="!discussion.diff_file.viewer.automaticallyCollapsed"
+ :expanded="!isCollapsed"
/>
diff --git a/app/assets/stylesheets/framework/diffs.scss b/app/assets/stylesheets/framework/diffs.scss
index c0a2350d08074286f4409f671342549702891f1b..62c2e15fcc8e13022ce258820cff77c63abed903 100644
--- a/app/assets/stylesheets/framework/diffs.scss
+++ b/app/assets/stylesheets/framework/diffs.scss
@@ -6,11 +6,18 @@
border-top: 1px solid $border-color;
}
+ &.has-body {
+ .file-title {
+ box-shadow: 0 -2px 0 0 var(--white);
+ }
+ }
+
+ table.code tr:last-of-type td:last-of-type {
+ @include gl-rounded-bottom-right-base();
+ }
+
.file-title,
.file-title-flex-parent {
- border-top-left-radius: $border-radius-default;
- border-top-right-radius: $border-radius-default;
- box-shadow: 0 -2px 0 0 var(--white);
cursor: pointer;
.dropdown-menu {
@@ -113,7 +120,6 @@
.diff-content {
background: $white;
color: $gl-text-color;
- border-radius: 0 0 3px 3px;
.unfold {
cursor: pointer;
@@ -457,8 +463,11 @@ table.code {
border-top: 0;
}
- tr:nth-last-of-type(2).line_expansion > td {
- border-bottom: 0;
+ tr:nth-last-of-type(2).line_expansion,
+ tr:last-of-type.line_expansion {
+ > td {
+ border-bottom: 0;
+ }
}
tr.line_holder td {
diff --git a/app/assets/stylesheets/page_bundles/merge_requests.scss b/app/assets/stylesheets/page_bundles/merge_requests.scss
index 5553dffac05631544d39fb29742f4d9b823da8ed..be74503c21f2030a850e7eace3f1991aeff37ac6 100644
--- a/app/assets/stylesheets/page_bundles/merge_requests.scss
+++ b/app/assets/stylesheets/page_bundles/merge_requests.scss
@@ -11,9 +11,19 @@
}
.diff-tree-list {
+ // This 11px value should match the additional value found in
+ // /assets/stylesheets/framework/diffs.scss
+ // for the $mr-file-header-top SCSS variable within the
+ // .file-title,
+ // .file-title-flex-parent {
+ // rule.
+ // If they don't match, the file tree and the diff files stick
+ // to the top at different heights, which is a bad-looking defect
+ $diff-file-header-top: 11px;
+ $top-pos: $header-height + $mr-tabs-height + $mr-version-controls-height + $diff-file-header-top;
+
position: -webkit-sticky;
position: sticky;
- $top-pos: $header-height + $mr-tabs-height + $mr-version-controls-height + 15px;
top: $top-pos;
max-height: calc(100vh - #{$top-pos});
z-index: 202;
diff --git a/changelogs/unreleased/feature-collapsed-diff-files-user-collapsed-flag.yml b/changelogs/unreleased/feature-collapsed-diff-files-user-collapsed-flag.yml
new file mode 100644
index 0000000000000000000000000000000000000000..5c38a1eb4696f2ec8bc6c560331e8c7e7be4d25d
--- /dev/null
+++ b/changelogs/unreleased/feature-collapsed-diff-files-user-collapsed-flag.yml
@@ -0,0 +1,6 @@
+---
+title: Manually collapsed diff files are now significantly shorter and less visually
+ intrusive
+merge_request: 43911
+author:
+type: changed
diff --git a/spec/frontend/diffs/components/app_spec.js b/spec/frontend/diffs/components/app_spec.js
index 86560470ada6f01005dc3f76437bb74395615176..225710eab6338907c05b710733a11232b6d2705b 100644
--- a/spec/frontend/diffs/components/app_spec.js
+++ b/spec/frontend/diffs/components/app_spec.js
@@ -697,7 +697,7 @@ describe('diffs/components/app', () => {
});
describe('collapsed files', () => {
- it('should render the collapsed files warning if there are any collapsed files', () => {
+ it('should render the collapsed files warning if there are any automatically collapsed files', () => {
createComponent({}, ({ state }) => {
state.diffs.diffFiles = [{ viewer: { automaticallyCollapsed: true } }];
});
@@ -705,16 +705,14 @@ describe('diffs/components/app', () => {
expect(getCollapsedFilesWarning(wrapper).exists()).toBe(true);
});
- it('should not render the collapsed files warning if the user has dismissed the alert already', async () => {
+ it('should not render the collapsed files warning if there are no automatically collapsed files', () => {
createComponent({}, ({ state }) => {
- state.diffs.diffFiles = [{ viewer: { automaticallyCollapsed: true } }];
+ state.diffs.diffFiles = [
+ { viewer: { automaticallyCollapsed: false, manuallyCollapsed: true } },
+ { viewer: { automaticallyCollapsed: false, manuallyCollapsed: false } },
+ ];
});
- expect(getCollapsedFilesWarning(wrapper).exists()).toBe(true);
-
- wrapper.vm.collapsedWarningDismissed = true;
- await wrapper.vm.$nextTick();
-
expect(getCollapsedFilesWarning(wrapper).exists()).toBe(false);
});
});
diff --git a/spec/frontend/diffs/components/diff_file_header_spec.js b/spec/frontend/diffs/components/diff_file_header_spec.js
index a04486fc5c73abb085f8615743b6579e3ee89933..4db46b1029370c0488849e53a57c807c85d6173f 100644
--- a/spec/frontend/diffs/components/diff_file_header_spec.js
+++ b/spec/frontend/diffs/components/diff_file_header_spec.js
@@ -1,6 +1,9 @@
import { shallowMount, createLocalVue } from '@vue/test-utils';
import Vuex from 'vuex';
import { cloneDeep } from 'lodash';
+
+import { mockTracking, triggerEvent } from 'helpers/tracking_helper';
+
import DiffFileHeader from '~/diffs/components/diff_file_header.vue';
import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
import diffDiscussionsMockData from '../mock_data/diff_discussions';
@@ -136,9 +139,25 @@ describe('DiffFileHeader component', () => {
});
});
- it('displays a copy to clipboard button', () => {
- createComponent();
- expect(wrapper.find(ClipboardButton).exists()).toBe(true);
+ describe('copy to clipboard', () => {
+ beforeEach(() => {
+ createComponent();
+ });
+
+ it('displays a copy to clipboard button', () => {
+ expect(wrapper.find(ClipboardButton).exists()).toBe(true);
+ });
+
+ it('triggers the copy to clipboard tracking event', () => {
+ const trackingSpy = mockTracking('_category_', wrapper.vm.$el, jest.spyOn);
+
+ triggerEvent('[data-testid="diff-file-copy-clipboard"]');
+
+ expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_copy_file_button', {
+ label: 'diff_copy_file_path_button',
+ property: 'diff_copy_file',
+ });
+ });
});
describe('for submodule', () => {
diff --git a/spec/frontend/diffs/components/diff_file_spec.js b/spec/frontend/diffs/components/diff_file_spec.js
index a6f0d2bf11d7c317353c5c07afa2d04d77c7e923..59aa6d811eee4d97b380be4131f5bd5b727f477f 100644
--- a/spec/frontend/diffs/components/diff_file_spec.js
+++ b/spec/frontend/diffs/components/diff_file_spec.js
@@ -1,262 +1,317 @@
-import Vue from 'vue';
-import { createComponentWithStore } from 'helpers/vue_mount_component_helper';
-import { mockTracking, triggerEvent } from 'helpers/tracking_helper';
-import { createStore } from '~/mr_notes/stores';
-import DiffFileComponent from '~/diffs/components/diff_file.vue';
-import { diffViewerModes, diffViewerErrors } from '~/ide/constants';
+import Vuex from 'vuex';
+import { shallowMount, createLocalVue } from '@vue/test-utils';
+
+import createDiffsStore from '~/diffs/store/modules';
import diffFileMockDataReadable from '../mock_data/diff_file';
import diffFileMockDataUnreadable from '../mock_data/diff_file_unreadable';
-describe('DiffFile', () => {
- let vm;
- let trackingSpy;
+import DiffFileComponent from '~/diffs/components/diff_file.vue';
+import DiffFileHeaderComponent from '~/diffs/components/diff_file_header.vue';
+import DiffContentComponent from '~/diffs/components/diff_content.vue';
- beforeEach(() => {
- vm = createComponentWithStore(Vue.extend(DiffFileComponent), createStore(), {
- file: JSON.parse(JSON.stringify(diffFileMockDataReadable)),
+import { diffViewerModes, diffViewerErrors } from '~/ide/constants';
+
+function changeViewer(store, index, { automaticallyCollapsed, manuallyCollapsed, name }) {
+ const file = store.state.diffs.diffFiles[index];
+ const newViewer = {
+ ...file.viewer,
+ };
+
+ if (automaticallyCollapsed !== undefined) {
+ newViewer.automaticallyCollapsed = automaticallyCollapsed;
+ }
+
+ if (manuallyCollapsed !== undefined) {
+ newViewer.manuallyCollapsed = manuallyCollapsed;
+ }
+
+ if (name !== undefined) {
+ newViewer.name = name;
+ }
+
+ Object.assign(file, {
+ viewer: newViewer,
+ });
+}
+
+function forceHasDiff({ store, index = 0, inlineLines, parallelLines, readableText }) {
+ const file = store.state.diffs.diffFiles[index];
+
+ Object.assign(file, {
+ highlighted_diff_lines: inlineLines,
+ parallel_diff_lines: parallelLines,
+ blob: {
+ ...file.blob,
+ readable_text: readableText,
+ },
+ });
+}
+
+function markFileToBeRendered(store, index = 0) {
+ const file = store.state.diffs.diffFiles[index];
+
+ Object.assign(file, {
+ renderIt: true,
+ });
+}
+
+function createComponent({ file }) {
+ const localVue = createLocalVue();
+
+ localVue.use(Vuex);
+
+ const store = new Vuex.Store({
+ modules: {
+ diffs: createDiffsStore(),
+ },
+ });
+
+ store.state.diffs.diffFiles = [file];
+
+ const wrapper = shallowMount(DiffFileComponent, {
+ store,
+ localVue,
+ propsData: {
+ file,
canCurrentUserFork: false,
viewDiffsFileByFile: false,
- }).$mount();
- trackingSpy = mockTracking('_category_', vm.$el, jest.spyOn);
+ },
});
- afterEach(() => {
- vm.$destroy();
+ return {
+ localVue,
+ wrapper,
+ store,
+ };
+}
+
+const findDiffHeader = wrapper => wrapper.find(DiffFileHeaderComponent);
+const findDiffContentArea = wrapper => wrapper.find('[data-testid="content-area"]');
+const findLoader = wrapper => wrapper.find('[data-testid="loader-icon"]');
+const findToggleLinks = wrapper => wrapper.findAll('[data-testid="toggle-link"]');
+
+const toggleFile = wrapper => findDiffHeader(wrapper).vm.$emit('toggleFile');
+const isDisplayNone = element => element.style.display === 'none';
+const getReadableFile = () => JSON.parse(JSON.stringify(diffFileMockDataReadable));
+const getUnreadableFile = () => JSON.parse(JSON.stringify(diffFileMockDataUnreadable));
+
+const makeFileAutomaticallyCollapsed = (store, index = 0) =>
+ changeViewer(store, index, { automaticallyCollapsed: true, manuallyCollapsed: null });
+const makeFileOpenByDefault = (store, index = 0) =>
+ changeViewer(store, index, { automaticallyCollapsed: false, manuallyCollapsed: null });
+const makeFileManuallyCollapsed = (store, index = 0) =>
+ changeViewer(store, index, { automaticallyCollapsed: false, manuallyCollapsed: true });
+const changeViewerType = (store, newType, index = 0) =>
+ changeViewer(store, index, { name: diffViewerModes[newType] });
+
+describe('DiffFile', () => {
+ let wrapper;
+ let store;
+
+ beforeEach(() => {
+ ({ wrapper, store } = createComponent({ file: getReadableFile() }));
});
- const findDiffContent = () => vm.$el.querySelector('.diff-content');
- const isVisible = el => el.style.display !== 'none';
+ afterEach(() => {
+ wrapper.destroy();
+ });
describe('template', () => {
- it('should render component with file header, file content components', done => {
- const el = vm.$el;
- const { file_hash, file_path } = vm.file;
+ it('should render component with file header, file content components', async () => {
+ const el = wrapper.vm.$el;
+ const { file_hash } = wrapper.vm.file;
expect(el.id).toEqual(file_hash);
expect(el.classList.contains('diff-file')).toEqual(true);
expect(el.querySelectorAll('.diff-content.hidden').length).toEqual(0);
expect(el.querySelector('.js-file-title')).toBeDefined();
- expect(el.querySelector('[data-testid="diff-file-copy-clipboard"]')).toBeDefined();
- expect(el.querySelector('.file-title-name').innerText.indexOf(file_path)).toBeGreaterThan(-1);
+ expect(wrapper.find(DiffFileHeaderComponent).exists()).toBe(true);
expect(el.querySelector('.js-syntax-highlight')).toBeDefined();
- vm.file.renderIt = true;
-
- vm.$nextTick()
- .then(() => {
- expect(el.querySelectorAll('.line_content').length).toBe(8);
- expect(el.querySelectorAll('.js-line-expansion-content').length).toBe(1);
- triggerEvent('[data-testid="diff-file-copy-clipboard"]');
- })
- .then(done)
- .catch(done.fail);
- });
-
- it('should track a click event on copy to clip board button', done => {
- const el = vm.$el;
+ markFileToBeRendered(store);
- expect(el.querySelector('[data-testid="diff-file-copy-clipboard"]')).toBeDefined();
- vm.file.renderIt = true;
- vm.$nextTick()
- .then(() => {
- triggerEvent('[data-testid="diff-file-copy-clipboard"]');
+ await wrapper.vm.$nextTick();
- expect(trackingSpy).toHaveBeenCalledWith('_category_', 'click_copy_file_button', {
- label: 'diff_copy_file_path_button',
- property: 'diff_copy_file',
- });
- })
- .then(done)
- .catch(done.fail);
+ expect(wrapper.find(DiffContentComponent).exists()).toBe(true);
});
+ });
- describe('collapsed', () => {
- it('should not have file content', done => {
- expect(isVisible(findDiffContent())).toBe(true);
- expect(vm.isCollapsed).toEqual(false);
- vm.isCollapsed = true;
- vm.file.renderIt = true;
+ describe('collapsing', () => {
+ describe('user collapsed', () => {
+ beforeEach(() => {
+ makeFileManuallyCollapsed(store);
+ });
- vm.$nextTick(() => {
- expect(isVisible(findDiffContent())).toBe(false);
+ it('should not have any content at all', async () => {
+ await wrapper.vm.$nextTick();
- done();
+ Array.from(findDiffContentArea(wrapper).element.children).forEach(child => {
+ expect(isDisplayNone(child)).toBe(true);
});
});
- it('should have collapsed text and link', done => {
- vm.renderIt = true;
- vm.isCollapsed = true;
+ it('should not have the class `has-body` to present the header differently', () => {
+ expect(wrapper.classes('has-body')).toBe(false);
+ });
+ });
- vm.$nextTick(() => {
- expect(vm.$el.innerText).toContain('This diff is collapsed');
- expect(vm.$el.querySelectorAll('.js-click-to-expand').length).toEqual(1);
+ describe('automatically collapsed', () => {
+ beforeEach(() => {
+ makeFileAutomaticallyCollapsed(store);
+ });
- done();
- });
+ it('should show the collapsed file warning with expansion link', () => {
+ expect(findDiffContentArea(wrapper).html()).toContain('This diff is collapsed');
+ expect(findToggleLinks(wrapper).length).toEqual(1);
});
- it('should have collapsed text and link even before rendered', done => {
- vm.renderIt = false;
- vm.isCollapsed = true;
+ it('should style the component so that it `.has-body` for layout purposes', () => {
+ expect(wrapper.classes('has-body')).toBe(true);
+ });
+ });
- vm.$nextTick(() => {
- expect(vm.$el.innerText).toContain('This diff is collapsed');
- expect(vm.$el.querySelectorAll('.js-click-to-expand').length).toEqual(1);
+ describe('not collapsed', () => {
+ beforeEach(() => {
+ makeFileOpenByDefault(store);
+ markFileToBeRendered(store);
+ });
- done();
- });
+ it('should have the file content', async () => {
+ expect(wrapper.find(DiffContentComponent).exists()).toBe(true);
});
- it('should be collapsable for unreadable files', done => {
- vm.$destroy();
- vm = createComponentWithStore(Vue.extend(DiffFileComponent), createStore(), {
- file: JSON.parse(JSON.stringify(diffFileMockDataUnreadable)),
- canCurrentUserFork: false,
- viewDiffsFileByFile: false,
- }).$mount();
+ it('should style the component so that it `.has-body` for layout purposes', () => {
+ expect(wrapper.classes('has-body')).toBe(true);
+ });
+ });
- vm.renderIt = false;
- vm.isCollapsed = true;
+ describe('toggle', () => {
+ it('should update store state', async () => {
+ jest.spyOn(wrapper.vm.$store, 'dispatch').mockImplementation(() => {});
- vm.$nextTick(() => {
- expect(vm.$el.innerText).toContain('This diff is collapsed');
- expect(vm.$el.querySelectorAll('.js-click-to-expand').length).toEqual(1);
+ toggleFile(wrapper);
- done();
+ expect(wrapper.vm.$store.dispatch).toHaveBeenCalledWith('diffs/setFileCollapsedByUser', {
+ filePath: wrapper.vm.file.file_path,
+ collapsed: true,
});
});
- it('should be collapsed for renamed files', done => {
- vm.renderIt = true;
- vm.isCollapsed = false;
- vm.file.highlighted_diff_lines = null;
- vm.file.viewer.name = diffViewerModes.renamed;
-
- vm.$nextTick(() => {
- expect(vm.$el.innerText).not.toContain('This diff is collapsed');
+ describe('fetch collapsed diff', () => {
+ const prepFile = async (inlineLines, parallelLines, readableText) => {
+ forceHasDiff({
+ store,
+ inlineLines,
+ parallelLines,
+ readableText,
+ });
- done();
- });
- });
+ await wrapper.vm.$nextTick();
- it('should be collapsed for mode changed files', done => {
- vm.renderIt = true;
- vm.isCollapsed = false;
- vm.file.highlighted_diff_lines = null;
- vm.file.viewer.name = diffViewerModes.mode_changed;
+ toggleFile(wrapper);
+ };
- vm.$nextTick(() => {
- expect(vm.$el.innerText).not.toContain('This diff is collapsed');
+ beforeEach(() => {
+ jest.spyOn(wrapper.vm, 'requestDiff').mockImplementation(() => {});
- done();
+ makeFileAutomaticallyCollapsed(store);
});
- });
- it('should have loading icon while loading a collapsed diffs', done => {
- vm.isCollapsed = true;
- vm.isLoadingCollapsedDiff = true;
+ it.each`
+ inlineLines | parallelLines | readableText
+ ${[1]} | ${[1]} | ${true}
+ ${[]} | ${[1]} | ${true}
+ ${[1]} | ${[]} | ${true}
+ ${[1]} | ${[1]} | ${false}
+ ${[]} | ${[]} | ${false}
+ `(
+ 'does not make a request to fetch the diff for a diff file like { inline: $inlineLines, parallel: $parallelLines, readableText: $readableText }',
+ async ({ inlineLines, parallelLines, readableText }) => {
+ await prepFile(inlineLines, parallelLines, readableText);
+
+ expect(wrapper.vm.requestDiff).not.toHaveBeenCalled();
+ },
+ );
- vm.$nextTick(() => {
- expect(vm.$el.querySelectorAll('.diff-content.loading').length).toEqual(1);
+ it.each`
+ inlineLines | parallelLines | readableText
+ ${[]} | ${[]} | ${true}
+ `(
+ 'makes a request to fetch the diff for a diff file like { inline: $inlineLines, parallel: $parallelLines, readableText: $readableText }',
+ async ({ inlineLines, parallelLines, readableText }) => {
+ await prepFile(inlineLines, parallelLines, readableText);
- done();
- });
+ expect(wrapper.vm.requestDiff).toHaveBeenCalled();
+ },
+ );
});
+ });
- it('should update store state', done => {
- jest.spyOn(vm.$store, 'dispatch').mockImplementation(() => {});
-
- vm.isCollapsed = true;
+ describe('loading', () => {
+ it('should have loading icon while loading a collapsed diffs', async () => {
+ makeFileAutomaticallyCollapsed(store);
+ wrapper.vm.isLoadingCollapsedDiff = true;
- vm.$nextTick(() => {
- expect(vm.$store.dispatch).toHaveBeenCalledWith('diffs/setFileCollapsed', {
- filePath: vm.file.file_path,
- collapsed: true,
- });
+ await wrapper.vm.$nextTick();
- done();
- });
+ expect(findLoader(wrapper).exists()).toBe(true);
});
+ });
- it('updates local state when changing file state', done => {
- vm.file.viewer.automaticallyCollapsed = true;
+ describe('general (other) collapsed', () => {
+ it('should be expandable for unreadable files', async () => {
+ ({ wrapper, store } = createComponent({ file: getUnreadableFile() }));
+ makeFileAutomaticallyCollapsed(store);
- vm.$nextTick(() => {
- expect(vm.isCollapsed).toBe(true);
+ await wrapper.vm.$nextTick();
- done();
- });
+ expect(findDiffContentArea(wrapper).html()).toContain('This diff is collapsed');
+ expect(findToggleLinks(wrapper).length).toEqual(1);
});
+
+ it.each`
+ mode
+ ${'renamed'}
+ ${'mode_changed'}
+ `(
+ 'should render the DiffContent component for files whose mode is $mode',
+ async ({ mode }) => {
+ makeFileOpenByDefault(store);
+ markFileToBeRendered(store);
+ changeViewerType(store, mode);
+
+ await wrapper.vm.$nextTick();
+
+ expect(wrapper.classes('has-body')).toBe(true);
+ expect(wrapper.find(DiffContentComponent).exists()).toBe(true);
+ expect(wrapper.find(DiffContentComponent).isVisible()).toBe(true);
+ },
+ );
});
});
describe('too large diff', () => {
- it('should have too large warning and blob link', done => {
+ it('should have too large warning and blob link', async () => {
+ const file = store.state.diffs.diffFiles[0];
const BLOB_LINK = '/file/view/path';
- vm.file.viewer.error = diffViewerErrors.too_large;
- vm.file.viewer.error_message =
- 'This source diff could not be displayed because it is too large';
- vm.file.view_path = BLOB_LINK;
- vm.file.renderIt = true;
-
- vm.$nextTick(() => {
- expect(vm.$el.innerText).toContain(
- 'This source diff could not be displayed because it is too large',
- );
- done();
+ Object.assign(store.state.diffs.diffFiles[0], {
+ ...file,
+ view_path: BLOB_LINK,
+ renderIt: true,
+ viewer: {
+ ...file.viewer,
+ error: diffViewerErrors.too_large,
+ error_message: 'This source diff could not be displayed because it is too large',
+ },
});
- });
- });
- describe('watch collapsed', () => {
- it('calls handleLoadCollapsedDiff if collapsed changed & file has no lines', done => {
- jest.spyOn(vm, 'handleLoadCollapsedDiff').mockImplementation(() => {});
-
- vm.file.highlighted_diff_lines = [];
- vm.file.parallel_diff_lines = [];
- vm.isCollapsed = true;
-
- vm.$nextTick()
- .then(() => {
- vm.isCollapsed = false;
-
- return vm.$nextTick();
- })
- .then(() => {
- expect(vm.handleLoadCollapsedDiff).toHaveBeenCalled();
- })
- .then(done)
- .catch(done.fail);
- });
+ await wrapper.vm.$nextTick();
- it('does not call handleLoadCollapsedDiff if collapsed changed & file is unreadable', done => {
- vm.$destroy();
- vm = createComponentWithStore(Vue.extend(DiffFileComponent), createStore(), {
- file: JSON.parse(JSON.stringify(diffFileMockDataUnreadable)),
- canCurrentUserFork: false,
- viewDiffsFileByFile: false,
- }).$mount();
-
- jest.spyOn(vm, 'handleLoadCollapsedDiff').mockImplementation(() => {});
-
- vm.file.highlighted_diff_lines = [];
- vm.file.parallel_diff_lines = undefined;
- vm.isCollapsed = true;
-
- vm.$nextTick()
- .then(() => {
- vm.isCollapsed = false;
-
- return vm.$nextTick();
- })
- .then(() => {
- expect(vm.handleLoadCollapsedDiff).not.toHaveBeenCalled();
- })
- .then(done)
- .catch(done.fail);
+ expect(wrapper.vm.$el.innerText).toContain(
+ 'This source diff could not be displayed because it is too large',
+ );
});
});
});
diff --git a/spec/frontend/diffs/mock_data/diff_file.js b/spec/frontend/diffs/mock_data/diff_file.js
index d3886819a911d83400eed0705138805efbea9a54..cef776c885a11e7e67a28c83603ab45bbf55a980 100644
--- a/spec/frontend/diffs/mock_data/diff_file.js
+++ b/spec/frontend/diffs/mock_data/diff_file.js
@@ -27,6 +27,7 @@ export default {
name: 'text',
error: null,
automaticallyCollapsed: false,
+ manuallyCollapsed: null,
},
added_lines: 2,
removed_lines: 0,
diff --git a/spec/frontend/diffs/mock_data/diff_file_unreadable.js b/spec/frontend/diffs/mock_data/diff_file_unreadable.js
index f6cdca9950a5bda2bdd0dd933d92fe2c55efab78..2a5d694e3b8102b87939f467c5ac1bebba3cd42b 100644
--- a/spec/frontend/diffs/mock_data/diff_file_unreadable.js
+++ b/spec/frontend/diffs/mock_data/diff_file_unreadable.js
@@ -26,6 +26,7 @@ export default {
name: 'text',
error: null,
automaticallyCollapsed: false,
+ manuallyCollapsed: null,
},
added_lines: 0,
removed_lines: 0,
diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js
index c3e4ee9c531c2375a890fea1dcb18cfafb97c6a7..d41c4ae44ebddd7653baca2de95ce75c5cf5d089 100644
--- a/spec/frontend/diffs/store/actions_spec.js
+++ b/spec/frontend/diffs/store/actions_spec.js
@@ -42,7 +42,7 @@ import {
fetchFullDiff,
toggleFullDiff,
switchToFullDiffFromRenamedFile,
- setFileCollapsed,
+ setFileCollapsedByUser,
setExpandedDiffLines,
setSuggestPopoverDismissed,
changeCurrentCommit,
@@ -1216,13 +1216,18 @@ describe('DiffsStoreActions', () => {
});
});
- describe('setFileCollapsed', () => {
+ describe('setFileUserCollapsed', () => {
it('commits SET_FILE_COLLAPSED', done => {
testAction(
- setFileCollapsed,
+ setFileCollapsedByUser,
{ filePath: 'test', collapsed: true },
null,
- [{ type: types.SET_FILE_COLLAPSED, payload: { filePath: 'test', collapsed: true } }],
+ [
+ {
+ type: types.SET_FILE_COLLAPSED,
+ payload: { filePath: 'test', collapsed: true, trigger: 'manual' },
+ },
+ ],
[],
done,
);
diff --git a/spec/frontend/diffs/store/getters_spec.js b/spec/frontend/diffs/store/getters_spec.js
index 0083f1d8b4466c83fc291fabac7e6d705ca7803b..7e936c561fc0977f4e83213197d2be3854a5068b 100644
--- a/spec/frontend/diffs/store/getters_spec.js
+++ b/spec/frontend/diffs/store/getters_spec.js
@@ -49,23 +49,53 @@ describe('Diffs Module Getters', () => {
});
});
- describe('hasCollapsedFile', () => {
- it('returns true when all files are collapsed', () => {
- localState.diffFiles = [
- { viewer: { automaticallyCollapsed: true } },
- { viewer: { automaticallyCollapsed: true } },
- ];
+ describe('whichCollapsedTypes', () => {
+ const autoCollapsedFile = { viewer: { automaticallyCollapsed: true, manuallyCollapsed: null } };
+ const manuallyCollapsedFile = {
+ viewer: { automaticallyCollapsed: false, manuallyCollapsed: true },
+ };
+ const openFile = { viewer: { automaticallyCollapsed: false, manuallyCollapsed: false } };
+
+ it.each`
+ description | value | files
+ ${'all files are automatically collapsed'} | ${true} | ${[{ ...autoCollapsedFile }, { ...autoCollapsedFile }]}
+ ${'all files are manually collapsed'} | ${true} | ${[{ ...manuallyCollapsedFile }, { ...manuallyCollapsedFile }]}
+ ${'no files are collapsed in any way'} | ${false} | ${[{ ...openFile }, { ...openFile }]}
+ ${'some files are collapsed in either way'} | ${true} | ${[{ ...manuallyCollapsedFile }, { ...autoCollapsedFile }, { ...openFile }]}
+ `('`any` is $value when $description', ({ value, files }) => {
+ localState.diffFiles = files;
+
+ const getterResult = getters.whichCollapsedTypes(localState);
+
+ expect(getterResult.any).toEqual(value);
+ });
+
+ it.each`
+ description | value | files
+ ${'all files are automatically collapsed'} | ${true} | ${[{ ...autoCollapsedFile }, { ...autoCollapsedFile }]}
+ ${'all files are manually collapsed'} | ${false} | ${[{ ...manuallyCollapsedFile }, { ...manuallyCollapsedFile }]}
+ ${'no files are collapsed in any way'} | ${false} | ${[{ ...openFile }, { ...openFile }]}
+ ${'some files are collapsed in either way'} | ${true} | ${[{ ...manuallyCollapsedFile }, { ...autoCollapsedFile }, { ...openFile }]}
+ `('`automatic` is $value when $description', ({ value, files }) => {
+ localState.diffFiles = files;
- expect(getters.hasCollapsedFile(localState)).toEqual(true);
+ const getterResult = getters.whichCollapsedTypes(localState);
+
+ expect(getterResult.automatic).toEqual(value);
});
- it('returns true when at least one file is collapsed', () => {
- localState.diffFiles = [
- { viewer: { automaticallyCollapsed: false } },
- { viewer: { automaticallyCollapsed: true } },
- ];
+ it.each`
+ description | value | files
+ ${'all files are automatically collapsed'} | ${false} | ${[{ ...autoCollapsedFile }, { ...autoCollapsedFile }]}
+ ${'all files are manually collapsed'} | ${true} | ${[{ ...manuallyCollapsedFile }, { ...manuallyCollapsedFile }]}
+ ${'no files are collapsed in any way'} | ${false} | ${[{ ...openFile }, { ...openFile }]}
+ ${'some files are collapsed in either way'} | ${true} | ${[{ ...manuallyCollapsedFile }, { ...autoCollapsedFile }, { ...openFile }]}
+ `('`manual` is $value when $description', ({ value, files }) => {
+ localState.diffFiles = files;
+
+ const getterResult = getters.whichCollapsedTypes(localState);
- expect(getters.hasCollapsedFile(localState)).toEqual(true);
+ expect(getterResult.manual).toEqual(value);
});
});