Commit 441ea69a authored by Phil Hughes's avatar Phil Hughes

Merge branch 'feature/collapsed-diff-files/user-collapsed-flag' into 'master'

Add `manuallyCollapsed` flag to Diff Files

See merge request gitlab-org/gitlab!43911
parents 41543e44 d6c42a8b
...@@ -124,7 +124,6 @@ export default { ...@@ -124,7 +124,6 @@ export default {
return { return {
treeWidth, treeWidth,
diffFilesLength: 0, diffFilesLength: 0,
collapsedWarningDismissed: false,
}; };
}, },
computed: { computed: {
...@@ -153,7 +152,7 @@ export default { ...@@ -153,7 +152,7 @@ export default {
'canMerge', 'canMerge',
'hasConflicts', 'hasConflicts',
]), ]),
...mapGetters('diffs', ['hasCollapsedFile', 'isParallelView', 'currentDiffIndex']), ...mapGetters('diffs', ['whichCollapsedTypes', 'isParallelView', 'currentDiffIndex']),
...mapGetters(['isNotesFetched', 'getNoteableData']), ...mapGetters(['isNotesFetched', 'getNoteableData']),
diffs() { diffs() {
if (!this.viewDiffsFileByFile) { if (!this.viewDiffsFileByFile) {
...@@ -206,11 +205,7 @@ export default { ...@@ -206,11 +205,7 @@ export default {
visible = this.$options.alerts.ALERT_OVERFLOW_HIDDEN; visible = this.$options.alerts.ALERT_OVERFLOW_HIDDEN;
} else if (this.isDiffHead && this.hasConflicts) { } else if (this.isDiffHead && this.hasConflicts) {
visible = this.$options.alerts.ALERT_MERGE_CONFLICT; visible = this.$options.alerts.ALERT_MERGE_CONFLICT;
} else if ( } else if (this.whichCollapsedTypes.automatic && !this.viewDiffsFileByFile) {
this.hasCollapsedFile &&
!this.collapsedWarningDismissed &&
!this.viewDiffsFileByFile
) {
visible = this.$options.alerts.ALERT_COLLAPSED_FILES; visible = this.$options.alerts.ALERT_COLLAPSED_FILES;
} }
...@@ -429,9 +424,6 @@ export default { ...@@ -429,9 +424,6 @@ export default {
this.toggleShowTreeList(false); this.toggleShowTreeList(false);
} }
}, },
dismissCollapsedWarning() {
this.collapsedWarningDismissed = true;
},
}, },
minTreeWidth: MIN_TREE_WIDTH, minTreeWidth: MIN_TREE_WIDTH,
maxTreeWidth: MAX_TREE_WIDTH, maxTreeWidth: MAX_TREE_WIDTH,
...@@ -464,7 +456,6 @@ export default { ...@@ -464,7 +456,6 @@ export default {
<collapsed-files-warning <collapsed-files-warning
v-if="visibleWarning == $options.alerts.ALERT_COLLAPSED_FILES" v-if="visibleWarning == $options.alerts.ALERT_COLLAPSED_FILES"
:limited="isLimitedContainer" :limited="isLimitedContainer"
@dismiss="dismissCollapsedWarning"
/> />
<div <div
......
...@@ -38,7 +38,7 @@ export default { ...@@ -38,7 +38,7 @@ export default {
}, },
computed: { computed: {
...mapGetters('diffs', [ ...mapGetters('diffs', [
'hasCollapsedFile', 'whichCollapsedTypes',
'diffCompareDropdownTargetVersions', 'diffCompareDropdownTargetVersions',
'diffCompareDropdownSourceVersions', 'diffCompareDropdownSourceVersions',
]), ]),
...@@ -129,7 +129,7 @@ export default { ...@@ -129,7 +129,7 @@ export default {
{{ __('Show latest version') }} {{ __('Show latest version') }}
</gl-button> </gl-button>
<gl-button <gl-button
v-show="hasCollapsedFile" v-show="whichCollapsedTypes.any"
variant="default" variant="default"
class="gl-mr-3" class="gl-mr-3"
@click="expandAllFiles" @click="expandAllFiles"
......
...@@ -10,6 +10,8 @@ import eventHub from '../../notes/event_hub'; ...@@ -10,6 +10,8 @@ import eventHub from '../../notes/event_hub';
import DiffFileHeader from './diff_file_header.vue'; import DiffFileHeader from './diff_file_header.vue';
import DiffContent from './diff_content.vue'; import DiffContent from './diff_content.vue';
import { diffViewerErrors } from '~/ide/constants'; import { diffViewerErrors } from '~/ide/constants';
import { collapsedType, isCollapsed } from '../diff_file';
import { DIFF_FILE_AUTOMATIC_COLLAPSE, DIFF_FILE_MANUAL_COLLAPSE } from '../constants';
export default { export default {
components: { components: {
...@@ -44,7 +46,7 @@ export default { ...@@ -44,7 +46,7 @@ export default {
return { return {
isLoadingCollapsedDiff: false, isLoadingCollapsedDiff: false,
forkMessageVisible: false, forkMessageVisible: false,
isCollapsed: this.file.viewer.automaticallyCollapsed || false, isCollapsed: isCollapsed(this.file),
}; };
}, },
computed: { computed: {
...@@ -71,7 +73,7 @@ export default { ...@@ -71,7 +73,7 @@ export default {
return this.file.viewer.error === diffViewerErrors.too_large; return this.file.viewer.error === diffViewerErrors.too_large;
}, },
errorMessage() { errorMessage() {
return this.file.viewer.error_message; return !this.manuallyCollapsed ? this.file.viewer.error_message : '';
}, },
forkMessage() { forkMessage() {
return sprintf( return sprintf(
...@@ -85,57 +87,94 @@ export default { ...@@ -85,57 +87,94 @@ export default {
false, false,
); );
}, },
}, hasBodyClasses() {
watch: { const domParts = {
isCollapsed: function fileCollapsedWatch(newVal, oldVal) { header: 'gl-rounded-base!',
if (!newVal && oldVal && !this.hasDiff) { contentByHash: '',
this.handleLoadCollapsedDiff(); content: '',
};
if (this.showBody) {
domParts.header = 'gl-rounded-bottom-left-none gl-rounded-bottom-right-none';
domParts.contentByHash =
'gl-rounded-none gl-rounded-bottom-left-base gl-rounded-bottom-right-base gl-border-1 gl-border-t-0! gl-border-solid gl-border-gray-100';
domParts.content = 'gl-rounded-bottom-left-base gl-rounded-bottom-right-base';
} }
this.setFileCollapsed({ filePath: this.file.file_path, collapsed: newVal }); return domParts;
},
automaticallyCollapsed() {
return collapsedType(this.file) === DIFF_FILE_AUTOMATIC_COLLAPSE;
},
manuallyCollapsed() {
return collapsedType(this.file) === DIFF_FILE_MANUAL_COLLAPSE;
},
showBody() {
return !this.isCollapsed || this.automaticallyCollapsed;
}, },
showWarning() {
return this.isCollapsed && (this.automaticallyCollapsed && !this.viewDiffsFileByFile);
},
showContent() {
return !this.isCollapsed && !this.isFileTooLarge;
},
},
watch: {
'file.file_hash': { 'file.file_hash': {
handler: function watchFileHash() { handler: function hashChangeWatch(newHash, oldHash) {
if (this.viewDiffsFileByFile && this.file.viewer.automaticallyCollapsed) { this.isCollapsed = isCollapsed(this.file);
this.isCollapsed = false;
this.handleLoadCollapsedDiff(); if (newHash && oldHash && !this.hasDiff) {
} else { this.requestDiff();
this.isCollapsed = this.file.viewer.automaticallyCollapsed || false;
} }
}, },
immediate: true, immediate: true,
}, },
'file.viewer.automaticallyCollapsed': function setIsCollapsed(newVal) { 'file.viewer.automaticallyCollapsed': {
if (!this.viewDiffsFileByFile) { handler: function autoChangeWatch(automaticValue) {
this.isCollapsed = newVal; if (collapsedType(this.file) !== DIFF_FILE_MANUAL_COLLAPSE) {
} this.isCollapsed = this.viewDiffsFileByFile ? false : automaticValue;
}
},
immediate: true,
},
'file.viewer.manuallyCollapsed': {
handler: function manualChangeWatch(manualValue) {
if (manualValue !== null) {
this.isCollapsed = manualValue;
}
},
immediate: true,
}, },
}, },
created() { created() {
eventHub.$on(`loadCollapsedDiff/${this.file.file_hash}`, this.handleLoadCollapsedDiff); eventHub.$on(`loadCollapsedDiff/${this.file.file_hash}`, this.requestDiff);
}, },
methods: { methods: {
...mapActions('diffs', [ ...mapActions('diffs', [
'loadCollapsedDiff', 'loadCollapsedDiff',
'assignDiscussionsToDiff', 'assignDiscussionsToDiff',
'setRenderIt', 'setRenderIt',
'setFileCollapsed', 'setFileCollapsedByUser',
]), ]),
handleToggle() { handleToggle() {
if (!this.hasDiff) { const currentCollapsedFlag = this.isCollapsed;
this.handleLoadCollapsedDiff();
} else { this.setFileCollapsedByUser({
this.isCollapsed = !this.isCollapsed; filePath: this.file.file_path,
this.setRenderIt(this.file); collapsed: !currentCollapsedFlag,
});
if (!this.hasDiff && currentCollapsedFlag) {
this.requestDiff();
} }
}, },
handleLoadCollapsedDiff() { requestDiff() {
this.isLoadingCollapsedDiff = true; this.isLoadingCollapsedDiff = true;
this.loadCollapsedDiff(this.file) this.loadCollapsedDiff(this.file)
.then(() => { .then(() => {
this.isLoadingCollapsedDiff = false; this.isLoadingCollapsedDiff = false;
this.isCollapsed = false;
this.setRenderIt(this.file); this.setRenderIt(this.file);
}) })
.then(() => { .then(() => {
...@@ -167,9 +206,10 @@ export default { ...@@ -167,9 +206,10 @@ export default {
:class="{ :class="{
'is-active': currentDiffFileId === file.file_hash, 'is-active': currentDiffFileId === file.file_hash,
'comments-disabled': Boolean(file.brokenSymlink), 'comments-disabled': Boolean(file.brokenSymlink),
'has-body': showBody,
}" }"
:data-path="file.new_path" :data-path="file.new_path"
class="diff-file file-holder" class="diff-file file-holder gl-border-none"
> >
<diff-file-header <diff-file-header
:can-current-user-fork="canCurrentUserFork" :can-current-user-fork="canCurrentUserFork"
...@@ -178,7 +218,8 @@ export default { ...@@ -178,7 +218,8 @@ export default {
:expanded="!isCollapsed" :expanded="!isCollapsed"
:add-merge-request-buttons="true" :add-merge-request-buttons="true"
:view-diffs-file-by-file="viewDiffsFileByFile" :view-diffs-file-by-file="viewDiffsFileByFile"
class="js-file-title file-title" class="js-file-title file-title gl-border-1 gl-border-solid gl-border-gray-100"
:class="hasBodyClasses.header"
@toggleFile="handleToggle" @toggleFile="handleToggle"
@showForkMessage="showForkMessage" @showForkMessage="showForkMessage"
/> />
...@@ -198,21 +239,35 @@ export default { ...@@ -198,21 +239,35 @@ export default {
{{ __('Cancel') }} {{ __('Cancel') }}
</button> </button>
</div> </div>
<gl-loading-icon v-if="showLoadingIcon" class="diff-content loading" />
<template v-else> <template v-else>
<div :id="`diff-content-${file.file_hash}`"> <div
<div v-if="errorMessage" class="diff-viewer"> :id="`diff-content-${file.file_hash}`"
:class="hasBodyClasses.contentByHash"
data-testid="content-area"
>
<gl-loading-icon
v-if="showLoadingIcon"
class="diff-content loading gl-my-0 gl-pt-3"
data-testid="loader-icon"
/>
<div v-else-if="errorMessage" class="diff-viewer">
<div v-safe-html="errorMessage" class="nothing-here-block"></div> <div v-safe-html="errorMessage" class="nothing-here-block"></div>
</div> </div>
<template v-else> <template v-else>
<div v-show="isCollapsed" class="nothing-here-block diff-collapsed"> <div v-show="showWarning" class="nothing-here-block diff-collapsed">
{{ __('This diff is collapsed.') }} {{ __('This diff is collapsed.') }}
<a class="click-to-expand js-click-to-expand" href="#" @click.prevent="handleToggle">{{ <a
__('Click to expand it.') class="click-to-expand"
}}</a> data-testid="toggle-link"
href="#"
@click.prevent="handleToggle"
>
{{ __('Click to expand it.') }}
</a>
</div> </div>
<diff-content <diff-content
v-show="!isCollapsed && !isFileTooLarge" v-show="showContent"
:class="hasBodyClasses.content"
:diff-file="file" :diff-file="file"
:help-page-path="helpPagePath" :help-page-path="helpPagePath"
/> />
......
...@@ -18,6 +18,7 @@ import { __, s__, sprintf } from '~/locale'; ...@@ -18,6 +18,7 @@ import { __, s__, sprintf } from '~/locale';
import { diffViewerModes } from '~/ide/constants'; import { diffViewerModes } from '~/ide/constants';
import DiffStats from './diff_stats.vue'; import DiffStats from './diff_stats.vue';
import { scrollToElement } from '~/lib/utils/common_utils'; import { scrollToElement } from '~/lib/utils/common_utils';
import { isCollapsed } from '../diff_file';
import { DIFF_FILE_HEADER } from '../i18n'; import { DIFF_FILE_HEADER } from '../i18n';
export default { export default {
...@@ -125,6 +126,9 @@ export default { ...@@ -125,6 +126,9 @@ export default {
isUsingLfs() { isUsingLfs() {
return this.diffFile.stored_externally && this.diffFile.external_storage === 'lfs'; return this.diffFile.stored_externally && this.diffFile.external_storage === 'lfs';
}, },
isCollapsed() {
return isCollapsed(this.diffFile, { fileByFile: this.viewDiffsFileByFile });
},
collapseIcon() { collapseIcon() {
return this.expanded ? 'chevron-down' : 'chevron-right'; return this.expanded ? 'chevron-down' : 'chevron-right';
}, },
...@@ -334,7 +338,7 @@ export default { ...@@ -334,7 +338,7 @@ export default {
</gl-dropdown-item> </gl-dropdown-item>
</template> </template>
<template v-if="!diffFile.viewer.automaticallyCollapsed"> <template v-if="!isCollapsed">
<gl-dropdown-divider <gl-dropdown-divider
v-if="!diffFile.is_fully_expanded || diffHasDiscussions(diffFile)" v-if="!diffFile.is_fully_expanded || diffHasDiscussions(diffFile)"
/> />
......
...@@ -73,6 +73,10 @@ export const ALERT_OVERFLOW_HIDDEN = 'overflow'; ...@@ -73,6 +73,10 @@ export const ALERT_OVERFLOW_HIDDEN = 'overflow';
export const ALERT_MERGE_CONFLICT = 'merge-conflict'; export const ALERT_MERGE_CONFLICT = 'merge-conflict';
export const ALERT_COLLAPSED_FILES = 'collapsed'; 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 // State machine states
export const STATE_IDLING = 'idle'; export const STATE_IDLING = 'idle';
export const STATE_LOADING = 'loading'; export const STATE_LOADING = 'loading';
......
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) { function fileSymlinkInformation(file, fileList) {
const duplicates = fileList.filter(iteratedFile => iteratedFile.file_hash === file.file_hash); const duplicates = fileList.filter(iteratedFile => iteratedFile.file_hash === file.file_hash);
...@@ -23,6 +28,7 @@ function collapsed(file) { ...@@ -23,6 +28,7 @@ function collapsed(file) {
return { return {
automaticallyCollapsed: viewer.automaticallyCollapsed || viewer.collapsed || false, automaticallyCollapsed: viewer.automaticallyCollapsed || viewer.collapsed || false,
manuallyCollapsed: null,
}; };
} }
...@@ -37,3 +43,19 @@ export function prepareRawDiffFile({ file, allFiles }) { ...@@ -37,3 +43,19 @@ export function prepareRawDiffFile({ file, allFiles }) {
return file; 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];
}
...@@ -40,8 +40,11 @@ import { ...@@ -40,8 +40,11 @@ import {
DIFF_WHITESPACE_COOKIE_NAME, DIFF_WHITESPACE_COOKIE_NAME,
SHOW_WHITESPACE, SHOW_WHITESPACE,
NO_SHOW_WHITESPACE, NO_SHOW_WHITESPACE,
DIFF_FILE_MANUAL_COLLAPSE,
DIFF_FILE_AUTOMATIC_COLLAPSE,
} from '../constants'; } from '../constants';
import { diffViewerModes } from '~/ide/constants'; import { diffViewerModes } from '~/ide/constants';
import { isCollapsed } from '../diff_file';
export const setBaseConfig = ({ commit }, options) => { export const setBaseConfig = ({ commit }, options) => {
const { const {
...@@ -239,6 +242,13 @@ export const renderFileForDiscussionId = ({ commit, rootState, state }, discussi ...@@ -239,6 +242,13 @@ export const renderFileForDiscussionId = ({ commit, rootState, state }, discussi
if (file.viewer.automaticallyCollapsed) { 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 if (file.viewer.manuallyCollapsed) {
commit(types.SET_FILE_COLLAPSED, {
filePath: file.file_path,
collapsed: false,
trigger: DIFF_FILE_AUTOMATIC_COLLAPSE,
});
eventHub.$emit('scrollToDiscussion');
} else { } else {
eventHub.$emit('scrollToDiscussion'); eventHub.$emit('scrollToDiscussion');
} }
...@@ -252,8 +262,7 @@ export const startRenderDiffsQueue = ({ state, commit }) => { ...@@ -252,8 +262,7 @@ 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 && (!isCollapsed(file) || file.viewer.name !== diffViewerModes.text)),
(!file.viewer.automaticallyCollapsed || file.viewer.name !== diffViewerModes.text)),
); );
if (nextFile) { if (nextFile) {
...@@ -641,8 +650,9 @@ export function switchToFullDiffFromRenamedFile({ commit, dispatch, state }, { d ...@@ -641,8 +650,9 @@ export function switchToFullDiffFromRenamedFile({ commit, dispatch, state }, { d
}); });
} }
export const setFileCollapsed = ({ commit }, { filePath, collapsed }) => export const setFileCollapsedByUser = ({ commit }, { filePath, collapsed }) => {
commit(types.SET_FILE_COLLAPSED, { filePath, collapsed }); commit(types.SET_FILE_COLLAPSED, { filePath, collapsed, trigger: DIFF_FILE_MANUAL_COLLAPSE });
};
export const setSuggestPopoverDismissed = ({ commit, state }) => export const setSuggestPopoverDismissed = ({ commit, state }) =>
axios axios
......
...@@ -8,8 +8,16 @@ export const isParallelView = state => state.diffViewType === PARALLEL_DIFF_VIEW ...@@ -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 isInlineView = state => state.diffViewType === INLINE_DIFF_VIEW_TYPE;
export const hasCollapsedFile = state => export const whichCollapsedTypes = state => {
state.diffFiles.some(file => file.viewer && file.viewer.automaticallyCollapsed); 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); export const commitId = state => (state.commit && state.commit.id ? state.commit.id : null);
......
import Vue from 'vue'; import Vue from 'vue';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; 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 { import {
findDiffFile, findDiffFile,
addLineReferences, addLineReferences,
...@@ -16,6 +20,12 @@ function updateDiffFilesInState(state, files) { ...@@ -16,6 +20,12 @@ function updateDiffFilesInState(state, files) {
return Object.assign(state, { diffFiles: files }); return Object.assign(state, { diffFiles: files });
} }
function renderFile(file) {
Object.assign(file, {
renderIt: true,
});
}
export default { export default {
[types.SET_BASE_CONFIG](state, options) { [types.SET_BASE_CONFIG](state, options) {
const { const {
...@@ -81,9 +91,7 @@ export default { ...@@ -81,9 +91,7 @@ export default {
}, },
[types.RENDER_FILE](state, file) { [types.RENDER_FILE](state, file) {
Object.assign(file, { renderFile(file);
renderIt: true,
});
}, },
[types.SET_MERGE_REQUEST_DIFFS](state, mergeRequestDiffs) { [types.SET_MERGE_REQUEST_DIFFS](state, mergeRequestDiffs) {
...@@ -173,6 +181,7 @@ export default { ...@@ -173,6 +181,7 @@ export default {
Object.assign(file, { Object.assign(file, {
viewer: Object.assign(file.viewer, { viewer: Object.assign(file.viewer, {
automaticallyCollapsed: false, automaticallyCollapsed: false,
manuallyCollapsed: false,
}), }),
}); });
}); });
...@@ -351,11 +360,24 @@ export default { ...@@ -351,11 +360,24 @@ export default {
file.isShowingFullFile = true; file.isShowingFullFile = true;
file.isLoadingFullFile = false; 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); const file = state.diffFiles.find(f => f.file_path === filePath);
if (file && file.viewer) { 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 }) { [types.SET_HIDDEN_VIEW_DIFF_FILE_LINES](state, { filePath, lines }) {
......
...@@ -7,6 +7,7 @@ import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.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 ImageDiffOverlay from '~/diffs/components/image_diff_overlay.vue';
import { getDiffMode } from '~/diffs/store/utils'; import { getDiffMode } from '~/diffs/store/utils';
import { diffViewerModes } from '~/ide/constants'; import { diffViewerModes } from '~/ide/constants';
import { isCollapsed } from '../../diffs/diff_file';
const FIRST_CHAR_REGEX = /^(\+|-| )/; const FIRST_CHAR_REGEX = /^(\+|-| )/;
...@@ -46,6 +47,9 @@ export default { ...@@ -46,6 +47,9 @@ export default {
this.discussion.truncated_diff_lines && this.discussion.truncated_diff_lines.length !== 0 this.discussion.truncated_diff_lines && this.discussion.truncated_diff_lines.length !== 0
); );
}, },
isCollapsed() {
return isCollapsed(this.discussion.diff_file);
},
}, },
mounted() { mounted() {
if (this.isTextFile && !this.hasTruncatedDiffLines) { if (this.isTextFile && !this.hasTruncatedDiffLines) {
...@@ -76,7 +80,7 @@ export default { ...@@ -76,7 +80,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.automaticallyCollapsed" :expanded="!isCollapsed"
/> />
<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">
......
...@@ -6,11 +6,18 @@ ...@@ -6,11 +6,18 @@
border-top: 1px solid $border-color; 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,
.file-title-flex-parent { .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; cursor: pointer;
.dropdown-menu { .dropdown-menu {
...@@ -113,7 +120,6 @@ ...@@ -113,7 +120,6 @@
.diff-content { .diff-content {
background: $white; background: $white;
color: $gl-text-color; color: $gl-text-color;
border-radius: 0 0 3px 3px;
.unfold { .unfold {
cursor: pointer; cursor: pointer;
...@@ -457,8 +463,11 @@ table.code { ...@@ -457,8 +463,11 @@ table.code {
border-top: 0; border-top: 0;
} }
tr:nth-last-of-type(2).line_expansion > td { tr:nth-last-of-type(2).line_expansion,
border-bottom: 0; tr:last-of-type.line_expansion {
> td {
border-bottom: 0;
}
} }
tr.line_holder td { tr.line_holder td {
......
...@@ -11,9 +11,19 @@ ...@@ -11,9 +11,19 @@
} }
.diff-tree-list { .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: -webkit-sticky;
position: sticky; position: sticky;
$top-pos: $header-height + $mr-tabs-height + $mr-version-controls-height + 15px;
top: $top-pos; top: $top-pos;
max-height: calc(100vh - #{$top-pos}); max-height: calc(100vh - #{$top-pos});
z-index: 202; z-index: 202;
......
---
title: Manually collapsed diff files are now significantly shorter and less visually
intrusive
merge_request: 43911
author:
type: changed
...@@ -697,7 +697,7 @@ describe('diffs/components/app', () => { ...@@ -697,7 +697,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 automatically collapsed files', () => {
createComponent({}, ({ state }) => { createComponent({}, ({ state }) => {
state.diffs.diffFiles = [{ viewer: { automaticallyCollapsed: true } }]; state.diffs.diffFiles = [{ viewer: { automaticallyCollapsed: true } }];
}); });
...@@ -705,16 +705,14 @@ describe('diffs/components/app', () => { ...@@ -705,16 +705,14 @@ describe('diffs/components/app', () => {
expect(getCollapsedFilesWarning(wrapper).exists()).toBe(true); 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 }) => { 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); expect(getCollapsedFilesWarning(wrapper).exists()).toBe(false);
}); });
}); });
......
import { shallowMount, createLocalVue } from '@vue/test-utils'; import { shallowMount, createLocalVue } from '@vue/test-utils';
import Vuex from 'vuex'; import Vuex from 'vuex';
import { cloneDeep } from 'lodash'; import { cloneDeep } from 'lodash';
import { mockTracking, triggerEvent } from 'helpers/tracking_helper';
import DiffFileHeader from '~/diffs/components/diff_file_header.vue'; import DiffFileHeader from '~/diffs/components/diff_file_header.vue';
import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
import diffDiscussionsMockData from '../mock_data/diff_discussions'; import diffDiscussionsMockData from '../mock_data/diff_discussions';
...@@ -136,9 +139,25 @@ describe('DiffFileHeader component', () => { ...@@ -136,9 +139,25 @@ describe('DiffFileHeader component', () => {
}); });
}); });
it('displays a copy to clipboard button', () => { describe('copy to clipboard', () => {
createComponent(); beforeEach(() => {
expect(wrapper.find(ClipboardButton).exists()).toBe(true); 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', () => { describe('for submodule', () => {
......
...@@ -27,6 +27,7 @@ export default { ...@@ -27,6 +27,7 @@ export default {
name: 'text', name: 'text',
error: null, error: null,
automaticallyCollapsed: false, automaticallyCollapsed: false,
manuallyCollapsed: null,
}, },
added_lines: 2, added_lines: 2,
removed_lines: 0, removed_lines: 0,
......
...@@ -26,6 +26,7 @@ export default { ...@@ -26,6 +26,7 @@ export default {
name: 'text', name: 'text',
error: null, error: null,
automaticallyCollapsed: false, automaticallyCollapsed: false,
manuallyCollapsed: null,
}, },
added_lines: 0, added_lines: 0,
removed_lines: 0, removed_lines: 0,
......
...@@ -42,7 +42,7 @@ import { ...@@ -42,7 +42,7 @@ import {
fetchFullDiff, fetchFullDiff,
toggleFullDiff, toggleFullDiff,
switchToFullDiffFromRenamedFile, switchToFullDiffFromRenamedFile,
setFileCollapsed, setFileCollapsedByUser,
setExpandedDiffLines, setExpandedDiffLines,
setSuggestPopoverDismissed, setSuggestPopoverDismissed,
changeCurrentCommit, changeCurrentCommit,
...@@ -1216,13 +1216,18 @@ describe('DiffsStoreActions', () => { ...@@ -1216,13 +1216,18 @@ describe('DiffsStoreActions', () => {
}); });
}); });
describe('setFileCollapsed', () => { describe('setFileUserCollapsed', () => {
it('commits SET_FILE_COLLAPSED', done => { it('commits SET_FILE_COLLAPSED', done => {
testAction( testAction(
setFileCollapsed, setFileCollapsedByUser,
{ filePath: 'test', collapsed: true }, { filePath: 'test', collapsed: true },
null, null,
[{ type: types.SET_FILE_COLLAPSED, payload: { filePath: 'test', collapsed: true } }], [
{
type: types.SET_FILE_COLLAPSED,
payload: { filePath: 'test', collapsed: true, trigger: 'manual' },
},
],
[], [],
done, done,
); );
......
...@@ -49,23 +49,53 @@ describe('Diffs Module Getters', () => { ...@@ -49,23 +49,53 @@ describe('Diffs Module Getters', () => {
}); });
}); });
describe('hasCollapsedFile', () => { describe('whichCollapsedTypes', () => {
it('returns true when all files are collapsed', () => { const autoCollapsedFile = { viewer: { automaticallyCollapsed: true, manuallyCollapsed: null } };
localState.diffFiles = [ const manuallyCollapsedFile = {
{ viewer: { automaticallyCollapsed: true } }, viewer: { automaticallyCollapsed: false, manuallyCollapsed: true },
{ viewer: { automaticallyCollapsed: 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', () => { it.each`
localState.diffFiles = [ description | value | files
{ viewer: { automaticallyCollapsed: false } }, ${'all files are automatically collapsed'} | ${false} | ${[{ ...autoCollapsedFile }, { ...autoCollapsedFile }]}
{ viewer: { automaticallyCollapsed: true } }, ${'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);
}); });
}); });
......
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