summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/assets/javascripts/diffs/components/app.vue12
-rw-r--r--app/assets/javascripts/diffs/components/diff_file.vue16
-rw-r--r--app/assets/javascripts/diffs/constants.js3
-rw-r--r--app/assets/javascripts/diffs/store/actions.js21
-rw-r--r--app/assets/javascripts/diffs/store/mutation_types.js1
-rw-r--r--app/assets/javascripts/diffs/store/mutations.js43
-rw-r--r--changelogs/unreleased/tz-mr-incremental-rendering.yml4
-rw-r--r--spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb2
-rw-r--r--spec/javascripts/diffs/components/diff_file_spec.js10
-rw-r--r--spec/javascripts/diffs/mock_data/diff_file.js1
-rw-r--r--spec/javascripts/diffs/store/mutations_spec.js18
11 files changed, 121 insertions, 10 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue
index 7cc4e6a2c3a..b5b05df4d34 100644
--- a/app/assets/javascripts/diffs/components/app.vue
+++ b/app/assets/javascripts/diffs/components/app.vue
@@ -114,11 +114,15 @@ export default {
this.adjustView();
},
methods: {
- ...mapActions('diffs', ['setBaseConfig', 'fetchDiffFiles']),
+ ...mapActions('diffs', ['setBaseConfig', 'fetchDiffFiles', 'startRenderDiffsQueue']),
fetchData() {
- this.fetchDiffFiles().catch(() => {
- createFlash(__('Something went wrong on our end. Please try again!'));
- });
+ this.fetchDiffFiles()
+ .then(() => {
+ requestIdleCallback(this.startRenderDiffsQueue, { timeout: 1000 });
+ })
+ .catch(() => {
+ createFlash(__('Something went wrong on our end. Please try again!'));
+ });
if (!this.isNotesFetched) {
eventHub.$emit('fetchNotesData');
diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue
index 7e7058d8d08..59e9ba08b8b 100644
--- a/app/assets/javascripts/diffs/components/diff_file.vue
+++ b/app/assets/javascripts/diffs/components/diff_file.vue
@@ -46,16 +46,25 @@ export default {
showExpandMessage() {
return this.isCollapsed && !this.isLoadingCollapsedDiff && !this.file.tooLarge;
},
+ showLoadingIcon() {
+ return this.isLoadingCollapsedDiff || (!this.file.renderIt && !this.isCollapsed);
+ },
},
methods: {
...mapActions('diffs', ['loadCollapsedDiff']),
handleToggle() {
const { collapsed, highlightedDiffLines, parallelDiffLines } = this.file;
- if (collapsed && !highlightedDiffLines && !parallelDiffLines.length) {
+ if (
+ collapsed &&
+ !highlightedDiffLines &&
+ parallelDiffLines !== undefined &&
+ !parallelDiffLines.length
+ ) {
this.handleLoadCollapsedDiff();
} else {
this.file.collapsed = !this.file.collapsed;
+ this.file.renderIt = true;
}
},
handleLoadCollapsedDiff() {
@@ -65,6 +74,7 @@ export default {
.then(() => {
this.isLoadingCollapsedDiff = false;
this.file.collapsed = false;
+ this.file.renderIt = true;
})
.catch(() => {
this.isLoadingCollapsedDiff = false;
@@ -121,12 +131,12 @@ export default {
</div>
<diff-content
- v-if="!isCollapsed"
+ v-if="!isCollapsed && file.renderIt"
:class="{ hidden: isCollapsed || file.tooLarge }"
:diff-file="file"
/>
<loading-icon
- v-if="isLoadingCollapsedDiff"
+ v-else-if="showLoadingIcon"
class="diff-content loading"
/>
<div
diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js
index 2fa8367f528..f68afa44837 100644
--- a/app/assets/javascripts/diffs/constants.js
+++ b/app/assets/javascripts/diffs/constants.js
@@ -25,3 +25,6 @@ export const CONTEXT_LINE_CLASS_NAME = 'diff-expanded';
export const UNFOLD_COUNT = 20;
export const COUNT_OF_AVATARS_IN_GUTTER = 3;
export const LENGTH_OF_AVATAR_TOOLTIP = 17;
+
+export const LINES_TO_BE_RENDERED_DIRECTLY = 100;
+export const MAX_LINES_TO_BE_RENDERED = 2000;
diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js
index 27001142257..4ab6ceb249a 100644
--- a/app/assets/javascripts/diffs/store/actions.js
+++ b/app/assets/javascripts/diffs/store/actions.js
@@ -29,6 +29,27 @@ export const fetchDiffFiles = ({ state, commit }) => {
.then(handleLocationHash);
};
+export const startRenderDiffsQueue = ({ state, commit }) => {
+ const checkItem = () => {
+ const nextFile = state.diffFiles.find(
+ file => !file.renderIt && (!file.collapsed || !file.text),
+ );
+ if (nextFile) {
+ requestAnimationFrame(() => {
+ commit(types.RENDER_FILE, nextFile);
+ });
+ requestIdleCallback(
+ () => {
+ checkItem();
+ },
+ { timeout: 1000 },
+ );
+ }
+ };
+
+ checkItem();
+};
+
export const setInlineDiffViewType = ({ commit }) => {
commit(types.SET_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE);
diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js
index 2c8e1a1466f..c999d637d50 100644
--- a/app/assets/javascripts/diffs/store/mutation_types.js
+++ b/app/assets/javascripts/diffs/store/mutation_types.js
@@ -8,3 +8,4 @@ export const REMOVE_COMMENT_FORM_LINE = 'REMOVE_COMMENT_FORM_LINE';
export const ADD_CONTEXT_LINES = 'ADD_CONTEXT_LINES';
export const ADD_COLLAPSED_DIFFS = 'ADD_COLLAPSED_DIFFS';
export const EXPAND_ALL_FILES = 'EXPAND_ALL_FILES';
+export const RENDER_FILE = 'RENDER_FILE';
diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js
index a98b2be89a3..0522e32c410 100644
--- a/app/assets/javascripts/diffs/store/mutations.js
+++ b/app/assets/javascripts/diffs/store/mutations.js
@@ -2,6 +2,7 @@ import Vue from 'vue';
import _ from 'underscore';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import { findDiffFile, addLineReferences, removeMatchLine, addContextLines } from './utils';
+import { LINES_TO_BE_RENDERED_DIRECTLY, MAX_LINES_TO_BE_RENDERED } from '../constants';
import * as types from './mutation_types';
export default {
@@ -15,8 +16,48 @@ export default {
},
[types.SET_DIFF_DATA](state, data) {
+ const diffData = convertObjectPropsToCamelCase(data, { deep: true });
+ let showingLines = 0;
+ const filesLength = diffData.diffFiles.length;
+ let i;
+ for (i = 0; i < filesLength; i += 1) {
+ const file = diffData.diffFiles[i];
+ if (file.parallelDiffLines) {
+ const linesLength = file.parallelDiffLines.length;
+ let u = 0;
+ for (u = 0; u < linesLength; u += 1) {
+ const line = file.parallelDiffLines[u];
+ if (line.left) delete line.left.text;
+ if (line.right) delete line.right.text;
+ }
+ }
+
+ if (file.highlightedDiffLines) {
+ const linesLength = file.highlightedDiffLines.length;
+ let u;
+ for (u = 0; u < linesLength; u += 1) {
+ const line = file.highlightedDiffLines[u];
+ delete line.text;
+ }
+ }
+
+ if (file.highlightedDiffLines) {
+ showingLines += file.parallelDiffLines.length;
+ }
+ Object.assign(file, {
+ renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY,
+ collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED,
+ });
+ }
+
Object.assign(state, {
- ...convertObjectPropsToCamelCase(data, { deep: true }),
+ ...diffData,
+ });
+ },
+
+ [types.RENDER_FILE](state, file) {
+ Object.assign(file, {
+ renderIt: true,
});
},
diff --git a/changelogs/unreleased/tz-mr-incremental-rendering.yml b/changelogs/unreleased/tz-mr-incremental-rendering.yml
new file mode 100644
index 00000000000..a35fa200363
--- /dev/null
+++ b/changelogs/unreleased/tz-mr-incremental-rendering.yml
@@ -0,0 +1,4 @@
+title: Incremental rendering with Vue on merge request page
+merge_request: 21063
+author:
+type: performance
diff --git a/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb b/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb
index c1608be402a..fd4175d5227 100644
--- a/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb
+++ b/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb
@@ -28,7 +28,7 @@ describe 'Merge request > User sees MR with deleted source branch', :js do
click_on 'Changes'
wait_for_requests
- expect(page).to have_selector('.diffs.tab-pane .nothing-here-block')
+ expect(page).to have_selector('.diffs.tab-pane .file-holder')
expect(page).to have_content('Source branch does not exist.')
end
end
diff --git a/spec/javascripts/diffs/components/diff_file_spec.js b/spec/javascripts/diffs/components/diff_file_spec.js
index 7a4616ec8eb..44a38f7ca82 100644
--- a/spec/javascripts/diffs/components/diff_file_spec.js
+++ b/spec/javascripts/diffs/components/diff_file_spec.js
@@ -22,11 +22,18 @@ describe('DiffFile', () => {
expect(el.id).toEqual(fileHash);
expect(el.classList.contains('diff-file')).toEqual(true);
+
expect(el.querySelectorAll('.diff-content.hidden').length).toEqual(0);
expect(el.querySelector('.js-file-title')).toBeDefined();
expect(el.querySelector('.file-title-name').innerText.indexOf(filePath) > -1).toEqual(true);
expect(el.querySelector('.js-syntax-highlight')).toBeDefined();
- expect(el.querySelectorAll('.line_content').length > 5).toEqual(true);
+
+ expect(vm.file.renderIt).toEqual(false);
+ vm.file.renderIt = true;
+
+ vm.$nextTick(() => {
+ expect(el.querySelectorAll('.line_content').length > 5).toEqual(true);
+ });
});
describe('collapsed', () => {
@@ -34,6 +41,7 @@ describe('DiffFile', () => {
expect(vm.$el.querySelectorAll('.diff-content').length).toEqual(1);
expect(vm.file.collapsed).toEqual(false);
vm.file.collapsed = true;
+ vm.file.renderIt = true;
vm.$nextTick(() => {
expect(vm.$el.querySelectorAll('.diff-content').length).toEqual(0);
diff --git a/spec/javascripts/diffs/mock_data/diff_file.js b/spec/javascripts/diffs/mock_data/diff_file.js
index d3bf9525924..cce36ecc91f 100644
--- a/spec/javascripts/diffs/mock_data/diff_file.js
+++ b/spec/javascripts/diffs/mock_data/diff_file.js
@@ -39,6 +39,7 @@ export default {
viewPath: '/gitlab-org/gitlab-test/blob/spooky-stuff/CHANGELOG',
replacedViewPath: null,
collapsed: false,
+ renderIt: false,
tooLarge: false,
contextLinesPath:
'/gitlab-org/gitlab-test/blob/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG/diff',
diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js
index 1af49f4985c..8f89984c6e5 100644
--- a/spec/javascripts/diffs/store/mutations_spec.js
+++ b/spec/javascripts/diffs/store/mutations_spec.js
@@ -1,6 +1,7 @@
import mutations from '~/diffs/store/mutations';
import * as types from '~/diffs/store/mutation_types';
import { INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants';
+import diffFileMockData from '../mock_data/diff_file';
describe('DiffsStoreMutations', () => {
describe('SET_BASE_CONFIG', () => {
@@ -24,6 +25,23 @@ describe('DiffsStoreMutations', () => {
});
});
+ describe('SET_DIFF_DATA', () => {
+ it('should set diff data type properly', () => {
+ const state = {};
+ const diffMock = {
+ diff_files: [diffFileMockData],
+ };
+
+ mutations[types.SET_DIFF_DATA](state, diffMock);
+
+ const firstLine = state.diffFiles[0].parallelDiffLines[0];
+
+ expect(firstLine.right.text).toBeUndefined();
+ expect(state.diffFiles[0].renderIt).toEqual(true);
+ expect(state.diffFiles[0].collapsed).toEqual(false);
+ });
+ });
+
describe('SET_DIFF_VIEW_TYPE', () => {
it('should set diff view type properly', () => {
const state = {};