Commit 483864db authored by Fatih Acet's avatar Fatih Acet Committed by Clement Ho

Improve performance of toggling diff view type

parent 26998c68
......@@ -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() {
......
<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>
<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"
/>
......
<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"
......
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;
},
},
};
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 = {};
......
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