From 83e2cb434c4f0a36c241e7fd06e19189636e0268 Mon Sep 17 00:00:00 2001 From: Tim Zallmann Date: Fri, 20 Jul 2018 15:00:19 +0200 Subject: Reducing the memory footpring for the rendering --- .../diffs/components/diff_line_gutter_content.vue | 19 +++++----- .../diffs/components/diff_table_cell.vue | 6 +++ .../diffs/components/inline_diff_comment_row.vue | 9 +++-- .../diffs/components/inline_diff_table_row.vue | 7 ++++ .../diffs/components/inline_diff_view.vue | 4 +- .../diffs/components/parallel_diff_comment_row.vue | 43 +++++++++++----------- .../diffs/components/parallel_diff_table_row.vue | 12 ++++++ .../diffs/components/parallel_diff_view.vue | 6 ++- app/assets/javascripts/diffs/store/getters.js | 6 +++ 9 files changed, 74 insertions(+), 38 deletions(-) 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 0fe0007057b..d184a76f038 100644 --- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue +++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue @@ -71,6 +71,11 @@ export default { required: false, default: false, }, + discussions: { + type: Array, + required: false, + default: () => [], + }, }, computed: { ...mapState({ @@ -78,7 +83,6 @@ export default { diffFiles: state => state.diffs.diffFiles, }), ...mapGetters(['isLoggedIn']), - ...mapGetters('diffs', ['discussionsByLineCode']), lineHref() { return this.lineCode ? `#${this.lineCode}` : '#'; }, @@ -88,24 +92,19 @@ export default { this.showCommentButton && !this.isMatchLine && !this.isContextLine && - !this.hasDiscussions && - !this.isMetaLine + !this.isMetaLine && + !this.hasDiscussions ); }, - discussions() { - return this.discussionsByLineCode[this.lineCode] || []; - }, hasDiscussions() { return this.discussions.length > 0; }, shouldShowAvatarsOnGutter() { - let render = this.hasDiscussions && this.showCommentButton; - if (!this.lineType && this.linePosition === LINE_POSITION_RIGHT) { - render = false; + return false; } - return render; + return this.hasDiscussions && this.showCommentButton; }, }, methods: { diff --git a/app/assets/javascripts/diffs/components/diff_table_cell.vue b/app/assets/javascripts/diffs/components/diff_table_cell.vue index 5962f30d9bb..e8e8ddc6c5e 100644 --- a/app/assets/javascripts/diffs/components/diff_table_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_table_cell.vue @@ -67,6 +67,11 @@ export default { required: false, default: false, }, + discussions: { + type: Array, + required: false, + default: () => [], + }, }, computed: { ...mapGetters(['isLoggedIn']), @@ -136,6 +141,7 @@ export default { :is-match-line="isMatchLine" :is-context-line="isContentLine" :is-meta-line="isMetaLine" + :discussions="discussions" /> diff --git a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue index a6f011ff31e..373ed055d58 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue @@ -21,15 +21,16 @@ export default { type: Number, required: true, }, + discussions: { + type: Array, + required: false, + default: () => [], + }, }, computed: { ...mapState({ diffLineCommentForms: state => state.diffs.diffLineCommentForms, }), - ...mapGetters('diffs', ['discussionsByLineCode']), - discussions() { - return this.discussionsByLineCode[this.line.lineCode] || []; - }, className() { return this.discussions.length ? '' : 'js-temp-notes-holder'; }, 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 0e306f39a9f..32d65ff994f 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue @@ -33,6 +33,11 @@ export default { required: false, default: false, }, + discussions: { + type: Array, + required: false, + default: () => [], + }, }, data() { return { @@ -89,6 +94,7 @@ export default { :is-bottom="isBottom" :is-hover="isHover" :show-comment-button="true" + :discussions="discussions" class="diff-line-num old_line" /> state.diffs.diffLineCommentForms, }), @@ -64,6 +64,7 @@ export default { :line="line" :is-bottom="index + 1 === diffLinesLength" :key="line.lineCode" + :discussions="singleDiscussionByLineCode(line.lineCode)" /> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue index 05e5cafc717..90cdbb2f8d8 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue @@ -21,12 +21,21 @@ export default { type: Number, required: true, }, + leftDiscussions: { + type: Array, + required: false, + default: () => [], + }, + rightDiscussions: { + type: Array, + required: false, + default: () => [], + }, }, computed: { ...mapState({ diffLineCommentForms: state => state.diffs.diffLineCommentForms, }), - ...mapGetters('diffs', ['discussionsByLineCode']), leftLineCode() { return this.line.left.lineCode; }, @@ -34,32 +43,24 @@ export default { return this.line.right.lineCode; }, hasDiscussion() { - const discussions = this.discussionsByLineCode; - - return discussions[this.leftLineCode] || discussions[this.rightLineCode]; + return this.leftDiscussions || this.rightDiscussions; }, hasExpandedDiscussionOnLeft() { - const discussions = this.discussionsByLineCode[this.leftLineCode]; - + const discussions = this.leftDiscussions; return discussions ? discussions.every(discussion => discussion.expanded) : false; }, hasExpandedDiscussionOnRight() { - const discussions = this.discussionsByLineCode[this.rightLineCode]; - + const discussions = this.rightDiscussions; return discussions ? discussions.every(discussion => discussion.expanded) : false; }, hasAnyExpandedDiscussion() { return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight; }, shouldRenderDiscussionsOnLeft() { - return this.discussionsByLineCode[this.leftLineCode] && this.hasExpandedDiscussionOnLeft; + return this.leftDiscussions && this.hasExpandedDiscussionOnLeft; }, shouldRenderDiscussionsOnRight() { - return ( - this.discussionsByLineCode[this.rightLineCode] && - this.hasExpandedDiscussionOnRight && - this.line.right.type - ); + return this.rightDiscussions && this.hasExpandedDiscussionOnRight && this.line.right.type; }, className() { return this.hasDiscussion ? '' : 'js-temp-notes-holder'; @@ -80,13 +81,12 @@ export default { class="content" > [], + }, + rightDiscussions: { + type: Array, + required: false, + default: () => [], + }, }, data() { return { @@ -116,6 +126,7 @@ export default { :is-hover="isLeftHover" :show-comment-button="true" :diff-view-type="parallelDiffViewType" + :discussions="leftDiscussions" class="diff-line-num old_line" /> state.diffs.diffLineCommentForms, }), @@ -97,6 +97,8 @@ export default { :line="line" :is-bottom="index + 1 === diffLinesLength" :key="index" + :left-discussions="singleDiscussionByLineCode(line.left.lineCode)" + :right-discussions="singleDiscussionByLineCode(line.right.lineCode)" /> diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index d3881fa1a0a..ff6c30d6a8e 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -96,6 +96,12 @@ export const discussionsByLineCode = (state, getters, rootState, rootGetters) => }, {}); }; +export const singleDiscussionByLineCode = (state, getters) => lineCode => { + if (!lineCode) return []; + const discussions = getters.discussionsByLineCode; + return discussions[lineCode] || []; +}; + // prevent babel-plugin-rewire from generating an invalid default during karma∂ tests export const getDiffFileByHash = state => fileHash => state.diffFiles.find(file => file.fileHash === fileHash); -- cgit v1.2.1