summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPhil Hughes <me@iamphill.com>2019-05-03 08:33:15 +0100
committerPhil Hughes <me@iamphill.com>2019-05-03 08:33:15 +0100
commit9d24d4a8fdd299a1e84f2e549fb58ee526a2f0f9 (patch)
tree5644c2b7f96e0b668e45ef39f5a959236fd06633
parentb75e92a1b8e6100e4c099fc2f941c6e73f5632e6 (diff)
downloadgitlab-ce-9d24d4a8fdd299a1e84f2e549fb58ee526a2f0f9.tar.gz
Impove the performance of expanding full diff
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/58597
-rw-r--r--app/assets/javascripts/diffs/components/diff_content.vue3
-rw-r--r--app/assets/javascripts/diffs/components/diff_line_gutter_content.vue6
-rw-r--r--app/assets/javascripts/diffs/components/diff_table_cell.vue24
-rw-r--r--app/assets/javascripts/diffs/components/inline_diff_table_row.vue15
-rw-r--r--app/assets/javascripts/diffs/constants.js7
-rw-r--r--app/assets/javascripts/diffs/store/actions.js113
-rw-r--r--app/assets/javascripts/diffs/store/mutation_types.js5
-rw-r--r--app/assets/javascripts/diffs/store/mutations.js64
-rw-r--r--app/assets/javascripts/diffs/store/utils.js33
-rw-r--r--changelogs/unreleased/expand-diff-performance.yml5
-rw-r--r--spec/javascripts/diffs/components/diff_content_spec.js14
-rw-r--r--spec/javascripts/diffs/mock_data/diff_file.js1
-rw-r--r--spec/javascripts/diffs/store/actions_spec.js92
-rw-r--r--spec/javascripts/diffs/store/mutations_spec.js94
14 files changed, 379 insertions, 97 deletions
diff --git a/app/assets/javascripts/diffs/components/diff_content.vue b/app/assets/javascripts/diffs/components/diff_content.vue
index 8d09c2a7399..2b3d6d1a3fa 100644
--- a/app/assets/javascripts/diffs/components/diff_content.vue
+++ b/app/assets/javascripts/diffs/components/diff_content.vue
@@ -1,5 +1,6 @@
<script>
import { mapActions, mapGetters, mapState } from 'vuex';
+import { GlLoadingIcon } from '@gitlab/ui';
import diffLineNoteFormMixin from 'ee_else_ce/notes/mixins/diff_line_note_form';
import draftCommentsMixin from 'ee_else_ce/diffs/mixins/draft_comments';
import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue';
@@ -16,6 +17,7 @@ import { diffViewerModes } from '~/ide/constants';
export default {
components: {
+ GlLoadingIcon,
InlineDiffView,
ParallelDiffView,
DiffViewer,
@@ -108,6 +110,7 @@ export default {
:diff-lines="diffFile.parallel_diff_lines || []"
:help-page-path="helpPagePath"
/>
+ <gl-loading-icon v-if="diffFile.renderingLines" size="md" class="mt-3" />
</template>
<not-diffable-viewer v-else-if="notDiffable" />
<no-preview-viewer v-else-if="noPreview" />
diff --git a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
index 6709df48637..1281f9b17ef 100644
--- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
+++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
@@ -84,8 +84,6 @@ export default {
},
shouldShowCommentButton() {
return (
- this.isLoggedIn &&
- this.showCommentButton &&
this.isHover &&
!this.isMatchLine &&
!this.isContextLine &&
@@ -102,6 +100,9 @@ export default {
}
return this.showCommentButton && this.hasDiscussions;
},
+ shouldRenderCommentButton() {
+ return this.isLoggedIn && this.showCommentButton;
+ },
},
methods: {
...mapActions('diffs', ['loadMoreLines', 'showCommentForm', 'setHighlightedRow']),
@@ -167,6 +168,7 @@ export default {
>
<template v-else>
<button
+ v-if="shouldRenderCommentButton"
v-show="shouldShowCommentButton"
type="button"
class="add-diff-note js-add-diff-note-button qa-diff-comment"
diff --git a/app/assets/javascripts/diffs/components/diff_table_cell.vue b/app/assets/javascripts/diffs/components/diff_table_cell.vue
index d174b13e133..0f3e9208d21 100644
--- a/app/assets/javascripts/diffs/components/diff_table_cell.vue
+++ b/app/assets/javascripts/diffs/components/diff_table_cell.vue
@@ -89,17 +89,19 @@ export default {
classNameMap() {
const { type } = this.line;
- return {
- hll: this.isHighlighted,
- [type]: type,
- [LINE_UNFOLD_CLASS_NAME]: this.isMatchLine,
- [LINE_HOVER_CLASS_NAME]:
- this.isLoggedIn &&
- this.isHover &&
- !this.isMatchLine &&
- !this.isContextLine &&
- !this.isMetaLine,
- };
+ return [
+ type,
+ {
+ hll: this.isHighlighted,
+ [LINE_UNFOLD_CLASS_NAME]: this.isMatchLine,
+ [LINE_HOVER_CLASS_NAME]:
+ this.isLoggedIn &&
+ this.isHover &&
+ !this.isMatchLine &&
+ !this.isContextLine &&
+ !this.isMetaLine,
+ },
+ ];
},
lineNumber() {
return this.lineType === OLD_LINE_TYPE ? this.line.old_line : this.line.new_line;
diff --git a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue
index c764cbeb8e0..2d5262baeec 100644
--- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue
+++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue
@@ -1,12 +1,11 @@
<script>
-import { mapGetters, mapActions, mapState } from 'vuex';
+import { mapActions, mapState } 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';
@@ -45,16 +44,16 @@ export default {
return this.line.line_code !== null && this.line.line_code === state.diffs.highlightedRow;
},
}),
- ...mapGetters('diffs', ['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,
- };
+ return [
+ this.line.type,
+ {
+ [CONTEXT_LINE_CLASS_NAME]: this.isContextLine,
+ },
+ ];
},
inlineRowId() {
return this.line.line_code || `${this.fileHash}_${this.line.old_line}_${this.line.new_line}`;
diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js
index 4feb73cfef2..d84e1af11f3 100644
--- a/app/assets/javascripts/diffs/constants.js
+++ b/app/assets/javascripts/diffs/constants.js
@@ -50,3 +50,10 @@ export const LEFT_LINE_KEY = 'left';
export const CENTERED_LIMITED_CONTAINER_CLASSES =
'container-limited limit-container-width mx-lg-auto px-3';
+
+export const MAX_RENDERING_DIFF_LINES = 500;
+export const MAX_RENDERING_BULK_ROWS = 30;
+export const MIN_RENDERING_MS = 2;
+export const START_RENDERING_INDEX = 200;
+export const INLINE_DIFF_LINES_KEY = 'highlighted_diff_lines';
+export const PARALLEL_DIFF_LINES_KEY = 'parallel_diff_lines';
diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js
index b58ae0d248c..386d08aed2b 100644
--- a/app/assets/javascripts/diffs/store/actions.js
+++ b/app/assets/javascripts/diffs/store/actions.js
@@ -7,7 +7,12 @@ import { handleLocationHash, historyPushState, scrollToElement } from '~/lib/uti
import { mergeUrlParams, getLocationHash } from '~/lib/utils/url_utility';
import TreeWorker from '../workers/tree_worker';
import eventHub from '../../notes/event_hub';
-import { getDiffPositionByLineCode, getNoteFormData } from './utils';
+import {
+ getDiffPositionByLineCode,
+ getNoteFormData,
+ convertExpandLines,
+ idleCallback,
+} from './utils';
import * as types from './mutation_types';
import {
PARALLEL_DIFF_VIEW_TYPE,
@@ -17,6 +22,16 @@ import {
TREE_LIST_STORAGE_KEY,
WHITESPACE_STORAGE_KEY,
TREE_LIST_WIDTH_STORAGE_KEY,
+ OLD_LINE_KEY,
+ NEW_LINE_KEY,
+ TYPE_KEY,
+ LEFT_LINE_KEY,
+ MAX_RENDERING_DIFF_LINES,
+ MAX_RENDERING_BULK_ROWS,
+ MIN_RENDERING_MS,
+ START_RENDERING_INDEX,
+ INLINE_DIFF_LINES_KEY,
+ PARALLEL_DIFF_LINES_KEY,
} from '../constants';
import { diffViewerModes } from '~/ide/constants';
@@ -313,13 +328,98 @@ export const cacheTreeListWidth = (_, size) => {
};
export const requestFullDiff = ({ commit }, filePath) => commit(types.REQUEST_FULL_DIFF, filePath);
-export const receiveFullDiffSucess = ({ commit }, { filePath, data }) =>
- commit(types.RECEIVE_FULL_DIFF_SUCCESS, { filePath, data });
+export const receiveFullDiffSucess = ({ commit }, { filePath }) =>
+ commit(types.RECEIVE_FULL_DIFF_SUCCESS, { filePath });
export const receiveFullDiffError = ({ commit }, filePath) => {
commit(types.RECEIVE_FULL_DIFF_ERROR, filePath);
createFlash(s__('MergeRequest|Error loading full diff. Please try again.'));
};
+export const setExpandedDiffLines = ({ commit, state }, { file, data }) => {
+ const expandedDiffLines = {
+ highlighted_diff_lines: convertExpandLines({
+ diffLines: file.highlighted_diff_lines,
+ typeKey: TYPE_KEY,
+ oldLineKey: OLD_LINE_KEY,
+ newLineKey: NEW_LINE_KEY,
+ data,
+ mapLine: ({ line, oldLine, newLine }) =>
+ Object.assign(line, {
+ old_line: oldLine,
+ new_line: newLine,
+ line_code: `${file.file_hash}_${oldLine}_${newLine}`,
+ }),
+ }),
+ parallel_diff_lines: convertExpandLines({
+ diffLines: file.parallel_diff_lines,
+ typeKey: [LEFT_LINE_KEY, TYPE_KEY],
+ oldLineKey: [LEFT_LINE_KEY, OLD_LINE_KEY],
+ newLineKey: [LEFT_LINE_KEY, NEW_LINE_KEY],
+ data,
+ mapLine: ({ line, oldLine, newLine }) => ({
+ left: {
+ ...line,
+ old_line: oldLine,
+ line_code: `${file.file_hash}_${oldLine}_${newLine}`,
+ },
+ right: {
+ ...line,
+ new_line: newLine,
+ line_code: `${file.file_hash}_${newLine}_${oldLine}`,
+ },
+ }),
+ }),
+ };
+ const currentDiffLinesKey =
+ state.diffViewType === INLINE_DIFF_VIEW_TYPE ? INLINE_DIFF_LINES_KEY : PARALLEL_DIFF_LINES_KEY;
+ const hiddenDiffLinesKey =
+ state.diffViewType === INLINE_DIFF_VIEW_TYPE ? PARALLEL_DIFF_LINES_KEY : INLINE_DIFF_LINES_KEY;
+
+ commit(types.SET_HIDDEN_VIEW_DIFF_FILE_LINES, {
+ filePath: file.file_path,
+ lines: expandedDiffLines[hiddenDiffLinesKey],
+ });
+
+ if (expandedDiffLines[currentDiffLinesKey].length > MAX_RENDERING_DIFF_LINES) {
+ let index = START_RENDERING_INDEX;
+ commit(types.SET_CURRENT_VIEW_DIFF_FILE_LINES, {
+ filePath: file.file_path,
+ lines: expandedDiffLines[currentDiffLinesKey].slice(0, index),
+ });
+ commit(types.TOGGLE_DIFF_FILE_RENDERING_MORE, file.file_path);
+
+ const idleCb = t => {
+ const startIndex = index;
+
+ while (
+ t.timeRemaining() >= MIN_RENDERING_MS &&
+ index !== expandedDiffLines[currentDiffLinesKey].length &&
+ index - startIndex !== MAX_RENDERING_BULK_ROWS
+ ) {
+ const line = expandedDiffLines[currentDiffLinesKey][index];
+
+ if (line) {
+ commit(types.ADD_CURRENT_VIEW_DIFF_FILE_LINES, { filePath: file.file_path, line });
+ index += 1;
+ }
+ }
+
+ if (index !== expandedDiffLines[currentDiffLinesKey].length) {
+ idleCallback(idleCb);
+ } else {
+ commit(types.TOGGLE_DIFF_FILE_RENDERING_MORE, file.file_path);
+ }
+ };
+
+ idleCallback(idleCb);
+ } else {
+ commit(types.SET_CURRENT_VIEW_DIFF_FILE_LINES, {
+ filePath: file.file_path,
+ lines: expandedDiffLines[currentDiffLinesKey],
+ });
+ }
+};
+
export const fetchFullDiff = ({ dispatch }, file) =>
axios
.get(file.context_lines_path, {
@@ -328,8 +428,10 @@ export const fetchFullDiff = ({ dispatch }, file) =>
from_merge_request: true,
},
})
- .then(({ data }) => dispatch('receiveFullDiffSucess', { filePath: file.file_path, data }))
- .then(() => scrollToElement(`#${file.file_hash}`))
+ .then(({ data }) => {
+ dispatch('receiveFullDiffSucess', { filePath: file.file_path });
+ dispatch('setExpandedDiffLines', { file, data });
+ })
.catch(() => dispatch('receiveFullDiffError', file.file_path));
export const toggleFullDiff = ({ dispatch, getters, state }, filePath) => {
@@ -340,7 +442,6 @@ export const toggleFullDiff = ({ dispatch, getters, state }, filePath) => {
if (file.isShowingFullFile) {
dispatch('loadCollapsedDiff', file)
.then(() => dispatch('assignDiscussionsToDiff', getters.getDiffFileDiscussions(file)))
- .then(() => scrollToElement(`#${file.file_hash}`))
.catch(() => dispatch('receiveFullDiffError', filePath));
} else {
dispatch('fetchFullDiff', file);
diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js
index adf56eba3f8..6bb24c97139 100644
--- a/app/assets/javascripts/diffs/store/mutation_types.js
+++ b/app/assets/javascripts/diffs/store/mutation_types.js
@@ -28,3 +28,8 @@ export const REQUEST_FULL_DIFF = 'REQUEST_FULL_DIFF';
export const RECEIVE_FULL_DIFF_SUCCESS = 'RECEIVE_FULL_DIFF_SUCCESS';
export const RECEIVE_FULL_DIFF_ERROR = 'RECEIVE_FULL_DIFF_ERROR';
export const SET_FILE_COLLAPSED = 'SET_FILE_COLLAPSED';
+
+export const SET_HIDDEN_VIEW_DIFF_FILE_LINES = 'SET_HIDDEN_VIEW_DIFF_FILE_LINES';
+export const SET_CURRENT_VIEW_DIFF_FILE_LINES = 'SET_CURRENT_VIEW_DIFF_FILE_LINES';
+export const ADD_CURRENT_VIEW_DIFF_FILE_LINES = 'ADD_CURRENT_VIEW_DIFF_FILE_LINES';
+export const TOGGLE_DIFF_FILE_RENDERING_MORE = 'TOGGLE_DIFF_FILE_RENDERING_MORE';
diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js
index 572fbfb5be4..67bc1724738 100644
--- a/app/assets/javascripts/diffs/store/mutations.js
+++ b/app/assets/javascripts/diffs/store/mutations.js
@@ -6,10 +6,8 @@ import {
addContextLines,
prepareDiffData,
isDiscussionApplicableToLine,
- convertExpandLines,
} from './utils';
import * as types from './mutation_types';
-import { OLD_LINE_KEY, NEW_LINE_KEY, TYPE_KEY, LEFT_LINE_KEY } from '../constants';
export default {
[types.SET_BASE_CONFIG](state, options) {
@@ -265,45 +263,11 @@ export default {
file.isLoadingFullFile = false;
},
- [types.RECEIVE_FULL_DIFF_SUCCESS](state, { filePath, data }) {
+ [types.RECEIVE_FULL_DIFF_SUCCESS](state, { filePath }) {
const file = findDiffFile(state.diffFiles, filePath, 'file_path');
file.isShowingFullFile = true;
file.isLoadingFullFile = false;
-
- file.highlighted_diff_lines = convertExpandLines({
- diffLines: file.highlighted_diff_lines,
- typeKey: [TYPE_KEY],
- oldLineKey: [OLD_LINE_KEY],
- newLineKey: [NEW_LINE_KEY],
- data,
- mapLine: ({ line, oldLine, newLine }) => ({
- ...line,
- old_line: oldLine,
- new_line: newLine,
- line_code: `${file.file_hash}_${oldLine}_${newLine}`,
- }),
- });
-
- file.parallel_diff_lines = convertExpandLines({
- diffLines: file.parallel_diff_lines,
- typeKey: [LEFT_LINE_KEY, TYPE_KEY],
- oldLineKey: [LEFT_LINE_KEY, OLD_LINE_KEY],
- newLineKey: [LEFT_LINE_KEY, NEW_LINE_KEY],
- data,
- mapLine: ({ line, oldLine, newLine }) => ({
- left: {
- ...line,
- old_line: oldLine,
- line_code: `${file.file_hash}_${oldLine}_${newLine}`,
- },
- right: {
- ...line,
- new_line: newLine,
- line_code: `${file.file_hash}_${newLine}_${oldLine}`,
- },
- }),
- });
},
[types.SET_FILE_COLLAPSED](state, { filePath, collapsed }) {
const file = state.diffFiles.find(f => f.file_path === filePath);
@@ -312,4 +276,30 @@ export default {
file.viewer.collapsed = collapsed;
}
},
+ [types.SET_HIDDEN_VIEW_DIFF_FILE_LINES](state, { filePath, lines }) {
+ const file = state.diffFiles.find(f => f.file_path === filePath);
+ const hiddenDiffLinesKey =
+ state.diffViewType === 'inline' ? 'parallel_diff_lines' : 'highlighted_diff_lines';
+
+ file[hiddenDiffLinesKey] = lines;
+ },
+ [types.SET_CURRENT_VIEW_DIFF_FILE_LINES](state, { filePath, lines }) {
+ const file = state.diffFiles.find(f => f.file_path === filePath);
+ const currentDiffLinesKey =
+ state.diffViewType === 'inline' ? 'highlighted_diff_lines' : 'parallel_diff_lines';
+
+ file[currentDiffLinesKey] = lines;
+ },
+ [types.ADD_CURRENT_VIEW_DIFF_FILE_LINES](state, { filePath, line }) {
+ const file = state.diffFiles.find(f => f.file_path === filePath);
+ const currentDiffLinesKey =
+ state.diffViewType === 'inline' ? 'highlighted_diff_lines' : 'parallel_diff_lines';
+
+ file[currentDiffLinesKey].push(line);
+ },
+ [types.TOGGLE_DIFF_FILE_RENDERING_MORE](state, filePath) {
+ const file = state.diffFiles.find(f => f.file_path === filePath);
+
+ file.renderingLines = !file.renderingLines;
+ },
};
diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js
index 27a79369a24..71956255eef 100644
--- a/app/assets/javascripts/diffs/store/utils.js
+++ b/app/assets/javascripts/diffs/store/utils.js
@@ -253,6 +253,7 @@ export function prepareDiffData(diffData) {
isShowingFullFile: false,
isLoadingFullFile: false,
discussions: [],
+ renderingLines: false,
});
}
}
@@ -423,27 +424,33 @@ export const convertExpandLines = ({
mapLine,
}) => {
const dataLength = data.length;
+ const lines = [];
+
+ for (let i = 0, diffLinesLength = diffLines.length; i < diffLinesLength; i += 1) {
+ const line = diffLines[i];
- return diffLines.reduce((acc, line, i) => {
if (_.property(typeKey)(line) === 'match') {
const beforeLine = diffLines[i - 1];
const afterLine = diffLines[i + 1];
- const beforeLineIndex = _.property(newLineKey)(beforeLine) || 0;
- const afterLineIndex = _.property(newLineKey)(afterLine) - 1 || dataLength;
-
- acc.push(
- ...data.slice(beforeLineIndex, afterLineIndex).map((l, index) => ({
- ...mapLine({
- line: { ...l, hasForm: false, discussions: [] },
+ const newLineProperty = _.property(newLineKey);
+ const beforeLineIndex = newLineProperty(beforeLine) || 0;
+ const afterLineIndex = newLineProperty(afterLine) - 1 || dataLength;
+
+ lines.push(
+ ...data.slice(beforeLineIndex, afterLineIndex).map((l, index) =>
+ mapLine({
+ line: Object.assign(l, { hasForm: false, discussions: [] }),
oldLine: (_.property(oldLineKey)(beforeLine) || 0) + index + 1,
- newLine: (_.property(newLineKey)(beforeLine) || 0) + index + 1,
+ newLine: (newLineProperty(beforeLine) || 0) + index + 1,
}),
- })),
+ ),
);
} else {
- acc.push(line);
+ lines.push(line);
}
+ }
- return acc;
- }, []);
+ return lines;
};
+
+export const idleCallback = cb => requestIdleCallback(cb);
diff --git a/changelogs/unreleased/expand-diff-performance.yml b/changelogs/unreleased/expand-diff-performance.yml
new file mode 100644
index 00000000000..134ea4081e4
--- /dev/null
+++ b/changelogs/unreleased/expand-diff-performance.yml
@@ -0,0 +1,5 @@
+---
+title: Improve expanding diff to full file performance
+merge_request:
+author:
+type: changed
diff --git a/spec/javascripts/diffs/components/diff_content_spec.js b/spec/javascripts/diffs/components/diff_content_spec.js
index bc9288e4150..124bdeea45d 100644
--- a/spec/javascripts/diffs/components/diff_content_spec.js
+++ b/spec/javascripts/diffs/components/diff_content_spec.js
@@ -29,6 +29,10 @@ describe('DiffContent', () => {
});
});
+ afterEach(() => {
+ vm.$destroy();
+ });
+
describe('text based files', () => {
it('should render diff inline view', done => {
vm.$store.state.diffs.diffViewType = 'inline';
@@ -49,6 +53,16 @@ describe('DiffContent', () => {
done();
});
});
+
+ it('renders rendering more lines loading icon', done => {
+ vm.diffFile.renderingLines = true;
+
+ vm.$nextTick(() => {
+ expect(vm.$el.querySelector('.loading-container')).not.toBe(null);
+
+ done();
+ });
+ });
});
describe('empty files', () => {
diff --git a/spec/javascripts/diffs/mock_data/diff_file.js b/spec/javascripts/diffs/mock_data/diff_file.js
index 32af9ea8ddd..27428197c1c 100644
--- a/spec/javascripts/diffs/mock_data/diff_file.js
+++ b/spec/javascripts/diffs/mock_data/diff_file.js
@@ -240,4 +240,5 @@ export default {
},
],
discussions: [],
+ renderingLines: false,
};
diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js
index bca99caa920..c82dcadd2f1 100644
--- a/spec/javascripts/diffs/store/actions_spec.js
+++ b/spec/javascripts/diffs/store/actions_spec.js
@@ -36,6 +36,7 @@ import actions, {
fetchFullDiff,
toggleFullDiff,
setFileCollapsed,
+ setExpandedDiffLines,
} from '~/diffs/store/actions';
import eventHub from '~/notes/event_hub';
import * as types from '~/diffs/store/mutation_types';
@@ -879,9 +880,9 @@ describe('DiffsStoreActions', () => {
it('commits REQUEST_FULL_DIFF', done => {
testAction(
receiveFullDiffSucess,
- { filePath: 'test', data: 'test' },
+ { filePath: 'test' },
{},
- [{ type: types.RECEIVE_FULL_DIFF_SUCCESS, payload: { filePath: 'test', data: 'test' } }],
+ [{ type: types.RECEIVE_FULL_DIFF_SUCCESS, payload: { filePath: 'test' } }],
[],
done,
);
@@ -903,11 +904,8 @@ describe('DiffsStoreActions', () => {
describe('fetchFullDiff', () => {
let mock;
- let scrollToElementSpy;
beforeEach(() => {
- scrollToElementSpy = spyOnDependency(actions, 'scrollToElement').and.stub();
-
mock = new MockAdapter(axios);
});
@@ -921,28 +919,23 @@ describe('DiffsStoreActions', () => {
});
it('dispatches receiveFullDiffSucess', done => {
+ const file = {
+ context_lines_path: `${gl.TEST_HOST}/context`,
+ file_path: 'test',
+ file_hash: 'test',
+ };
testAction(
fetchFullDiff,
- { context_lines_path: `${gl.TEST_HOST}/context`, file_path: 'test', file_hash: 'test' },
+ file,
null,
[],
- [{ type: 'receiveFullDiffSucess', payload: { filePath: 'test', data: ['test'] } }],
+ [
+ { type: 'receiveFullDiffSucess', payload: { filePath: 'test' } },
+ { type: 'setExpandedDiffLines', payload: { file, data: ['test'] } },
+ ],
done,
);
});
-
- it('scrolls to element', done => {
- fetchFullDiff(
- { dispatch() {} },
- { context_lines_path: `${gl.TEST_HOST}/context`, file_path: 'test', file_hash: 'test' },
- )
- .then(() => {
- expect(scrollToElementSpy).toHaveBeenCalledWith('#test');
-
- done();
- })
- .catch(done.fail);
- });
});
describe('error', () => {
@@ -999,4 +992,63 @@ describe('DiffsStoreActions', () => {
);
});
});
+
+ describe('setExpandedDiffLines', () => {
+ beforeEach(() => {
+ spyOnDependency(actions, 'idleCallback').and.callFake(cb => {
+ cb({ timeRemaining: () => 50 });
+ });
+ });
+
+ it('commits SET_CURRENT_VIEW_DIFF_FILE_LINES when lines less than MAX_RENDERING_DIFF_LINES', done => {
+ spyOnDependency(actions, 'convertExpandLines').and.callFake(() => ['test']);
+
+ testAction(
+ setExpandedDiffLines,
+ { file: { file_path: 'path' }, data: [] },
+ { diffViewType: 'inline' },
+ [
+ {
+ type: 'SET_HIDDEN_VIEW_DIFF_FILE_LINES',
+ payload: { filePath: 'path', lines: ['test'] },
+ },
+ {
+ type: 'SET_CURRENT_VIEW_DIFF_FILE_LINES',
+ payload: { filePath: 'path', lines: ['test'] },
+ },
+ ],
+ [],
+ done,
+ );
+ });
+
+ it('commits ADD_CURRENT_VIEW_DIFF_FILE_LINES when lines more than MAX_RENDERING_DIFF_LINES', done => {
+ const lines = new Array(501).fill().map((_, i) => `line-${i}`);
+ spyOnDependency(actions, 'convertExpandLines').and.callFake(() => lines);
+
+ testAction(
+ setExpandedDiffLines,
+ { file: { file_path: 'path' }, data: [] },
+ { diffViewType: 'inline' },
+ [
+ {
+ type: 'SET_HIDDEN_VIEW_DIFF_FILE_LINES',
+ payload: { filePath: 'path', lines },
+ },
+ {
+ type: 'SET_CURRENT_VIEW_DIFF_FILE_LINES',
+ payload: { filePath: 'path', lines: lines.slice(0, 200) },
+ },
+ { type: 'TOGGLE_DIFF_FILE_RENDERING_MORE', payload: 'path' },
+ ...new Array(301).fill().map((_, i) => ({
+ type: 'ADD_CURRENT_VIEW_DIFF_FILE_LINES',
+ payload: { filePath: 'path', line: `line-${i + 200}` },
+ })),
+ { type: 'TOGGLE_DIFF_FILE_RENDERING_MORE', payload: 'path' },
+ ],
+ [],
+ done,
+ );
+ });
+ });
});
diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js
index 5556a994a14..fa193e1d3b9 100644
--- a/spec/javascripts/diffs/store/mutations_spec.js
+++ b/spec/javascripts/diffs/store/mutations_spec.js
@@ -756,4 +756,98 @@ describe('DiffsStoreMutations', () => {
expect(state.diffFiles[0].viewer.collapsed).toBe(true);
});
});
+
+ describe('SET_HIDDEN_VIEW_DIFF_FILE_LINES', () => {
+ [
+ { current: 'highlighted', hidden: 'parallel', diffViewType: 'inline' },
+ { current: 'parallel', hidden: 'highlighted', diffViewType: 'parallel' },
+ ].forEach(({ current, hidden, diffViewType }) => {
+ it(`sets the ${hidden} lines when diff view is ${diffViewType}`, () => {
+ const file = { file_path: 'test', parallel_diff_lines: [], highlighted_diff_lines: [] };
+ const state = {
+ diffFiles: [file],
+ diffViewType,
+ };
+
+ mutations[types.SET_HIDDEN_VIEW_DIFF_FILE_LINES](state, {
+ filePath: 'test',
+ lines: ['test'],
+ });
+
+ expect(file[`${current}_diff_lines`]).toEqual([]);
+ expect(file[`${hidden}_diff_lines`]).toEqual(['test']);
+ });
+ });
+ });
+
+ describe('SET_CURRENT_VIEW_DIFF_FILE_LINES', () => {
+ [
+ { current: 'highlighted', hidden: 'parallel', diffViewType: 'inline' },
+ { current: 'parallel', hidden: 'highlighted', diffViewType: 'parallel' },
+ ].forEach(({ current, hidden, diffViewType }) => {
+ it(`sets the ${current} lines when diff view is ${diffViewType}`, () => {
+ const file = { file_path: 'test', parallel_diff_lines: [], highlighted_diff_lines: [] };
+ const state = {
+ diffFiles: [file],
+ diffViewType,
+ };
+
+ mutations[types.SET_CURRENT_VIEW_DIFF_FILE_LINES](state, {
+ filePath: 'test',
+ lines: ['test'],
+ });
+
+ expect(file[`${current}_diff_lines`]).toEqual(['test']);
+ expect(file[`${hidden}_diff_lines`]).toEqual([]);
+ });
+ });
+ });
+
+ describe('ADD_CURRENT_VIEW_DIFF_FILE_LINES', () => {
+ [
+ { current: 'highlighted', hidden: 'parallel', diffViewType: 'inline' },
+ { current: 'parallel', hidden: 'highlighted', diffViewType: 'parallel' },
+ ].forEach(({ current, hidden, diffViewType }) => {
+ it(`pushes to ${current} lines when diff view is ${diffViewType}`, () => {
+ const file = { file_path: 'test', parallel_diff_lines: [], highlighted_diff_lines: [] };
+ const state = {
+ diffFiles: [file],
+ diffViewType,
+ };
+
+ mutations[types.ADD_CURRENT_VIEW_DIFF_FILE_LINES](state, {
+ filePath: 'test',
+ line: 'test',
+ });
+
+ expect(file[`${current}_diff_lines`]).toEqual(['test']);
+ expect(file[`${hidden}_diff_lines`]).toEqual([]);
+
+ mutations[types.ADD_CURRENT_VIEW_DIFF_FILE_LINES](state, {
+ filePath: 'test',
+ line: 'test2',
+ });
+
+ expect(file[`${current}_diff_lines`]).toEqual(['test', 'test2']);
+ expect(file[`${hidden}_diff_lines`]).toEqual([]);
+ });
+ });
+ });
+
+ describe('TOGGLE_DIFF_FILE_RENDERING_MORE', () => {
+ it('toggles renderingLines on file', () => {
+ const file = { file_path: 'test', renderingLines: false };
+ const state = {
+ diffFiles: [file],
+ };
+
+ mutations[types.TOGGLE_DIFF_FILE_RENDERING_MORE](state, 'test');
+
+ expect(file.renderingLines).toBe(true);
+
+ mutations[types.TOGGLE_DIFF_FILE_RENDERING_MORE](state, 'test');
+
+ expect(file.renderingLines).toBe(false);
+ });
+ });
});