Merge branch '_acet-mr-diff-performance' into 'master'
Improve performance of toggling diff view type Closes #48666 and #48733 See merge request gitlab-org/gitlab-ce!20278
This commit is contained in:
commit
eba05eb8d4
|
@ -39,12 +39,12 @@ export default {
|
|||
<div class="diff-viewer">
|
||||
<template v-if="isTextFile">
|
||||
<inline-diff-view
|
||||
v-if="isInlineView"
|
||||
v-show="isInlineView"
|
||||
:diff-file="diffFile"
|
||||
:diff-lines="diffFile.highlightedDiffLines || []"
|
||||
/>
|
||||
<parallel-diff-view
|
||||
v-else-if="isParallelView"
|
||||
v-show="isParallelView"
|
||||
:diff-file="diffFile"
|
||||
:diff-lines="diffFile.parallelDiffLines || []"
|
||||
/>
|
||||
|
|
|
@ -10,6 +10,7 @@ import {
|
|||
NEW_NO_NEW_LINE_TYPE,
|
||||
LINE_HOVER_CLASS_NAME,
|
||||
LINE_UNFOLD_CLASS_NAME,
|
||||
INLINE_DIFF_VIEW_TYPE,
|
||||
} from '../constants';
|
||||
|
||||
export default {
|
||||
|
@ -25,6 +26,11 @@ export default {
|
|||
type: Object,
|
||||
required: true,
|
||||
},
|
||||
diffViewType: {
|
||||
type: String,
|
||||
required: false,
|
||||
default: INLINE_DIFF_VIEW_TYPE,
|
||||
},
|
||||
showCommentButton: {
|
||||
type: Boolean,
|
||||
required: false,
|
||||
|
@ -57,9 +63,9 @@ export default {
|
|||
},
|
||||
},
|
||||
computed: {
|
||||
...mapGetters(['isLoggedIn', 'isInlineView']),
|
||||
...mapGetters(['isLoggedIn']),
|
||||
normalizedLine() {
|
||||
if (this.isInlineView) {
|
||||
if (this.diffViewType === INLINE_DIFF_VIEW_TYPE) {
|
||||
return this.line;
|
||||
}
|
||||
|
||||
|
@ -72,10 +78,10 @@ export default {
|
|||
return this.normalizedLine.type === CONTEXT_LINE_TYPE;
|
||||
},
|
||||
isMetaLine() {
|
||||
const { type } = this.normalizedLine;
|
||||
|
||||
return (
|
||||
this.normalizedLine.type === OLD_NO_NEW_LINE_TYPE ||
|
||||
this.normalizedLine.type === NEW_NO_NEW_LINE_TYPE ||
|
||||
this.normalizedLine.type === EMPTY_CELL_TYPE
|
||||
type === OLD_NO_NEW_LINE_TYPE || type === NEW_NO_NEW_LINE_TYPE || type === EMPTY_CELL_TYPE
|
||||
);
|
||||
},
|
||||
classNameMap() {
|
||||
|
|
|
@ -0,0 +1,104 @@
|
|||
<script>
|
||||
import { mapGetters } from 'vuex';
|
||||
import DiffTableCell from './diff_table_cell.vue';
|
||||
import {
|
||||
NEW_LINE_TYPE,
|
||||
OLD_LINE_TYPE,
|
||||
CONTEXT_LINE_TYPE,
|
||||
CONTEXT_LINE_CLASS_NAME,
|
||||
PARALLEL_DIFF_VIEW_TYPE,
|
||||
LINE_POSITION_LEFT,
|
||||
LINE_POSITION_RIGHT,
|
||||
} from '../constants';
|
||||
|
||||
export default {
|
||||
components: {
|
||||
DiffTableCell,
|
||||
},
|
||||
props: {
|
||||
diffFile: {
|
||||
type: Object,
|
||||
required: true,
|
||||
},
|
||||
line: {
|
||||
type: Object,
|
||||
required: true,
|
||||
},
|
||||
isBottom: {
|
||||
type: Boolean,
|
||||
required: false,
|
||||
default: false,
|
||||
},
|
||||
},
|
||||
data() {
|
||||
return {
|
||||
isHover: false,
|
||||
};
|
||||
},
|
||||
computed: {
|
||||
...mapGetters(['isInlineView']),
|
||||
isContextLine() {
|
||||
return this.line.type === CONTEXT_LINE_TYPE;
|
||||
},
|
||||
classNameMap() {
|
||||
return {
|
||||
[this.line.type]: this.line.type,
|
||||
[CONTEXT_LINE_CLASS_NAME]: this.isContextLine,
|
||||
[PARALLEL_DIFF_VIEW_TYPE]: this.isParallelView,
|
||||
};
|
||||
},
|
||||
inlineRowId() {
|
||||
const { lineCode, oldLine, newLine } = this.line;
|
||||
|
||||
return lineCode || `${this.diffFile.fileHash}_${oldLine}_${newLine}`;
|
||||
},
|
||||
},
|
||||
created() {
|
||||
this.newLineType = NEW_LINE_TYPE;
|
||||
this.oldLineType = OLD_LINE_TYPE;
|
||||
this.linePositionLeft = LINE_POSITION_LEFT;
|
||||
this.linePositionRight = LINE_POSITION_RIGHT;
|
||||
},
|
||||
methods: {
|
||||
handleMouseMove(e) {
|
||||
// To show the comment icon on the gutter we need to know if we hover the line.
|
||||
// Current table structure doesn't allow us to do this with CSS in both of the diff view types
|
||||
this.isHover = e.type === 'mouseover';
|
||||
},
|
||||
},
|
||||
};
|
||||
</script>
|
||||
|
||||
<template>
|
||||
<tr
|
||||
:id="inlineRowId"
|
||||
:class="classNameMap"
|
||||
class="line_holder"
|
||||
@mouseover="handleMouseMove"
|
||||
@mouseout="handleMouseMove"
|
||||
>
|
||||
<diff-table-cell
|
||||
:diff-file="diffFile"
|
||||
:line="line"
|
||||
:line-type="oldLineType"
|
||||
:is-bottom="isBottom"
|
||||
:is-hover="isHover"
|
||||
:show-comment-button="true"
|
||||
class="diff-line-num old_line"
|
||||
/>
|
||||
<diff-table-cell
|
||||
:diff-file="diffFile"
|
||||
:line="line"
|
||||
:line-type="newLineType"
|
||||
:is-bottom="isBottom"
|
||||
:is-hover="isHover"
|
||||
class="diff-line-num new_line"
|
||||
/>
|
||||
<diff-table-cell
|
||||
:class="line.type"
|
||||
:diff-file="diffFile"
|
||||
:line="line"
|
||||
:is-content-line="true"
|
||||
/>
|
||||
</tr>
|
||||
</template>
|
|
@ -1,12 +1,39 @@
|
|||
<script>
|
||||
import diffContentMixin from '../mixins/diff_content';
|
||||
import { mapGetters } from 'vuex';
|
||||
import inlineDiffTableRow from './inline_diff_table_row.vue';
|
||||
import inlineDiffCommentRow from './inline_diff_comment_row.vue';
|
||||
import { trimFirstCharOfLineContent } from '../store/utils';
|
||||
|
||||
export default {
|
||||
components: {
|
||||
inlineDiffCommentRow,
|
||||
inlineDiffTableRow,
|
||||
},
|
||||
props: {
|
||||
diffFile: {
|
||||
type: Object,
|
||||
required: true,
|
||||
},
|
||||
diffLines: {
|
||||
type: Array,
|
||||
required: true,
|
||||
},
|
||||
},
|
||||
computed: {
|
||||
...mapGetters(['commit']),
|
||||
normalizedDiffLines() {
|
||||
return this.diffLines.map(line => (line.richText ? trimFirstCharOfLineContent(line) : line));
|
||||
},
|
||||
diffLinesLength() {
|
||||
return this.normalizedDiffLines.length;
|
||||
},
|
||||
commitId() {
|
||||
return this.commit && this.commit.id;
|
||||
},
|
||||
userColorScheme() {
|
||||
return window.gon.user_color_scheme;
|
||||
},
|
||||
},
|
||||
mixins: [diffContentMixin],
|
||||
};
|
||||
</script>
|
||||
|
||||
|
@ -19,7 +46,7 @@ export default {
|
|||
<template
|
||||
v-for="(line, index) in normalizedDiffLines"
|
||||
>
|
||||
<diff-table-row
|
||||
<inline-diff-table-row
|
||||
:diff-file="diffFile"
|
||||
:line="line"
|
||||
:is-bottom="index + 1 === diffLinesLength"
|
||||
|
|
|
@ -35,30 +35,21 @@ export default {
|
|||
},
|
||||
data() {
|
||||
return {
|
||||
isHover: false,
|
||||
isLeftHover: false,
|
||||
isRightHover: false,
|
||||
};
|
||||
},
|
||||
computed: {
|
||||
...mapGetters(['isInlineView', 'isParallelView']),
|
||||
...mapGetters(['isParallelView']),
|
||||
isContextLine() {
|
||||
return this.line.left
|
||||
? this.line.left.type === CONTEXT_LINE_TYPE
|
||||
: this.line.type === CONTEXT_LINE_TYPE;
|
||||
return this.line.left.type === CONTEXT_LINE_TYPE;
|
||||
},
|
||||
classNameMap() {
|
||||
return {
|
||||
[this.line.type]: this.line.type,
|
||||
[CONTEXT_LINE_CLASS_NAME]: this.isContextLine,
|
||||
[PARALLEL_DIFF_VIEW_TYPE]: this.isParallelView,
|
||||
};
|
||||
},
|
||||
inlineRowId() {
|
||||
const { lineCode, oldLine, newLine } = this.line;
|
||||
|
||||
return lineCode || `${this.diffFile.fileHash}_${oldLine}_${newLine}`;
|
||||
},
|
||||
parallelViewLeftLineType() {
|
||||
if (this.line.right.type === NEW_NO_NEW_LINE_TYPE) {
|
||||
return OLD_NO_NEW_LINE_TYPE;
|
||||
|
@ -72,23 +63,19 @@ export default {
|
|||
this.oldLineType = OLD_LINE_TYPE;
|
||||
this.linePositionLeft = LINE_POSITION_LEFT;
|
||||
this.linePositionRight = LINE_POSITION_RIGHT;
|
||||
this.parallelDiffViewType = PARALLEL_DIFF_VIEW_TYPE;
|
||||
},
|
||||
methods: {
|
||||
handleMouseMove(e) {
|
||||
const isHover = e.type === 'mouseover';
|
||||
const hoveringCell = e.target.closest('td');
|
||||
const allCellsInHoveringRow = Array.from(e.currentTarget.children);
|
||||
const hoverIndex = allCellsInHoveringRow.indexOf(hoveringCell);
|
||||
|
||||
if (this.isInlineView) {
|
||||
this.isHover = isHover;
|
||||
if (hoverIndex >= 2) {
|
||||
this.isRightHover = isHover;
|
||||
} else {
|
||||
const hoveringCell = e.target.closest('td');
|
||||
const allCellsInHoveringRow = Array.from(e.currentTarget.children);
|
||||
const hoverIndex = allCellsInHoveringRow.indexOf(hoveringCell);
|
||||
|
||||
if (hoverIndex >= 2) {
|
||||
this.isRightHover = isHover;
|
||||
} else {
|
||||
this.isLeftHover = isHover;
|
||||
}
|
||||
this.isLeftHover = isHover;
|
||||
}
|
||||
},
|
||||
// Prevent text selecting on both sides of parallel diff view
|
||||
|
@ -110,40 +97,6 @@ export default {
|
|||
|
||||
<template>
|
||||
<tr
|
||||
v-if="isInlineView"
|
||||
:id="inlineRowId"
|
||||
:class="classNameMap"
|
||||
class="line_holder"
|
||||
@mouseover="handleMouseMove"
|
||||
@mouseout="handleMouseMove"
|
||||
>
|
||||
<diff-table-cell
|
||||
:diff-file="diffFile"
|
||||
:line="line"
|
||||
:line-type="oldLineType"
|
||||
:is-bottom="isBottom"
|
||||
:is-hover="isHover"
|
||||
:show-comment-button="true"
|
||||
class="diff-line-num old_line"
|
||||
/>
|
||||
<diff-table-cell
|
||||
:diff-file="diffFile"
|
||||
:line="line"
|
||||
:line-type="newLineType"
|
||||
:is-bottom="isBottom"
|
||||
:is-hover="isHover"
|
||||
class="diff-line-num new_line"
|
||||
/>
|
||||
<diff-table-cell
|
||||
:class="line.type"
|
||||
:diff-file="diffFile"
|
||||
:line="line"
|
||||
:is-content-line="true"
|
||||
/>
|
||||
</tr>
|
||||
|
||||
<tr
|
||||
v-else
|
||||
:class="classNameMap"
|
||||
class="line_holder"
|
||||
@mouseover="handleMouseMove"
|
||||
|
@ -157,6 +110,7 @@ export default {
|
|||
:is-bottom="isBottom"
|
||||
:is-hover="isLeftHover"
|
||||
:show-comment-button="true"
|
||||
:diff-view-type="parallelDiffViewType"
|
||||
class="diff-line-num old_line"
|
||||
/>
|
||||
<diff-table-cell
|
||||
|
@ -165,6 +119,7 @@ export default {
|
|||
:line="line"
|
||||
:is-content-line="true"
|
||||
:line-type="parallelViewLeftLineType"
|
||||
:diff-view-type="parallelDiffViewType"
|
||||
class="line_content parallel left-side"
|
||||
@mousedown.native="handleParallelLineMouseDown"
|
||||
/>
|
||||
|
@ -176,6 +131,7 @@ export default {
|
|||
:is-bottom="isBottom"
|
||||
:is-hover="isRightHover"
|
||||
:show-comment-button="true"
|
||||
:diff-view-type="parallelDiffViewType"
|
||||
class="diff-line-num new_line"
|
||||
/>
|
||||
<diff-table-cell
|
||||
|
@ -184,6 +140,7 @@ export default {
|
|||
:line="line"
|
||||
:is-content-line="true"
|
||||
:line-type="line.right.type"
|
||||
:diff-view-type="parallelDiffViewType"
|
||||
class="line_content parallel right-side"
|
||||
@mousedown.native="handleParallelLineMouseDown"
|
||||
/>
|
|
@ -1,25 +1,53 @@
|
|||
<script>
|
||||
import diffContentMixin from '../mixins/diff_content';
|
||||
import { mapGetters } from 'vuex';
|
||||
import parallelDiffTableRow from './parallel_diff_table_row.vue';
|
||||
import parallelDiffCommentRow from './parallel_diff_comment_row.vue';
|
||||
import { EMPTY_CELL_TYPE } from '../constants';
|
||||
import { trimFirstCharOfLineContent } from '../store/utils';
|
||||
|
||||
export default {
|
||||
components: {
|
||||
parallelDiffTableRow,
|
||||
parallelDiffCommentRow,
|
||||
},
|
||||
mixins: [diffContentMixin],
|
||||
props: {
|
||||
diffFile: {
|
||||
type: Object,
|
||||
required: true,
|
||||
},
|
||||
diffLines: {
|
||||
type: Array,
|
||||
required: true,
|
||||
},
|
||||
},
|
||||
computed: {
|
||||
...mapGetters(['commit']),
|
||||
parallelDiffLines() {
|
||||
return this.normalizedDiffLines.map(line => {
|
||||
if (!line.left) {
|
||||
return this.diffLines.map(line => {
|
||||
if (line.left) {
|
||||
Object.assign(line, { left: trimFirstCharOfLineContent(line.left) });
|
||||
} else {
|
||||
Object.assign(line, { left: { type: EMPTY_CELL_TYPE } });
|
||||
} else if (!line.right) {
|
||||
}
|
||||
|
||||
if (line.right) {
|
||||
Object.assign(line, { right: trimFirstCharOfLineContent(line.right) });
|
||||
} else {
|
||||
Object.assign(line, { right: { type: EMPTY_CELL_TYPE } });
|
||||
}
|
||||
|
||||
return line;
|
||||
});
|
||||
},
|
||||
diffLinesLength() {
|
||||
return this.parallelDiffLines.length;
|
||||
},
|
||||
commitId() {
|
||||
return this.commit && this.commit.id;
|
||||
},
|
||||
userColorScheme() {
|
||||
return window.gon.user_color_scheme;
|
||||
},
|
||||
},
|
||||
};
|
||||
</script>
|
||||
|
@ -35,7 +63,7 @@ export default {
|
|||
<template
|
||||
v-for="(line, index) in parallelDiffLines"
|
||||
>
|
||||
<diff-table-row
|
||||
<parallel-diff-table-row
|
||||
:diff-file="diffFile"
|
||||
:line="line"
|
||||
:is-bottom="index + 1 === diffLinesLength"
|
||||
|
|
|
@ -1,57 +0,0 @@
|
|||
import { mapGetters } from 'vuex';
|
||||
import diffDiscussions from '../components/diff_discussions.vue';
|
||||
import diffLineGutterContent from '../components/diff_line_gutter_content.vue';
|
||||
import diffLineNoteForm from '../components/diff_line_note_form.vue';
|
||||
import diffTableRow from '../components/diff_table_row.vue';
|
||||
import { trimFirstCharOfLineContent } from '../store/utils';
|
||||
|
||||
export default {
|
||||
props: {
|
||||
diffFile: {
|
||||
type: Object,
|
||||
required: true,
|
||||
},
|
||||
diffLines: {
|
||||
type: Array,
|
||||
required: true,
|
||||
},
|
||||
},
|
||||
components: {
|
||||
diffDiscussions,
|
||||
diffTableRow,
|
||||
diffLineNoteForm,
|
||||
diffLineGutterContent,
|
||||
},
|
||||
computed: {
|
||||
...mapGetters(['commit']),
|
||||
commitId() {
|
||||
return this.commit && this.commit.id;
|
||||
},
|
||||
userColorScheme() {
|
||||
return window.gon.user_color_scheme;
|
||||
},
|
||||
normalizedDiffLines() {
|
||||
return this.diffLines.map(line => {
|
||||
if (line.richText) {
|
||||
return trimFirstCharOfLineContent(line);
|
||||
}
|
||||
|
||||
if (line.left) {
|
||||
Object.assign(line, { left: trimFirstCharOfLineContent(line.left) });
|
||||
}
|
||||
|
||||
if (line.right) {
|
||||
Object.assign(line, { right: trimFirstCharOfLineContent(line.right) });
|
||||
}
|
||||
|
||||
return line;
|
||||
});
|
||||
},
|
||||
diffLinesLength() {
|
||||
return this.normalizedDiffLines.length;
|
||||
},
|
||||
fileHash() {
|
||||
return this.diffFile.fileHash;
|
||||
},
|
||||
},
|
||||
};
|
|
@ -1,7 +1,6 @@
|
|||
export const SET_BASE_CONFIG = 'SET_BASE_CONFIG';
|
||||
export const SET_LOADING = 'SET_LOADING';
|
||||
export const SET_DIFF_DATA = 'SET_DIFF_DATA';
|
||||
export const SET_DIFF_FILES = 'SET_DIFF_FILES';
|
||||
export const SET_DIFF_VIEW_TYPE = 'SET_DIFF_VIEW_TYPE';
|
||||
export const SET_MERGE_REQUEST_DIFFS = 'SET_MERGE_REQUEST_DIFFS';
|
||||
export const ADD_COMMENT_FORM_LINE = 'ADD_COMMENT_FORM_LINE';
|
||||
|
|
|
@ -20,12 +20,6 @@ export default {
|
|||
});
|
||||
},
|
||||
|
||||
[types.SET_DIFF_FILES](state, diffFiles) {
|
||||
Object.assign(state, {
|
||||
diffFiles: convertObjectPropsToCamelCase(diffFiles, { deep: true }),
|
||||
});
|
||||
},
|
||||
|
||||
[types.SET_MERGE_REQUEST_DIFFS](state, mergeRequestDiffs) {
|
||||
Object.assign(state, {
|
||||
mergeRequestDiffs: convertObjectPropsToCamelCase(mergeRequestDiffs, { deep: true }),
|
||||
|
|
|
@ -24,21 +24,6 @@ describe('DiffsStoreMutations', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('SET_DIFF_FILES', () => {
|
||||
it('should set diff files to state', () => {
|
||||
const filePath = '/first-diff-file-path';
|
||||
const state = {};
|
||||
const diffFiles = {
|
||||
a_mode: 1,
|
||||
highlighted_diff_lines: [{ file_path: filePath }],
|
||||
};
|
||||
|
||||
mutations[types.SET_DIFF_FILES](state, diffFiles);
|
||||
expect(state.diffFiles.aMode).toEqual(1);
|
||||
expect(state.diffFiles.highlightedDiffLines[0].filePath).toEqual(filePath);
|
||||
});
|
||||
});
|
||||
|
||||
describe('SET_DIFF_VIEW_TYPE', () => {
|
||||
it('should set diff view type properly', () => {
|
||||
const state = {};
|
||||
|
|
Loading…
Reference in New Issue