Commit 9eef6cd1 authored by Tim Zallmann's avatar Tim Zallmann Committed by Filipa Lacerda

Resolve "MR refactor: Performance costs of main element with `v-if`"

parent 8dfa7a42
...@@ -15,9 +15,7 @@ export default { ...@@ -15,9 +15,7 @@ export default {
</script> </script>
<template> <template>
<div <div>
v-if="discussions.length"
>
<div <div
v-for="discussion in discussions" v-for="discussion in discussions"
:key="discussion.id" :key="discussion.id"
......
...@@ -47,6 +47,9 @@ export default { ...@@ -47,6 +47,9 @@ export default {
false, false,
); );
}, },
showExpandMessage() {
return this.isCollapsed && !this.isLoadingCollapsedDiff && !this.file.tooLarge;
},
}, },
mounted() { mounted() {
document.addEventListener('scroll', this.handleScroll); document.addEventListener('scroll', this.handleScroll);
...@@ -159,7 +162,7 @@ export default { ...@@ -159,7 +162,7 @@ export default {
</div> </div>
<diff-content <diff-content
v-show="!isCollapsed" v-if="!isCollapsed"
:class="{ hidden: isCollapsed || file.tooLarge }" :class="{ hidden: isCollapsed || file.tooLarge }"
:diff-file="file" :diff-file="file"
/> />
...@@ -168,7 +171,7 @@ export default { ...@@ -168,7 +171,7 @@ export default {
class="diff-content loading" class="diff-content loading"
/> />
<div <div
v-show="isCollapsed && !isLoadingCollapsedDiff && !file.tooLarge" v-if="showExpandMessage"
class="nothing-here-block diff-collapsed" class="nothing-here-block diff-collapsed"
> >
{{ __('This diff is collapsed.') }} {{ __('This diff is collapsed.') }}
......
...@@ -31,22 +31,9 @@ export default { ...@@ -31,22 +31,9 @@ export default {
diffLineCommentForms: state => state.diffs.diffLineCommentForms, diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}), }),
...mapGetters(['discussionsByLineCode']), ...mapGetters(['discussionsByLineCode']),
isDiscussionExpanded() {
if (!this.discussions.length) {
return false;
}
return this.discussions.every(discussion => discussion.expanded);
},
hasCommentForm() {
return this.diffLineCommentForms[this.line.lineCode];
},
discussions() { discussions() {
return this.discussionsByLineCode[this.line.lineCode] || []; return this.discussionsByLineCode[this.line.lineCode] || [];
}, },
shouldRender() {
return this.isDiscussionExpanded || this.hasCommentForm;
},
className() { className() {
return this.discussions.length ? '' : 'js-temp-notes-holder'; return this.discussions.length ? '' : 'js-temp-notes-holder';
}, },
...@@ -56,7 +43,6 @@ export default { ...@@ -56,7 +43,6 @@ export default {
<template> <template>
<tr <tr
v-if="shouldRender"
:class="className" :class="className"
class="notes_holder" class="notes_holder"
> >
...@@ -67,6 +53,7 @@ export default { ...@@ -67,6 +53,7 @@ export default {
<td class="notes_content"> <td class="notes_content">
<div class="content"> <div class="content">
<diff-discussions <diff-discussions
v-if="discussions.length"
:discussions="discussions" :discussions="discussions"
/> />
<diff-line-note-form <diff-line-note-form
......
<script> <script>
import { mapGetters } from 'vuex'; import { mapGetters, mapState } from 'vuex';
import inlineDiffTableRow from './inline_diff_table_row.vue'; import inlineDiffTableRow from './inline_diff_table_row.vue';
import inlineDiffCommentRow from './inline_diff_comment_row.vue'; import inlineDiffCommentRow from './inline_diff_comment_row.vue';
import { trimFirstCharOfLineContent } from '../store/utils'; import { trimFirstCharOfLineContent } from '../store/utils';
...@@ -20,7 +20,10 @@ export default { ...@@ -20,7 +20,10 @@ export default {
}, },
}, },
computed: { computed: {
...mapGetters(['commitId']), ...mapGetters(['commitId', 'discussionsByLineCode']),
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
normalizedDiffLines() { normalizedDiffLines() {
return this.diffLines.map(line => (line.richText ? trimFirstCharOfLineContent(line) : line)); return this.diffLines.map(line => (line.richText ? trimFirstCharOfLineContent(line) : line));
}, },
...@@ -31,6 +34,18 @@ export default { ...@@ -31,6 +34,18 @@ export default {
return window.gon.user_color_scheme; return window.gon.user_color_scheme;
}, },
}, },
methods: {
shouldRenderCommentRow(line) {
if (this.diffLineCommentForms[line.lineCode]) return true;
const lineDiscussions = this.discussionsByLineCode[line.lineCode];
if (lineDiscussions === undefined) {
return false;
}
return lineDiscussions.every(discussion => discussion.expanded);
},
},
}; };
</script> </script>
...@@ -50,6 +65,7 @@ export default { ...@@ -50,6 +65,7 @@ export default {
:key="line.lineCode" :key="line.lineCode"
/> />
<inline-diff-comment-row <inline-diff-comment-row
v-if="shouldRenderCommentRow(line)"
:diff-file="diffFile" :diff-file="diffFile"
:diff-lines="normalizedDiffLines" :diff-lines="normalizedDiffLines"
:line="line" :line="line"
......
...@@ -55,13 +55,6 @@ export default { ...@@ -55,13 +55,6 @@ export default {
hasAnyExpandedDiscussion() { hasAnyExpandedDiscussion() {
return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight; return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight;
}, },
shouldRenderDiscussionsRow() {
const hasDiscussion = this.hasDiscussion && this.hasAnyExpandedDiscussion;
const hasCommentFormOnLeft = this.diffLineCommentForms[this.leftLineCode];
const hasCommentFormOnRight = this.diffLineCommentForms[this.rightLineCode];
return hasDiscussion || hasCommentFormOnLeft || hasCommentFormOnRight;
},
shouldRenderDiscussionsOnLeft() { shouldRenderDiscussionsOnLeft() {
return this.discussionsByLineCode[this.leftLineCode] && this.hasExpandedDiscussionOnLeft; return this.discussionsByLineCode[this.leftLineCode] && this.hasExpandedDiscussionOnLeft;
}, },
...@@ -81,7 +74,6 @@ export default { ...@@ -81,7 +74,6 @@ export default {
<template> <template>
<tr <tr
v-if="shouldRenderDiscussionsRow"
:class="className" :class="className"
class="notes_holder" class="notes_holder"
> >
...@@ -92,6 +84,7 @@ export default { ...@@ -92,6 +84,7 @@ export default {
class="content" class="content"
> >
<diff-discussions <diff-discussions
v-if="discussionsByLineCode[leftLineCode].length"
:discussions="discussionsByLineCode[leftLineCode]" :discussions="discussionsByLineCode[leftLineCode]"
/> />
</div> </div>
...@@ -112,6 +105,7 @@ export default { ...@@ -112,6 +105,7 @@ export default {
class="content" class="content"
> >
<diff-discussions <diff-discussions
v-if="discussionsByLineCode[rightLineCode].length"
:discussions="discussionsByLineCode[rightLineCode]" :discussions="discussionsByLineCode[rightLineCode]"
/> />
</div> </div>
......
<script> <script>
import { mapGetters } from 'vuex'; import { mapState, mapGetters } from 'vuex';
import parallelDiffTableRow from './parallel_diff_table_row.vue'; import parallelDiffTableRow from './parallel_diff_table_row.vue';
import parallelDiffCommentRow from './parallel_diff_comment_row.vue'; import parallelDiffCommentRow from './parallel_diff_comment_row.vue';
import { EMPTY_CELL_TYPE } from '../constants'; import { EMPTY_CELL_TYPE } from '../constants';
...@@ -21,7 +21,10 @@ export default { ...@@ -21,7 +21,10 @@ export default {
}, },
}, },
computed: { computed: {
...mapGetters(['commitId']), ...mapGetters(['commitId', 'discussionsByLineCode']),
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
parallelDiffLines() { parallelDiffLines() {
return this.diffLines.map(line => { return this.diffLines.map(line => {
const parallelLine = Object.assign({}, line); const parallelLine = Object.assign({}, line);
...@@ -48,6 +51,32 @@ export default { ...@@ -48,6 +51,32 @@ export default {
return window.gon.user_color_scheme; return window.gon.user_color_scheme;
}, },
}, },
methods: {
shouldRenderCommentRow(line) {
const leftLineCode = line.left.lineCode;
const rightLineCode = line.right.lineCode;
const discussions = this.discussionsByLineCode;
const leftDiscussions = discussions[leftLineCode];
const rightDiscussions = discussions[rightLineCode];
const hasDiscussion = leftDiscussions || rightDiscussions;
const hasExpandedDiscussionOnLeft = leftDiscussions
? leftDiscussions.every(discussion => discussion.expanded)
: false;
const hasExpandedDiscussionOnRight = rightDiscussions
? rightDiscussions.every(discussion => discussion.expanded)
: false;
if (hasDiscussion && (hasExpandedDiscussionOnLeft || hasExpandedDiscussionOnRight)) {
return true;
}
const hasCommentFormOnLeft = this.diffLineCommentForms[leftLineCode];
const hasCommentFormOnRight = this.diffLineCommentForms[rightLineCode];
return hasCommentFormOnLeft || hasCommentFormOnRight;
},
},
}; };
</script> </script>
...@@ -69,6 +98,7 @@ export default { ...@@ -69,6 +98,7 @@ export default {
:key="index" :key="index"
/> />
<parallel-diff-comment-row <parallel-diff-comment-row
v-if="shouldRenderCommentRow(line)"
:key="line.left.lineCode || line.right.lineCode" :key="line.left.lineCode || line.right.lineCode"
:line="line" :line="line"
:diff-file="diffFile" :diff-file="diffFile"
......
...@@ -31,12 +31,12 @@ describe('DiffFile', () => { ...@@ -31,12 +31,12 @@ describe('DiffFile', () => {
describe('collapsed', () => { describe('collapsed', () => {
it('should not have file content', done => { it('should not have file content', done => {
expect(vm.$el.querySelectorAll('.diff-content.hidden').length).toEqual(0); expect(vm.$el.querySelectorAll('.diff-content').length).toEqual(1);
expect(vm.file.collapsed).toEqual(false); expect(vm.file.collapsed).toEqual(false);
vm.file.collapsed = true; vm.file.collapsed = true;
vm.$nextTick(() => { vm.$nextTick(() => {
expect(vm.$el.querySelectorAll('.diff-content.hidden').length).toEqual(1); expect(vm.$el.querySelectorAll('.diff-content').length).toEqual(0);
done(); done();
}); });
......
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