Commit 6efa1c01 authored by Phil Hughes's avatar Phil Hughes

Merge branch '16047-highlight-auto-collapsed-diffs-in-merge-request-diffs' into 'master'

Update diff file `collapsed` value to more accurate `automaticallyCollapsed`

See merge request gitlab-org/gitlab!43296
parents 9e4d74e9 cba87e5c
...@@ -44,7 +44,7 @@ export default { ...@@ -44,7 +44,7 @@ export default {
return { return {
isLoadingCollapsedDiff: false, isLoadingCollapsedDiff: false,
forkMessageVisible: false, forkMessageVisible: false,
isCollapsed: this.file.viewer.collapsed || false, isCollapsed: this.file.viewer.automaticallyCollapsed || false,
}; };
}, },
computed: { computed: {
...@@ -96,16 +96,16 @@ export default { ...@@ -96,16 +96,16 @@ export default {
}, },
'file.file_hash': { 'file.file_hash': {
handler: function watchFileHash() { handler: function watchFileHash() {
if (this.viewDiffsFileByFile && this.file.viewer.collapsed) { if (this.viewDiffsFileByFile && this.file.viewer.automaticallyCollapsed) {
this.isCollapsed = false; this.isCollapsed = false;
this.handleLoadCollapsedDiff(); this.handleLoadCollapsedDiff();
} else { } else {
this.isCollapsed = this.file.viewer.collapsed || false; this.isCollapsed = this.file.viewer.automaticallyCollapsed || false;
} }
}, },
immediate: true, immediate: true,
}, },
'file.viewer.collapsed': function setIsCollapsed(newVal) { 'file.viewer.automaticallyCollapsed': function setIsCollapsed(newVal) {
if (!this.viewDiffsFileByFile) { if (!this.viewDiffsFileByFile) {
this.isCollapsed = newVal; this.isCollapsed = newVal;
} }
......
...@@ -330,7 +330,7 @@ export default { ...@@ -330,7 +330,7 @@ export default {
</gl-dropdown-item> </gl-dropdown-item>
</template> </template>
<template v-if="!diffFile.viewer.collapsed"> <template v-if="!diffFile.viewer.automaticallyCollapsed">
<gl-dropdown-divider <gl-dropdown-divider
v-if="!diffFile.is_fully_expanded || diffHasDiscussions(diffFile)" v-if="!diffFile.is_fully_expanded || diffHasDiscussions(diffFile)"
/> />
......
...@@ -18,9 +18,21 @@ function fileSymlinkInformation(file, fileList) { ...@@ -18,9 +18,21 @@ function fileSymlinkInformation(file, fileList) {
); );
} }
function collapsed(file) {
const viewer = file.viewer || {};
return {
automaticallyCollapsed: viewer.automaticallyCollapsed || viewer.collapsed || false,
};
}
export function prepareRawDiffFile({ file, allFiles }) { export function prepareRawDiffFile({ file, allFiles }) {
Object.assign(file, { Object.assign(file, {
brokenSymlink: fileSymlinkInformation(file, allFiles), brokenSymlink: fileSymlinkInformation(file, allFiles),
viewer: {
...file.viewer,
...collapsed(file),
},
}); });
return file; return file;
......
...@@ -236,7 +236,7 @@ export const renderFileForDiscussionId = ({ commit, rootState, state }, discussi ...@@ -236,7 +236,7 @@ export const renderFileForDiscussionId = ({ commit, rootState, state }, discussi
commit(types.RENDER_FILE, file); commit(types.RENDER_FILE, file);
} }
if (file.viewer.collapsed) { if (file.viewer.automaticallyCollapsed) {
eventHub.$emit(`loadCollapsedDiff/${file.file_hash}`); eventHub.$emit(`loadCollapsedDiff/${file.file_hash}`);
scrollToElement(document.getElementById(file.file_hash)); scrollToElement(document.getElementById(file.file_hash));
} else { } else {
...@@ -252,7 +252,8 @@ export const startRenderDiffsQueue = ({ state, commit }) => { ...@@ -252,7 +252,8 @@ export const startRenderDiffsQueue = ({ state, commit }) => {
const nextFile = state.diffFiles.find( const nextFile = state.diffFiles.find(
file => file =>
!file.renderIt && !file.renderIt &&
(file.viewer && (!file.viewer.collapsed || file.viewer.name !== diffViewerModes.text)), (file.viewer &&
(!file.viewer.automaticallyCollapsed || file.viewer.name !== diffViewerModes.text)),
); );
if (nextFile) { if (nextFile) {
...@@ -631,7 +632,7 @@ export function switchToFullDiffFromRenamedFile({ commit, dispatch, state }, { d ...@@ -631,7 +632,7 @@ export function switchToFullDiffFromRenamedFile({ commit, dispatch, state }, { d
filePath: diffFile.file_path, filePath: diffFile.file_path,
viewer: { viewer: {
...diffFile.alternate_viewer, ...diffFile.alternate_viewer,
collapsed: false, automaticallyCollapsed: false,
}, },
}); });
commit(types.SET_CURRENT_VIEW_DIFF_FILE_LINES, { filePath: diffFile.file_path, lines }); commit(types.SET_CURRENT_VIEW_DIFF_FILE_LINES, { filePath: diffFile.file_path, lines });
......
...@@ -9,7 +9,7 @@ export const isParallelView = state => state.diffViewType === PARALLEL_DIFF_VIEW ...@@ -9,7 +9,7 @@ export const isParallelView = state => state.diffViewType === PARALLEL_DIFF_VIEW
export const isInlineView = state => state.diffViewType === INLINE_DIFF_VIEW_TYPE; export const isInlineView = state => state.diffViewType === INLINE_DIFF_VIEW_TYPE;
export const hasCollapsedFile = state => export const hasCollapsedFile = state =>
state.diffFiles.some(file => file.viewer && file.viewer.collapsed); state.diffFiles.some(file => file.viewer && file.viewer.automaticallyCollapsed);
export const commitId = state => (state.commit && state.commit.id ? state.commit.id : null); export const commitId = state => (state.commit && state.commit.id ? state.commit.id : null);
......
...@@ -172,7 +172,7 @@ export default { ...@@ -172,7 +172,7 @@ export default {
state.diffFiles.forEach(file => { state.diffFiles.forEach(file => {
Object.assign(file, { Object.assign(file, {
viewer: Object.assign(file.viewer, { viewer: Object.assign(file.viewer, {
collapsed: false, automaticallyCollapsed: false,
}), }),
}); });
}); });
...@@ -355,7 +355,7 @@ export default { ...@@ -355,7 +355,7 @@ export default {
const file = state.diffFiles.find(f => f.file_path === filePath); const file = state.diffFiles.find(f => f.file_path === filePath);
if (file && file.viewer) { if (file && file.viewer) {
file.viewer.collapsed = collapsed; file.viewer.automaticallyCollapsed = collapsed;
} }
}, },
[types.SET_HIDDEN_VIEW_DIFF_FILE_LINES](state, { filePath, lines }) { [types.SET_HIDDEN_VIEW_DIFF_FILE_LINES](state, { filePath, lines }) {
......
...@@ -76,7 +76,7 @@ export default { ...@@ -76,7 +76,7 @@ export default {
:discussion-path="discussion.discussion_path" :discussion-path="discussion.discussion_path"
:diff-file="discussion.diff_file" :diff-file="discussion.diff_file"
:can-current-user-fork="false" :can-current-user-fork="false"
:expanded="!discussion.diff_file.viewer.collapsed" :expanded="!discussion.diff_file.viewer.automaticallyCollapsed"
/> />
<div v-if="isTextFile" class="diff-content"> <div v-if="isTextFile" class="diff-content">
<table class="code js-syntax-highlight" :class="$options.userColorSchemeClass"> <table class="code js-syntax-highlight" :class="$options.userColorSchemeClass">
......
...@@ -699,7 +699,7 @@ describe('diffs/components/app', () => { ...@@ -699,7 +699,7 @@ describe('diffs/components/app', () => {
describe('collapsed files', () => { 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 collapsed files', () => {
createComponent({}, ({ state }) => { createComponent({}, ({ state }) => {
state.diffs.diffFiles = [{ viewer: { collapsed: true } }]; state.diffs.diffFiles = [{ viewer: { automaticallyCollapsed: true } }];
}); });
expect(getCollapsedFilesWarning(wrapper).exists()).toBe(true); expect(getCollapsedFilesWarning(wrapper).exists()).toBe(true);
...@@ -707,7 +707,7 @@ describe('diffs/components/app', () => { ...@@ -707,7 +707,7 @@ describe('diffs/components/app', () => {
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 the user has dismissed the alert already', async () => {
createComponent({}, ({ state }) => { createComponent({}, ({ state }) => {
state.diffs.diffFiles = [{ viewer: { collapsed: true } }]; state.diffs.diffFiles = [{ viewer: { automaticallyCollapsed: true } }];
}); });
expect(getCollapsedFilesWarning(wrapper).exists()).toBe(true); expect(getCollapsedFilesWarning(wrapper).exists()).toBe(true);
......
...@@ -181,7 +181,7 @@ describe('DiffFile', () => { ...@@ -181,7 +181,7 @@ describe('DiffFile', () => {
}); });
it('updates local state when changing file state', done => { it('updates local state when changing file state', done => {
vm.file.viewer.collapsed = true; vm.file.viewer.automaticallyCollapsed = true;
vm.$nextTick(() => { vm.$nextTick(() => {
expect(vm.isCollapsed).toBe(true); expect(vm.isCollapsed).toBe(true);
......
...@@ -27,7 +27,7 @@ export default { ...@@ -27,7 +27,7 @@ export default {
viewer: { viewer: {
name: 'text', name: 'text',
error: null, error: null,
collapsed: false, automaticallyCollapsed: false,
}, },
added_lines: 2, added_lines: 2,
removed_lines: 0, removed_lines: 0,
......
...@@ -26,7 +26,7 @@ export default { ...@@ -26,7 +26,7 @@ export default {
viewer: { viewer: {
name: 'text', name: 'text',
error: null, error: null,
collapsed: false, automaticallyCollapsed: false,
}, },
added_lines: 0, added_lines: 0,
removed_lines: 0, removed_lines: 0,
......
...@@ -483,14 +483,14 @@ describe('DiffsStoreActions', () => { ...@@ -483,14 +483,14 @@ describe('DiffsStoreActions', () => {
id: 1, id: 1,
renderIt: false, renderIt: false,
viewer: { viewer: {
collapsed: false, automaticallyCollapsed: false,
}, },
}, },
{ {
id: 2, id: 2,
renderIt: false, renderIt: false,
viewer: { viewer: {
collapsed: false, automaticallyCollapsed: false,
}, },
}, },
], ],
...@@ -967,7 +967,7 @@ describe('DiffsStoreActions', () => { ...@@ -967,7 +967,7 @@ describe('DiffsStoreActions', () => {
{ {
file_hash: 'HASH', file_hash: 'HASH',
viewer: { viewer: {
collapsed, automaticallyCollapsed: collapsed,
}, },
renderIt, renderIt,
}, },
...@@ -1167,7 +1167,7 @@ describe('DiffsStoreActions', () => { ...@@ -1167,7 +1167,7 @@ describe('DiffsStoreActions', () => {
file_hash: 'testhash', file_hash: 'testhash',
alternate_viewer: { name: updatedViewerName }, alternate_viewer: { name: updatedViewerName },
}; };
const updatedViewer = { name: updatedViewerName, collapsed: false }; const updatedViewer = { name: updatedViewerName, automaticallyCollapsed: false };
const testData = [{ rich_text: 'test' }, { rich_text: 'file2' }]; const testData = [{ rich_text: 'test' }, { rich_text: 'file2' }];
let renamedFile; let renamedFile;
let mock; let mock;
......
...@@ -51,13 +51,19 @@ describe('Diffs Module Getters', () => { ...@@ -51,13 +51,19 @@ describe('Diffs Module Getters', () => {
describe('hasCollapsedFile', () => { describe('hasCollapsedFile', () => {
it('returns true when all files are collapsed', () => { it('returns true when all files are collapsed', () => {
localState.diffFiles = [{ viewer: { collapsed: true } }, { viewer: { collapsed: true } }]; localState.diffFiles = [
{ viewer: { automaticallyCollapsed: true } },
{ viewer: { automaticallyCollapsed: true } },
];
expect(getters.hasCollapsedFile(localState)).toEqual(true); expect(getters.hasCollapsedFile(localState)).toEqual(true);
}); });
it('returns true when at least one file is collapsed', () => { it('returns true when at least one file is collapsed', () => {
localState.diffFiles = [{ viewer: { collapsed: false } }, { viewer: { collapsed: true } }]; localState.diffFiles = [
{ viewer: { automaticallyCollapsed: false } },
{ viewer: { automaticallyCollapsed: true } },
];
expect(getters.hasCollapsedFile(localState)).toEqual(true); expect(getters.hasCollapsedFile(localState)).toEqual(true);
}); });
......
...@@ -130,14 +130,14 @@ describe('DiffsStoreMutations', () => { ...@@ -130,14 +130,14 @@ describe('DiffsStoreMutations', () => {
it('should change the collapsed prop from diffFiles', () => { it('should change the collapsed prop from diffFiles', () => {
const diffFile = { const diffFile = {
viewer: { viewer: {
collapsed: true, automaticallyCollapsed: true,
}, },
}; };
const state = { expandAllFiles: true, diffFiles: [diffFile] }; const state = { expandAllFiles: true, diffFiles: [diffFile] };
mutations[types.EXPAND_ALL_FILES](state); mutations[types.EXPAND_ALL_FILES](state);
expect(state.diffFiles[0].viewer.collapsed).toEqual(false); expect(state.diffFiles[0].viewer.automaticallyCollapsed).toEqual(false);
}); });
}); });
...@@ -933,12 +933,12 @@ describe('DiffsStoreMutations', () => { ...@@ -933,12 +933,12 @@ describe('DiffsStoreMutations', () => {
describe('SET_FILE_COLLAPSED', () => { describe('SET_FILE_COLLAPSED', () => {
it('sets collapsed', () => { it('sets collapsed', () => {
const state = { const state = {
diffFiles: [{ file_path: 'test', viewer: { collapsed: false } }], diffFiles: [{ file_path: 'test', viewer: { automaticallyCollapsed: false } }],
}; };
mutations[types.SET_FILE_COLLAPSED](state, { filePath: 'test', collapsed: true }); mutations[types.SET_FILE_COLLAPSED](state, { filePath: 'test', collapsed: true });
expect(state.diffFiles[0].viewer.collapsed).toBe(true); expect(state.diffFiles[0].viewer.automaticallyCollapsed).toBe(true);
}); });
}); });
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment