Commit 60c9e4e9 authored by Phil Hughes's avatar Phil Hughes

Fixed diff file states getting incorrectly set

This happens because the virtual scroller re-uses components
so using the data property won't work.
parent f9782c3d
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
import { GlButton, GlLoadingIcon, GlSafeHtmlDirective as SafeHtml, GlSprintf } from '@gitlab/ui'; import { GlButton, GlLoadingIcon, GlSafeHtmlDirective as SafeHtml, GlSprintf } from '@gitlab/ui';
import { escape } from 'lodash'; import { escape } from 'lodash';
import { mapActions, mapGetters, mapState } from 'vuex'; import { mapActions, mapGetters, mapState } from 'vuex';
import { IdState } from 'vendor/vue-virtual-scroller';
import createFlash from '~/flash'; import createFlash from '~/flash';
import { hasDiff } from '~/helpers/diffs_helper'; import { hasDiff } from '~/helpers/diffs_helper';
import { diffViewerErrors } from '~/ide/constants'; import { diffViewerErrors } from '~/ide/constants';
...@@ -34,7 +35,7 @@ export default { ...@@ -34,7 +35,7 @@ export default {
directives: { directives: {
SafeHtml, SafeHtml,
}, },
mixins: [glFeatureFlagsMixin()], mixins: [glFeatureFlagsMixin(), IdState({ idProp: (vm) => vm.file.file_hash })],
props: { props: {
file: { file: {
type: Object, type: Object,
...@@ -69,7 +70,7 @@ export default { ...@@ -69,7 +70,7 @@ export default {
required: true, required: true,
}, },
}, },
data() { idState() {
return { return {
isLoadingCollapsedDiff: false, isLoadingCollapsedDiff: false,
forkMessageVisible: false, forkMessageVisible: false,
...@@ -91,7 +92,9 @@ export default { ...@@ -91,7 +92,9 @@ export default {
return getShortShaFromFile(this.file); return getShortShaFromFile(this.file);
}, },
showLoadingIcon() { showLoadingIcon() {
return this.isLoadingCollapsedDiff || (!this.file.renderIt && !this.isCollapsed); return (
this.idState.isLoadingCollapsedDiff || (!this.file.renderIt && !this.idState.isCollapsed)
);
}, },
hasDiff() { hasDiff() {
return hasDiff(this.file); return hasDiff(this.file);
...@@ -135,13 +138,13 @@ export default { ...@@ -135,13 +138,13 @@ export default {
return collapsedType(this.file) === DIFF_FILE_MANUAL_COLLAPSE; return collapsedType(this.file) === DIFF_FILE_MANUAL_COLLAPSE;
}, },
showBody() { showBody() {
return !this.isCollapsed || this.automaticallyCollapsed; return !this.idState.isCollapsed || this.automaticallyCollapsed;
}, },
showWarning() { showWarning() {
return this.isCollapsed && this.automaticallyCollapsed && !this.viewDiffsFileByFile; return this.idState.isCollapsed && this.automaticallyCollapsed && !this.viewDiffsFileByFile;
}, },
showContent() { showContent() {
return !this.isCollapsed && !this.isFileTooLarge; return !this.idState.isCollapsed && !this.isFileTooLarge;
}, },
showLocalFileReviews() { showLocalFileReviews() {
const loggedIn = Boolean(gon.current_user_id); const loggedIn = Boolean(gon.current_user_id);
...@@ -167,23 +170,16 @@ export default { ...@@ -167,23 +170,16 @@ export default {
this.requestDiff(); this.requestDiff();
} }
}, },
immediate: true,
}, },
'file.viewer.automaticallyCollapsed': { 'file.viewer.automaticallyCollapsed': {
handler: function autoChangeWatch(automaticValue) { handler: function autoChangeWatch(automaticValue) {
if (collapsedType(this.file) !== DIFF_FILE_MANUAL_COLLAPSE) { this.handleAutomaticallyCollapsed(automaticValue);
this.isCollapsed = this.viewDiffsFileByFile ? false : automaticValue;
}
}, },
immediate: true,
}, },
'file.viewer.manuallyCollapsed': { 'file.viewer.manuallyCollapsed': {
handler: function manualChangeWatch(manualValue) { handler: function manualChangeWatch(manualValue) {
if (manualValue !== null) { this.handleManualChangeWatch(manualValue);
this.isCollapsed = manualValue;
}
}, },
immediate: true,
}, },
}, },
created() { created() {
...@@ -196,6 +192,8 @@ export default { ...@@ -196,6 +192,8 @@ export default {
} }
this.manageViewedEffects(); this.manageViewedEffects();
this.handleAutomaticallyCollapsed(this.file.viewer.automaticallyCollapsed);
this.handleManualChangeWatch(this.file.viewer.manuallyCollapsed);
}, },
beforeDestroy() { beforeDestroy() {
eventHub.$off(EVT_EXPAND_ALL_FILES, this.expandAllListener); eventHub.$off(EVT_EXPAND_ALL_FILES, this.expandAllListener);
...@@ -208,12 +206,12 @@ export default { ...@@ -208,12 +206,12 @@ export default {
'setFileCollapsedByUser', 'setFileCollapsedByUser',
]), ]),
manageViewedEffects() { manageViewedEffects() {
if (this.reviewed && !this.isCollapsed && this.showLocalFileReviews) { if (this.reviewed && !this.idState.isCollapsed && this.showLocalFileReviews) {
this.handleToggle(); this.handleToggle();
} }
}, },
expandAllListener() { expandAllListener() {
if (this.isCollapsed) { if (this.idState.isCollapsed) {
this.handleToggle(); this.handleToggle();
} }
}, },
...@@ -235,7 +233,7 @@ export default { ...@@ -235,7 +233,7 @@ export default {
}); });
}, },
handleToggle({ viaUserInteraction = false } = {}) { handleToggle({ viaUserInteraction = false } = {}) {
const collapsingNow = !this.isCollapsed; const collapsingNow = !this.idState.isCollapsed;
const contentElement = this.$el.querySelector(`#diff-content-${this.file.file_hash}`); const contentElement = this.$el.querySelector(`#diff-content-${this.file.file_hash}`);
this.setFileCollapsedByUser({ this.setFileCollapsedByUser({
...@@ -252,11 +250,11 @@ export default { ...@@ -252,11 +250,11 @@ export default {
} }
}, },
requestDiff() { requestDiff() {
this.isLoadingCollapsedDiff = true; this.idState.isLoadingCollapsedDiff = true;
this.loadCollapsedDiff(this.file) this.loadCollapsedDiff(this.file)
.then(() => { .then(() => {
this.isLoadingCollapsedDiff = false; this.idState.isLoadingCollapsedDiff = false;
this.setRenderIt(this.file); this.setRenderIt(this.file);
}) })
.then(() => { .then(() => {
...@@ -269,17 +267,27 @@ export default { ...@@ -269,17 +267,27 @@ export default {
); );
}) })
.catch(() => { .catch(() => {
this.isLoadingCollapsedDiff = false; this.idState.isLoadingCollapsedDiff = false;
createFlash({ createFlash({
message: this.$options.i18n.genericError, message: this.$options.i18n.genericError,
}); });
}); });
}, },
showForkMessage() { showForkMessage() {
this.forkMessageVisible = true; this.idState.forkMessageVisible = true;
}, },
hideForkMessage() { hideForkMessage() {
this.forkMessageVisible = false; this.idState.forkMessageVisible = false;
},
handleAutomaticallyCollapsed(automaticValue) {
if (collapsedType(this.file) !== DIFF_FILE_MANUAL_COLLAPSE) {
this.idState.isCollapsed = this.viewDiffsFileByFile ? false : automaticValue;
}
},
handleManualChangeWatch(manualValue) {
if (manualValue !== null) {
this.idState.isCollapsed = manualValue;
}
}, },
}, },
}; };
...@@ -302,7 +310,7 @@ export default { ...@@ -302,7 +310,7 @@ export default {
:diff-file="file" :diff-file="file"
:collapsible="true" :collapsible="true"
:reviewed="reviewed" :reviewed="reviewed"
:expanded="!isCollapsed" :expanded="!idState.isCollapsed"
:add-merge-request-buttons="true" :add-merge-request-buttons="true"
:view-diffs-file-by-file="viewDiffsFileByFile" :view-diffs-file-by-file="viewDiffsFileByFile"
:show-local-file-reviews="showLocalFileReviews" :show-local-file-reviews="showLocalFileReviews"
...@@ -313,7 +321,10 @@ export default { ...@@ -313,7 +321,10 @@ export default {
@showForkMessage="showForkMessage" @showForkMessage="showForkMessage"
/> />
<div v-if="forkMessageVisible" class="js-file-fork-suggestion-section file-fork-suggestion"> <div
v-if="idState.forkMessageVisible"
class="js-file-fork-suggestion-section file-fork-suggestion"
>
<span v-safe-html="forkMessage" class="file-fork-suggestion-note"></span> <span v-safe-html="forkMessage" class="file-fork-suggestion-note"></span>
<a <a
:href="file.fork_path" :href="file.fork_path"
......
...@@ -13,6 +13,7 @@ import { ...@@ -13,6 +13,7 @@ import {
} from '@gitlab/ui'; } from '@gitlab/ui';
import { escape } from 'lodash'; import { escape } from 'lodash';
import { mapActions, mapGetters, mapState } from 'vuex'; import { mapActions, mapGetters, mapState } from 'vuex';
import { IdState } from 'vendor/vue-virtual-scroller';
import { diffViewerModes } from '~/ide/constants'; import { diffViewerModes } from '~/ide/constants';
import { scrollToElement } from '~/lib/utils/common_utils'; import { scrollToElement } from '~/lib/utils/common_utils';
import { truncateSha } from '~/lib/utils/text_utility'; import { truncateSha } from '~/lib/utils/text_utility';
...@@ -47,7 +48,7 @@ export default { ...@@ -47,7 +48,7 @@ export default {
GlTooltip: GlTooltipDirective, GlTooltip: GlTooltipDirective,
SafeHtml: GlSafeHtmlDirective, SafeHtml: GlSafeHtmlDirective,
}, },
mixins: [glFeatureFlagsMixin()], mixins: [glFeatureFlagsMixin(), IdState({ idProp: (vm) => vm.diffFile.file_hash })],
i18n: { i18n: {
...DIFF_FILE_HEADER, ...DIFF_FILE_HEADER,
compareButtonLabel: s__('Compare submodule commit revisions'), compareButtonLabel: s__('Compare submodule commit revisions'),
...@@ -102,7 +103,7 @@ export default { ...@@ -102,7 +103,7 @@ export default {
default: () => [], default: () => [],
}, },
}, },
data() { idState() {
return { return {
moreActionsShown: false, moreActionsShown: false,
}; };
...@@ -239,7 +240,7 @@ export default { ...@@ -239,7 +240,7 @@ export default {
} }
}, },
setMoreActionsShown(val) { setMoreActionsShown(val) {
this.moreActionsShown = val; this.idState.moreActionsShown = val;
}, },
toggleReview(newReviewedStatus) { toggleReview(newReviewedStatus) {
const autoCollapsed = const autoCollapsed =
...@@ -268,7 +269,7 @@ export default { ...@@ -268,7 +269,7 @@ export default {
<template> <template>
<div <div
ref="header" ref="header"
:class="{ 'gl-z-dropdown-menu!': moreActionsShown }" :class="{ 'gl-z-dropdown-menu!': idState.moreActionsShown }"
class="js-file-title file-title file-title-flex-parent" class="js-file-title file-title file-title-flex-parent"
data-qa-selector="file_title_container" data-qa-selector="file_title_container"
:data-qa-file-name="filePath" :data-qa-file-name="filePath"
......
<script> <script>
import { mapGetters, mapState, mapActions } from 'vuex'; import { mapGetters, mapState, mapActions } from 'vuex';
import { IdState } from 'vendor/vue-virtual-scroller';
import DraftNote from '~/batch_comments/components/draft_note.vue'; import DraftNote from '~/batch_comments/components/draft_note.vue';
import draftCommentsMixin from '~/diffs/mixins/draft_comments'; import draftCommentsMixin from '~/diffs/mixins/draft_comments';
import { getCommentedLines } from '~/notes/components/multiline_comment_utils'; import { getCommentedLines } from '~/notes/components/multiline_comment_utils';
...@@ -17,7 +18,11 @@ export default { ...@@ -17,7 +18,11 @@ export default {
DiffCommentCell, DiffCommentCell,
DraftNote, DraftNote,
}, },
mixins: [draftCommentsMixin, glFeatureFlagsMixin()], mixins: [
draftCommentsMixin,
glFeatureFlagsMixin(),
IdState({ idProp: (vm) => vm.diffFile.file_hash }),
],
props: { props: {
diffFile: { diffFile: {
type: Object, type: Object,
...@@ -38,7 +43,7 @@ export default { ...@@ -38,7 +43,7 @@ export default {
default: false, default: false,
}, },
}, },
data() { idState() {
return { return {
dragStart: null, dragStart: null,
updatedLineRange: null, updatedLineRange: null,
...@@ -80,29 +85,29 @@ export default { ...@@ -80,29 +85,29 @@ export default {
if (event.target?.parentNode) { if (event.target?.parentNode) {
hide(event.target.parentNode); hide(event.target.parentNode);
} }
this.dragStart = line; this.idState.dragStart = line;
}, },
onDragOver(line) { onDragOver(line) {
if (line.chunk !== this.dragStart.chunk) return; if (line.chunk !== this.idState.dragStart.chunk) return;
let start = this.dragStart; let start = this.idState.dragStart;
let end = line; let end = line;
if (this.dragStart.index >= line.index) { if (this.idState.dragStart.index >= line.index) {
start = line; start = line;
end = this.dragStart; end = this.idState.dragStart;
} }
this.updatedLineRange = { start, end }; this.idState.updatedLineRange = { start, end };
this.setSelectedCommentPosition(this.updatedLineRange); this.setSelectedCommentPosition(this.idState.updatedLineRange);
}, },
onStopDragging() { onStopDragging() {
this.showCommentForm({ this.showCommentForm({
lineCode: this.updatedLineRange?.end?.line_code, lineCode: this.idState.updatedLineRange?.end?.line_code,
fileHash: this.diffFile.file_hash, fileHash: this.diffFile.file_hash,
}); });
this.dragStart = null; this.idState.dragStart = null;
}, },
isHighlighted(line) { isHighlighted(line) {
return isHighlighted( return isHighlighted(
......
...@@ -41,7 +41,7 @@ describe('DiffView', () => { ...@@ -41,7 +41,7 @@ describe('DiffView', () => {
}); });
const propsData = { const propsData = {
diffFile: {}, diffFile: { file_hash: '123' },
diffLines: [], diffLines: [],
...props, ...props,
}; };
...@@ -85,7 +85,7 @@ describe('DiffView', () => { ...@@ -85,7 +85,7 @@ describe('DiffView', () => {
const wrapper = createWrapper({ diffLines: [{}] }); const wrapper = createWrapper({ diffLines: [{}] });
wrapper.findComponent(DiffRow).vm.$emit('startdragging', { line: { test: true } }); wrapper.findComponent(DiffRow).vm.$emit('startdragging', { line: { test: true } });
expect(wrapper.vm.dragStart).toEqual({ test: true }); expect(wrapper.vm.idState.dragStart).toEqual({ test: true });
}); });
it('does not call `setSelectedCommentPosition` on different chunks onDragOver', () => { it('does not call `setSelectedCommentPosition` on different chunks onDragOver', () => {
...@@ -123,10 +123,10 @@ describe('DiffView', () => { ...@@ -123,10 +123,10 @@ describe('DiffView', () => {
const diffRow = getDiffRow(wrapper); const diffRow = getDiffRow(wrapper);
diffRow.$emit('startdragging', { line: { test: true } }); diffRow.$emit('startdragging', { line: { test: true } });
expect(wrapper.vm.dragStart).toEqual({ test: true }); expect(wrapper.vm.idState.dragStart).toEqual({ test: true });
diffRow.$emit('stopdragging'); diffRow.$emit('stopdragging');
expect(wrapper.vm.dragStart).toBeNull(); expect(wrapper.vm.idState.dragStart).toBeNull();
expect(showCommentForm).toHaveBeenCalled(); expect(showCommentForm).toHaveBeenCalled();
}); });
}); });
......
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