diff options
author | Fatih Acet <acetfatih@gmail.com> | 2018-07-03 23:18:27 +0000 |
---|---|---|
committer | Clement Ho <clemmakesapps@gmail.com> | 2018-07-03 23:18:27 +0000 |
commit | 483864db77acb6db479ecdb8ce4dd121731a8330 (patch) | |
tree | 68fad0ba107207a5df667805bbe1af4990b50175 /app/assets | |
parent | 26998c68c936f183ead1a84e404a61160fc646f7 (diff) | |
download | gitlab-ce-483864db77acb6db479ecdb8ce4dd121731a8330.tar.gz |
Improve performance of toggling diff view type
Diffstat (limited to 'app/assets')
-rw-r--r-- | app/assets/javascripts/diffs/components/diff_content.vue | 4 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/components/diff_table_cell.vue | 16 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/components/inline_diff_table_row.vue | 104 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/components/inline_diff_view.vue | 33 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/components/parallel_diff_table_row.vue (renamed from app/assets/javascripts/diffs/components/diff_table_row.vue) | 69 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/components/parallel_diff_view.vue | 40 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/mixins/diff_content.js | 57 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/store/mutation_types.js | 1 | ||||
-rw-r--r-- | app/assets/javascripts/diffs/store/mutations.js | 6 |
9 files changed, 194 insertions, 136 deletions
diff --git a/app/assets/javascripts/diffs/components/diff_content.vue b/app/assets/javascripts/diffs/components/diff_content.vue index 48ba967285f..b6af49c7e2e 100644 --- a/app/assets/javascripts/diffs/components/diff_content.vue +++ b/app/assets/javascripts/diffs/components/diff_content.vue @@ -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 || []" /> diff --git a/app/assets/javascripts/diffs/components/diff_table_cell.vue b/app/assets/javascripts/diffs/components/diff_table_cell.vue index 68fe6787f9b..fdefd63ced2 100644 --- a/app/assets/javascripts/diffs/components/diff_table_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_table_cell.vue @@ -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() { diff --git a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue new file mode 100644 index 00000000000..a2470843ca6 --- /dev/null +++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue @@ -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> diff --git a/app/assets/javascripts/diffs/components/inline_diff_view.vue b/app/assets/javascripts/diffs/components/inline_diff_view.vue index e72f85df77a..b884230fb63 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_view.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_view.vue @@ -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" diff --git a/app/assets/javascripts/diffs/components/diff_table_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue index 8716fdaf44d..9cb68971c49 100644 --- a/app/assets/javascripts/diffs/components/diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue @@ -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" /> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_view.vue b/app/assets/javascripts/diffs/components/parallel_diff_view.vue index ed92b4ee249..d7280338b68 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_view.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_view.vue @@ -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" diff --git a/app/assets/javascripts/diffs/mixins/diff_content.js b/app/assets/javascripts/diffs/mixins/diff_content.js deleted file mode 100644 index ebb511d3a7e..00000000000 --- a/app/assets/javascripts/diffs/mixins/diff_content.js +++ /dev/null @@ -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; - }, - }, -}; diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index 63e9239dce4..2c8e1a1466f 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -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'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 339a33f8b71..8aa8a114c6f 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -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 }), |