diff options
69 files changed, 2686 insertions, 475 deletions
diff --git a/app/assets/javascripts/autosave.js b/app/assets/javascripts/autosave.js index 0da872db7e5..3153f5156db 100644 --- a/app/assets/javascripts/autosave.js +++ b/app/assets/javascripts/autosave.js @@ -31,7 +31,9 @@ export default class Autosave { // https://github.com/vuejs/vue/issues/2804#issuecomment-216968137 const event = new Event('change', { bubbles: true, cancelable: false }); const field = this.field.get(0); - field.dispatchEvent(event); + if (field) { + field.dispatchEvent(event); + } } save() { diff --git a/app/assets/javascripts/awards_handler.js b/app/assets/javascripts/awards_handler.js index 6da33a26e58..06e2ac66152 100644 --- a/app/assets/javascripts/awards_handler.js +++ b/app/assets/javascripts/awards_handler.js @@ -4,7 +4,7 @@ import $ from 'jquery'; import _ from 'underscore'; import Cookies from 'js-cookie'; import { __ } from './locale'; -import { isInIssuePage, isInMRPage, hasVueMRDiscussionsCookie, updateTooltipTitle } from './lib/utils/common_utils'; +import { isInIssuePage, isInMRPage, updateTooltipTitle } from './lib/utils/common_utils'; import flash from './flash'; import axios from './lib/utils/axios_utils'; @@ -295,12 +295,8 @@ class AwardsHandler { } } - isVueMRDiscussions() { - return isInMRPage() && hasVueMRDiscussionsCookie() && !$('#diffs').is(':visible'); - } - isInVueNoteablePage() { - return isInIssuePage() || this.isVueMRDiscussions(); + return isInIssuePage() || isInMRPage(); } getVotesBlock() { diff --git a/app/assets/javascripts/diff_notes/components/resolve_btn.js b/app/assets/javascripts/diff_notes/components/resolve_btn.js index df4c72ba0ed..08e3a6302cf 100644 --- a/app/assets/javascripts/diff_notes/components/resolve_btn.js +++ b/app/assets/javascripts/diff_notes/components/resolve_btn.js @@ -1,4 +1,3 @@ -/* eslint-disable comma-dangle, object-shorthand, func-names, quote-props, no-else-return, camelcase, max-len */ /* global CommentsStore */ /* global ResolveService */ @@ -17,26 +16,26 @@ const ResolveBtn = Vue.extend({ authorAvatar: String, noteTruncated: String, }, - data: function () { + data: function() { return { discussions: CommentsStore.state, - loading: false + loading: false, }; }, watch: { - 'discussions': { + discussions: { handler: 'updateTooltip', - deep: true - } + deep: true, + }, }, computed: { - discussion: function () { + discussion: function() { return this.discussions[this.discussionId]; }, - note: function () { + note: function() { return this.discussion ? this.discussion.getNote(this.noteId) : {}; }, - buttonText: function () { + buttonText: function() { if (this.isResolved) { return `Resolved by ${this.resolvedByName}`; } else if (this.canResolve) { @@ -45,65 +44,71 @@ const ResolveBtn = Vue.extend({ return 'Unable to resolve'; } }, - isResolved: function () { + isResolved: function() { if (this.note) { return this.note.resolved; } else { return false; } }, - resolvedByName: function () { + resolvedByName: function() { return this.note.resolved_by; }, }, methods: { - updateTooltip: function () { + updateTooltip: function() { this.$nextTick(() => { $(this.$refs.button) .tooltip('hide') .tooltip('fixTitle'); }); }, - resolve: function () { + resolve: function() { if (!this.canResolve) return; let promise; this.loading = true; if (this.isResolved) { - promise = ResolveService - .unresolve(this.noteId); + promise = ResolveService.unresolve(this.noteId); } else { - promise = ResolveService - .resolve(this.noteId); + promise = ResolveService.resolve(this.noteId); } promise .then(resp => resp.json()) - .then((data) => { + .then(data => { this.loading = false; const resolved_by = data ? data.resolved_by : null; - CommentsStore.update(this.discussionId, this.noteId, !this.isResolved, resolved_by); + CommentsStore.update( + this.discussionId, + this.noteId, + !this.isResolved, + resolved_by, + ); this.discussion.updateHeadline(data); gl.mrWidget.checkStatus(); - document.dispatchEvent(new CustomEvent('refreshVueNotes')); - this.updateTooltip(); }) - .catch(() => new Flash('An error occurred when trying to resolve a comment. Please try again.')); - } + .catch( + () => + new Flash( + 'An error occurred when trying to resolve a comment. Please try again.', + ), + ); + }, }, - mounted: function () { + mounted: function() { $(this.$refs.button).tooltip({ - container: 'body' + container: 'body', }); }, - beforeDestroy: function () { + beforeDestroy: function() { CommentsStore.delete(this.discussionId, this.noteId); }, - created: function () { + created: function() { CommentsStore.create({ discussionId: this.discussionId, noteId: this.noteId, @@ -114,7 +119,7 @@ const ResolveBtn = Vue.extend({ authorAvatar: this.authorAvatar, noteTruncated: this.noteTruncated, }); - } + }, }); Vue.component('resolve-btn', ResolveBtn); diff --git a/app/assets/javascripts/diff_notes/diff_notes_bundle.js b/app/assets/javascripts/diff_notes/diff_notes_bundle.js index e17daec6a92..406091820e9 100644 --- a/app/assets/javascripts/diff_notes/diff_notes_bundle.js +++ b/app/assets/javascripts/diff_notes/diff_notes_bundle.js @@ -15,12 +15,14 @@ import './components/resolve_count'; import './components/resolve_discussion_btn'; import './components/diff_note_avatars'; import './components/new_issue_for_discussion'; -import { hasVueMRDiscussionsCookie } from '../lib/utils/common_utils'; export default () => { - const projectPathHolder = document.querySelector('.merge-request') || document.querySelector('.commit-box'); + const projectPathHolder = + document.querySelector('.merge-request') || + document.querySelector('.commit-box'); const projectPath = projectPathHolder.dataset.projectPath; - const COMPONENT_SELECTOR = 'resolve-btn, resolve-discussion-btn, jump-to-discussion, comment-and-resolve-btn, new-issue-for-discussion-btn'; + const COMPONENT_SELECTOR = + 'resolve-btn, resolve-discussion-btn, jump-to-discussion, comment-and-resolve-btn, new-issue-for-discussion-btn'; window.gl = window.gl || {}; window.gl.diffNoteApps = {}; @@ -28,9 +30,9 @@ export default () => { window.ResolveService = new gl.DiffNotesResolveServiceClass(projectPath); gl.diffNotesCompileComponents = () => { - $('diff-note-avatars').each(function () { + $('diff-note-avatars').each(function() { const tmp = Vue.extend({ - template: $(this).get(0).outerHTML + template: $(this).get(0).outerHTML, }); const tmpApp = new tmp().$mount(); @@ -41,12 +43,12 @@ export default () => { }); }); - const $components = $(COMPONENT_SELECTOR).filter(function () { + const $components = $(COMPONENT_SELECTOR).filter(function() { return $(this).closest('resolve-count').length !== 1; }); if ($components) { - $components.each(function () { + $components.each(function() { const $this = $(this); const noteId = $this.attr(':note-id'); const discussionId = $this.attr(':discussion-id'); @@ -54,7 +56,7 @@ export default () => { if ($this.is('comment-and-resolve-btn') && !discussionId) return; const tmp = Vue.extend({ - template: $this.get(0).outerHTML + template: $this.get(0).outerHTML, }); const tmpApp = new tmp().$mount(); @@ -69,14 +71,5 @@ export default () => { gl.diffNotesCompileComponents(); - if (!hasVueMRDiscussionsCookie()) { - new Vue({ - el: '#resolve-count-app', - components: { - 'resolve-count': ResolveCount - }, - }); - } - $(window).trigger('resize.nav'); }; diff --git a/app/assets/javascripts/diff_notes/services/resolve.js b/app/assets/javascripts/diff_notes/services/resolve.js index d16f9297de1..0b3568e432d 100644 --- a/app/assets/javascripts/diff_notes/services/resolve.js +++ b/app/assets/javascripts/diff_notes/services/resolve.js @@ -8,8 +8,12 @@ window.gl = window.gl || {}; class ResolveServiceClass { constructor(root) { - this.noteResource = Vue.resource(`${root}/notes{/noteId}/resolve?html=true`); - this.discussionResource = Vue.resource(`${root}/merge_requests{/mergeRequestId}/discussions{/discussionId}/resolve?html=true`); + this.noteResource = Vue.resource( + `${root}/notes{/noteId}/resolve?html=true`, + ); + this.discussionResource = Vue.resource( + `${root}/merge_requests{/mergeRequestId}/discussions{/discussionId}/resolve?html=true`, + ); } resolve(noteId) { @@ -33,7 +37,7 @@ class ResolveServiceClass { promise .then(resp => resp.json()) - .then((data) => { + .then(data => { discussion.loading = false; const resolvedBy = data ? data.resolved_by : null; @@ -45,9 +49,13 @@ class ResolveServiceClass { if (gl.mrWidget) gl.mrWidget.checkStatus(); discussion.updateHeadline(data); - document.dispatchEvent(new CustomEvent('refreshVueNotes')); }) - .catch(() => new Flash('An error occurred when trying to resolve a discussion. Please try again.')); + .catch( + () => + new Flash( + 'An error occurred when trying to resolve a discussion. Please try again.', + ), + ); } resolveAll(mergeRequestId, discussionId) { @@ -55,10 +63,13 @@ class ResolveServiceClass { discussion.loading = true; - return this.discussionResource.save({ - mergeRequestId, - discussionId, - }, {}); + return this.discussionResource.save( + { + mergeRequestId, + discussionId, + }, + {}, + ); } unResolveAll(mergeRequestId, discussionId) { @@ -66,10 +77,13 @@ class ResolveServiceClass { discussion.loading = true; - return this.discussionResource.delete({ - mergeRequestId, - discussionId, - }, {}); + return this.discussionResource.delete( + { + mergeRequestId, + discussionId, + }, + {}, + ); } } diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue new file mode 100644 index 00000000000..fde261a4aa7 --- /dev/null +++ b/app/assets/javascripts/diffs/components/app.vue @@ -0,0 +1,83 @@ +<script> +import { mapState, mapActions } from 'vuex'; +import loadingIcon from '../../vue_shared/components/loading_icon.vue'; +import compareVersions from './compare_versions.vue'; +import changedFiles from './changed_files.vue'; +import diffFile from './diff_file.vue'; + +export default { + name: 'DiffsApp', + components: { + loadingIcon, + compareVersions, + changedFiles, + diffFile, + }, + props: { + endpoint: { + type: String, + required: true, + }, + shouldShow: { + type: Boolean, + required: false, + default: false, + }, + }, + data() { + return { + activeFile: '', + }; + }, + computed: { + ...mapState({ + isLoading: state => state.diffs.isLoading, + diffFiles: state => state.diffs.diffFiles, + }), + }, + mounted() { + this.setEndpoint(this.endpoint); + this.fetchDiffFiles(); // TODO: @fatihacet Error handling + }, + methods: { + ...mapActions(['setEndpoint', 'fetchDiffFiles']), + setActive(filePath) { + this.activeFile = filePath; + }, + unsetActive(filePath) { + if (this.activeFile === filePath) { + this.activeFile = ''; + } + }, + }, +}; +</script> + +<template> + <div v-if="shouldShow"> + <loading-icon + v-if="isLoading" + size="3" + /> + <div + v-else + id="diffs" + class="diffs tab-pane" + > + <compare-versions /> + <changed-files + :diff-files="diffFiles" + :active-file="activeFile" + /> + <div class="files"> + <diff-file + @setActive="setActive(file.filePath)" + @unsetActive="unsetActive(file.filePath)" + v-for="file in diffFiles" + :key="file.newPath" + :file="file" + /> + </div> + </div> + </div> +</template> diff --git a/app/assets/javascripts/diffs/components/changed_files.vue b/app/assets/javascripts/diffs/components/changed_files.vue new file mode 100644 index 00000000000..dee3dbf95d0 --- /dev/null +++ b/app/assets/javascripts/diffs/components/changed_files.vue @@ -0,0 +1,275 @@ +<script> +import _ from 'underscore'; +import { mapGetters, mapActions } from 'vuex'; +import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; +import Icon from '~/vue_shared/components/icon.vue'; +import { pluralize } from '~/lib/utils/text_utility'; + +export default { + components: { + Icon, + ClipboardButton, + }, + props: { + diffFiles: { + type: Array, + required: true, + }, + activeFile: { + type: String, + required: false, + default: '', + }, + }, + data() { + return { + searchText: '', + isStuck: false, + showCurrentDiffTitle: false, + }; + }, + computed: { + ...mapGetters(['isInlineView', 'isParallelView']), + sumAddedLines() { + return this.sumValues('addedLines'); + }, + sumRemovedLines() { + return this.sumValues('removedLines'); + }, + filteredDiffFiles() { + return this.diffFiles.filter(file => + file.filePath.toLowerCase().includes(this.searchText.toLowerCase()), + ); + }, + stickyClass() { + return this.isStuck ? 'is-stuck' : ''; + }, + }, + mounted() { + this.throttledHandleScroll = _.throttle(this.handleScroll, 100); + document.addEventListener('scroll', this.throttledHandleScroll); + }, + beforeDestroy() { + document.removeEventListener('scroll', this.throttledHandleScroll); + }, + methods: { + ...mapActions(['setInlineDiffViewType', 'setParallelDiffViewType']), + pluralize, + handleScroll() { + if (!this.$refs.stickyBar) return; + + const barPosition = this.$refs.stickyBar.offsetTop; + const scrollPosition = window.scrollY; + + const top = Math.floor(barPosition - scrollPosition); + + this.isStuck = top < 112; + this.showCurrentDiffTitle = top < 0; + }, + sumValues(key) { + return this.diffFiles.reduce((total, file) => total + file[key], 0); + }, + fileChangedIcon(diffFile) { + if (diffFile.deletedFile) { + return 'file-deletion'; + } else if (diffFile.newFile) { + return 'file-addition'; + } + return 'file-modified'; + }, + fileChangedClass(diffFile) { + if (diffFile.deletedFile) { + return 'cred'; + } else if (diffFile.newFile) { + return 'cgreen'; + } + + return ''; + }, + truncatedDiffPath(path) { + const maxLength = 60; + + if (path.length > maxLength) { + const start = path.length - maxLength; + const end = start + maxLength; + return `...${path.slice(start, end)}`; + } + + return path; + }, + }, +}; +</script> + +<template> + <div + v-if="diffFiles.length > 0" + ref="stickyBar" + class="content-block oneline-block diff-files-changed diff-files-changed-merge-request + files-changed js-diff-files-changed" + :class="stickyClass" + > + <div class="files-changed-inner"> + <div + v-show="!showCurrentDiffTitle" + class="inline-parallel-buttons hidden-xs hidden-sm" + > + <a + class="hidden-xs btn btn-default" + href="/fatihacet/test/merge_requests/5/diffs?w=1&TODO" + > + {{ __('Hide whitespace changes') }} + </a> + <div class="btn-group"> + <button + type="button" + @click="setInlineDiffViewType" + :class="{ active: isInlineView }" + id="inline-diff-btn" + class="btn" + data-view-type="inline" + > + {{ __('Inline') }} + </button> + <button + type="button" + @click="setParallelDiffViewType" + :class="{ active: isParallelView }" + id="parallel-diff-btn" + class="btn" + data-view-type="parallel" + > + {{ __('Side-by-side') }} + </button> + </div> + </div> + + <div class="commit-stat-summary dropdown"> + Showing + <button + class="diff-stats-summary-toggler js-diff-stats-dropdown" + data-toggle="dropdown" + type="button" + aria-expanded="false" + > + <span> + {{ pluralize(`${diffFiles.length} changed file`, diffFiles.length) }} + </span> + <icon + name="chevron-down" + :size="8" + /> + </button> + <div class="dropdown-menu diff-file-changes"> + <div class="dropdown-input"> + <input + v-model="searchText" + type="search" + class="dropdown-input-field" + placeholder="Search files" + autocomplete="off" + /> + <i + v-if="searchText.length === 0" + aria-hidden="true" + data-hidden="true" + class="fa fa-search dropdown-input-search"> + </i> + <i + v-else + role="button" + aria-hidden="true" + data-hidden="true" + class="fa fa-times dropdown-input-search" + @click="searchText = ''" + ></i> + </div> + <ul> + <li + v-for="diffFile in filteredDiffFiles" + :key="diffFile.name" + > + <a + class="diff-changed-file" + :href="`#${diffFile.fileHash}`" + :title="diffFile.newPath" + > + <icon + :name="fileChangedIcon(diffFile)" + :size="16" + :class="fileChangedClass(diffFile)" + class="diff-file-changed-icon append-right-8" + /> + <span class="diff-changed-file-content append-right-8"> + <strong + v-if="diffFile.blobName" + class="diff-changed-file-name" + > + {{ diffFile.blobName }} + </strong> + <strong + v-else + class="diff-changed-blank-file-name" + > + {{ s__('Diffs|No file name available') }} + </strong> + <span class="diff-changed-file-path prepend-top-5"> + {{ truncatedDiffPath(diffFile.blobPath) }} + </span> + </span> + <span class="diff-changed-stats"> + <span class="cgreen"> + +{{ diffFile.addedLines }} + </span> + <span class="cred"> + -{{ diffFile.removedLines }} + </span> + </span> + </a> + </li> + + <li + v-show="filteredDiffFiles.length === 0" + class="dropdown-menu-empty-item" + > + <a href="javascript:void(0)"> + No files found + </a> + </li> + </ul> + </div> + + <span + v-show="!isStuck" + class="diff-stats-additions-deletions-expanded" + id="diff-stats" + > + with + <strong class="cgreen"> + {{ pluralize(`${sumAddedLines} addition`, sumAddedLines) }} + </strong> + and + <strong class="cred"> + {{ pluralize(`${sumRemovedLines} deletion`, sumRemovedLines) }} + </strong> + </span> + + <span + v-show="activeFile" + class="prepend-left-5" + > + <strong class="prepend-right-5"> + {{ truncatedDiffPath(activeFile) }} + </strong> + <clipboard-button + :text="activeFile" + :title="s__('Copy file name to clipboard')" + tooltip-placement="bottom" + tooltip-container="body" + class="btn btn-default btn-transparent btn-clipboard" + /> + </span> + </div> + </div> + </div> +</template> diff --git a/app/assets/javascripts/diffs/components/compare_versions.vue b/app/assets/javascripts/diffs/components/compare_versions.vue new file mode 100644 index 00000000000..3a1a1251add --- /dev/null +++ b/app/assets/javascripts/diffs/components/compare_versions.vue @@ -0,0 +1,38 @@ +<template> + <div class="mr-version-controls"> + <div class="mr-version-menus-container content-block"> + Changes between + <span class="dropdown inline mr-version-dropdown"> + <a + class="dropdown-toggle btn btn-default" + data-toggle="dropdown" + aria-expanded="false" + > + <span> + latest version + </span> + <i + aria-hidden="true" + data-hidden="true" + class="fa fa-caret-down" + ></i> + </a> + </span> + and + <span class="dropdown inline mr-version-compare-dropdown"> + <a + class="btn btn-default dropdown-toggle" + data-toggle="dropdown" + aria-expanded="false" + > + <span class="ref-name">master</span> + <i + aria-hidden="true" + data-hidden="true" + class="fa fa-caret-down" + ></i> + </a> + </span> + </div> + </div> +</template> diff --git a/app/assets/javascripts/diffs/components/diff_content.vue b/app/assets/javascripts/diffs/components/diff_content.vue new file mode 100644 index 00000000000..6313ee874b0 --- /dev/null +++ b/app/assets/javascripts/diffs/components/diff_content.vue @@ -0,0 +1,45 @@ +<script> +import { mapState } from 'vuex'; +import inlineDiffView from './inline_diff_view.vue'; +import parallelDiffView from './parallel_diff_view.vue'; +import { INLINE_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE } from '../constants'; + +export default { + components: { + inlineDiffView, + parallelDiffView, + }, + props: { + diffFile: { + type: Object, + required: true, + }, + }, + computed: { + ...mapState({ + diffViewType: state => state.diffs.diffViewType, + }), + }, + created() { + this.INLINE_DIFF_VIEW_TYPE = INLINE_DIFF_VIEW_TYPE; + this.PARALLEL_DIFF_VIEW_TYPE = PARALLEL_DIFF_VIEW_TYPE; + }, +}; +</script> + +<template> + <div class="diff-content"> + <div class="diff-viewer"> + <inline-diff-view + v-if="diffViewType === INLINE_DIFF_VIEW_TYPE" + :diff-file="diffFile" + :diff-lines="diffFile.highlightedDiffLines || []" + /> + <parallel-diff-view + v-if="diffViewType === PARALLEL_DIFF_VIEW_TYPE" + :diff-file="diffFile" + :diff-lines="diffFile.parallelDiffLines || []" + /> + </div> + </div> +</template> diff --git a/app/assets/javascripts/diffs/components/diff_discussions.vue b/app/assets/javascripts/diffs/components/diff_discussions.vue new file mode 100644 index 00000000000..18377978593 --- /dev/null +++ b/app/assets/javascripts/diffs/components/diff_discussions.vue @@ -0,0 +1,34 @@ +<script> +import noteableDiscussion from '../../notes/components/noteable_discussion.vue'; + +export default { + components: { + noteableDiscussion, + }, + props: { + notes: { + type: Array, + required: true, + }, + }, +}; +</script> + +<template> + <div> + <div + v-for="notesArr in notes" + :key="notesArr.id" + class="discussion-notes diff-discussions" + > + <ul class="notes"> + <noteable-discussion + :note="notesArr" + :render-header="false" + :render-diff-file="false" + :always-expanded="true" + /> + </ul> + </div> + </div> +</template> diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue new file mode 100644 index 00000000000..c15941282ee --- /dev/null +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -0,0 +1,99 @@ +<script> +import diffFileHeader from '../../notes/components/diff_file_header.vue'; +import diffContent from './diff_content.vue'; + +export default { + components: { + diffFileHeader, + diffContent, + }, + props: { + file: { + type: Object, + required: true, + }, + }, + data() { + return { + isExpanded: true, + isActive: false, + }; + }, + mounted() { + document.addEventListener('scroll', this.handleScroll); + }, + beforeDestroy() { + document.removeEventListener('scroll', this.handleScroll); + }, + methods: { + handleToggle() { + this.isExpanded = !this.isExpanded; + }, + handleScroll() { + if (!this.updating) { + requestAnimationFrame(this.scrollUpdate); + this.updating = true; + } + }, + scrollUpdate() { + const header = document.querySelector('.js-diff-files-changed'); + if (!header) { + this.updating = false; + return; + } + + const { top, bottom } = this.$el.getBoundingClientRect(); + const { + top: topOfFixedHeader, + bottom: bottomOfFixedHeader, + } = header.getBoundingClientRect(); + + const headerOverlapsContent = top < topOfFixedHeader && bottom > bottomOfFixedHeader; + const fullyAboveHeader = bottom < bottomOfFixedHeader; + const fullyBelowHeader = top > topOfFixedHeader; + + if (headerOverlapsContent && !this.isActive) { + this.$emit('setActive'); + this.isActive = true; + } else if (this.isActive && (fullyAboveHeader || fullyBelowHeader)) { + this.$emit('unsetActive'); + this.isActive = false; + } + + this.updating = false + } + }, +}; +</script> + +<template> + <div + class="diff-file file-holder" + :id="file.fileHash" + > + <diff-file-header + :diff-file="file" + :collapsible="true" + :add-merge-request-buttons="true" + @toggleFile="handleToggle" + class="js-file-title file-title" + /> + <diff-content + v-if="isExpanded" + :diff-file="file" + /> + <div + v-else + class="nothing-here-block diff-collapsed" + > + This diff is collapsed. + <a + @click.prevent="handleToggle" + class="click-to-expand" + href="#" + > + Click to expand it. + </a> + </div> + </div> +</template> diff --git a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue new file mode 100644 index 00000000000..ddec37a97b0 --- /dev/null +++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue @@ -0,0 +1,158 @@ +<script> +import { mapState, mapGetters, mapActions } from 'vuex'; +import Icon from '~/vue_shared/components/icon.vue'; +import { MATCH_LINE_TYPE, UNFOLD_COUNT, CONTEXT_LINE_TYPE } from '../constants'; +import * as utils from '../store/utils'; + +export default { + props: { + fileHash: { + type: String, + required: true, + }, + lineType: { + type: String, + required: false, + default: '', + }, + lineNumber: { + type: Number, + required: false, + default: 0, + }, + lineCode: { + type: String, + required: false, + default: '', + }, + linePosition: { + type: String, + required: false, + default: '', + }, + metaData: { + type: Object, + required: false, + default: () => ({}), + }, + showCommentButton: { + type: Boolean, + required: false, + default: false, + }, + contextLinesPath: { + type: String, + required: true, + }, + isBottom: { + type: Boolean, + required: false, + default: false, + }, + }, + components: { + Icon, + }, + computed: { + ...mapState({ + diffViewType: state => state.diffs.diffViewType, + diffFiles: state => state.diffs.diffFiles, + }), + ...mapGetters(['isLoggedIn']), + isMatchLine() { + return this.lineType === MATCH_LINE_TYPE; + }, + isContextLine() { + return this.lineType === CONTEXT_LINE_TYPE; + }, + getLineHref() { + return this.lineCode ? `#${this.lineCode}` : '#'; + }, + shouldShowCommentButton() { + return ( + this.isLoggedIn && + this.showCommentButton && + !this.isMatchLine && + !this.isContextLine + ); + }, + }, + methods: { + ...mapActions(['loadMoreLines']), + handleCommentButton() { + this.$emit('showCommentForm', { + lineCode: this.lineCode, + linePosition: this.linePosition, + }); + }, + handleLoadMoreLines() { + const endpoint = this.contextLinesPath; + const oldLineNumber = this.metaData.oldPos || 0; + const newLineNumber = this.metaData.newPos || 0; + const offset = newLineNumber - oldLineNumber; + const bottom = this.isBottom; + const fileHash = this.fileHash; + const view = this.diffViewType; + let unfold = true; + let lineNumber = newLineNumber - 1; + let since = lineNumber - UNFOLD_COUNT; + let to = lineNumber; + + if (bottom) { + lineNumber = newLineNumber + 1; + since = lineNumber; + to = lineNumber + UNFOLD_COUNT; + } else { + const diffFile = utils.findDiffFile(this.diffFiles, this.fileHash); + const indexForInline = utils.findIndexInInlineLines( + diffFile.highlightedDiffLines, + { oldLineNumber, newLineNumber }, + ); + const prevLine = diffFile.highlightedDiffLines[indexForInline - 2]; + const prevLineNumber = (prevLine && prevLine.newLine) || 0; + + if (since <= prevLineNumber + 1) { + since = prevLineNumber + 1; + unfold = false; + } + } + + const params = { since, to, bottom, offset, unfold, view }; + const lineNumbers = { oldLineNumber, newLineNumber }; + this.loadMoreLines({ endpoint, params, lineNumbers, fileHash }); + }, + }, +}; +</script> + +<template> + <div> + <span + v-if="isMatchLine" + @click="handleLoadMoreLines" + class="context-cell" + >...</span> + <template + v-else + > + <button + v-if="shouldShowCommentButton" + @click="handleCommentButton" + type="button" + class="add-diff-note js-add-diff-note-button" + title="Add a comment to this line" + > + <icon + name="comment" + :size="12" + /> + </button> + <a + v-if="lineNumber" + :data-linenumber="lineNumber" + :href="getLineHref" + > + </a> + </template> + </div> +</template> diff --git a/app/assets/javascripts/diffs/components/diff_line_note_form.vue b/app/assets/javascripts/diffs/components/diff_line_note_form.vue new file mode 100644 index 00000000000..fe6beeb184a --- /dev/null +++ b/app/assets/javascripts/diffs/components/diff_line_note_form.vue @@ -0,0 +1,85 @@ +<script> +import { mapState, mapGetters, mapActions } from 'vuex'; +import noteForm from '../../notes/components/note_form.vue'; +import * as utils from '../store/utils'; + +export default { + components: { + noteForm, + }, + props: { + diffFile: { + type: Object, + required: true, + }, + diffLines: { + type: Array, + required: true, + }, + line: { + type: Object, + required: true, + }, + position: { + type: String, + required: false, + default: '', + }, + noteTargetLine: { + type: Object, + required: true, + }, + }, + computed: { + ...mapState({ + noteableData: state => state.notes.noteableData, + diffViewType: state => state.diffs.diffViewType, + }), + ...mapGetters(['noteableType', 'getNotesDataByProp']), + }, + methods: { + ...mapActions(['cancelCommentForm', 'saveNote', 'fetchNotes']), + handleCancelCommentForm() { + const { diffLines, line, position } = this; + + this.cancelCommentForm({ + linePosition: position, + diffLines, + formId: line.id, + }); + }, + handleSaveNote(note) { + const postData = utils.getNoteFormData({ + note, + noteableData: this.noteableData, + noteableType: this.noteableType, + noteTargetLine: this.noteTargetLine, + diffViewType: this.diffViewType, + diffFile: this.diffFile, + linePosition: this.position, + }); + + // FIXME: @fatihacet -- This should be fixed, no need to fetchNotes again + this.saveNote(postData).then(() => { + const endpoint = this.getNotesDataByProp('discussionsPath'); + + this.fetchNotes(endpoint).then(() => { + this.handleCancelCommentForm(); + }); + }); + }, + }, +}; +</script> + +<template> + <div class="content discussion-form js-discussion-note-form discussion-form-container"> + <note-form + :is-editing="true" + save-button-title="Comment" + class="diff-comment-form" + @cancelForm="handleCancelCommentForm" + @handleFormUpdate="handleSaveNote" + /> + </div> +</template> diff --git a/app/assets/javascripts/diffs/components/inline_diff_view.vue b/app/assets/javascripts/diffs/components/inline_diff_view.vue new file mode 100644 index 00000000000..82e4497cd15 --- /dev/null +++ b/app/assets/javascripts/diffs/components/inline_diff_view.vue @@ -0,0 +1,123 @@ +<script> +import diffContentMixin from '../mixins/diff_content'; +import { + MATCH_LINE_TYPE, + CONTEXT_LINE_TYPE, + LINE_HOVER_CLASS_NAME, + LINE_UNFOLD_CLASS_NAME, +} from '../constants'; + +export default { + mixins: [diffContentMixin], + methods: { + handleMouse(lineCode, isOver) { + this.hoveredLineCode = isOver ? lineCode : null; + }, + getLineClass(line) { + const isSameLine = this.hoveredLineCode === line.lineCode; + const isMatchLine = line.type === MATCH_LINE_TYPE; + const isContextLine = line.type === CONTEXT_LINE_TYPE; + + return { + [line.type]: line.type, + [LINE_HOVER_CLASS_NAME]: + this.isLoggedIn && isSameLine && !isMatchLine && !isContextLine, + [LINE_UNFOLD_CLASS_NAME]: this.isLoggedIn && isMatchLine, + }; + }, + }, +}; +</script> + +<template> + <table + :class="userColorScheme" + class="code diff-wrap-lines js-syntax-highlight text-file"> + <tbody> + <template + v-for="(line, index) in normalizedDiffLines" + > + <tr + :id="line.lineCode" + :key="line.lineCode" + :class="getRowClass(line)" + class="line_holder" + @mouseover="handleMouse(line.lineCode, true)" + @mouseout="handleMouse(line.lineCode, false)" + > + <td + :class="getLineClass(line)" + class="diff-line-num old_line" + > + <diff-line-gutter-content + :file-hash="fileHash" + :line-type="line.type" + :line-code="line.lineCode" + :line-number="line.oldLine" + :meta-data="line.metaData" + :show-comment-button="true" + :context-lines-path="diffFile.contextLinesPath" + :is-bottom="index + 1 === diffLinesLength" + @showCommentForm="handleShowCommentForm" + /> + </td> + <td + :class="getLineClass(line)" + class="diff-line-num new_line" + > + <diff-line-gutter-content + :file-hash="fileHash" + :line-type="line.type" + :line-code="line.lineCode" + :line-number="line.newLine" + :meta-data="line.metaData" + :is-bottom="index + 1 === diffLinesLength" + :context-lines-path="diffFile.contextLinesPath" + /> + </td> + <td + :class="line.type" + class="line_content" + v-html="line.richText" + > + </td> + </tr> + <tr + v-if="discussionsByLineCode[line.lineCode]" + :key="discussionsByLineCode[line.lineCode].id" + class="notes_holder" + > + <td + class="notes_line" + colspan="2" + ></td> + <td class="notes_content"> + <div class="content"> + <diff-discussions + :notes="discussionsByLineCode[line.lineCode]" + /> + </div> + </td> + </tr> + <tr + v-if="line.type === 'commentForm'" + :key="line.id" + class="notes_holder js-temp-notes-holder" + > + <td + class="notes_line" + colspan="2" + ></td> + <td class="notes_content"> + <diff-line-note-form + :diff-file="diffFile" + :diff-lines="diffLines" + :line="line" + :note-target-line="diffLines[index - 1]" + /> + </td> + </tr> + </template> + </tbody> + </table> +</template> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_view.vue b/app/assets/javascripts/diffs/components/parallel_diff_view.vue new file mode 100644 index 00000000000..8ab3d7cdf6f --- /dev/null +++ b/app/assets/javascripts/diffs/components/parallel_diff_view.vue @@ -0,0 +1,195 @@ +<script> +import diffContentMixin from '../mixins/diff_content'; +import { + EMPTY_CELL_TYPE, + MATCH_LINE_TYPE, + CONTEXT_LINE_TYPE, + LINE_HOVER_CLASS_NAME, + LINE_UNFOLD_CLASS_NAME, +} from '../constants'; + +export default { + mixins: [diffContentMixin], + computed: { + parallelDiffLines() { + return this.normalizedDiffLines.map(line => { + if (!line.left) { + Object.assign(line, { left: { type: EMPTY_CELL_TYPE } }); + } else if (!line.right) { + Object.assign(line, { right: { type: EMPTY_CELL_TYPE } }); + } + + return line; + }); + }, + }, + methods: { + hasDiscussion(line) { + const discussions = this.discussionsByLineCode; + + return ( + discussions[line.left.lineCode] || discussions[line.right.lineCode] + ); + }, + getClassName(line, position) { + const { type, lineCode } = line[position]; + const isMatchLine = type === MATCH_LINE_TYPE; + const isContextLine = + !isMatchLine && type !== EMPTY_CELL_TYPE && type !== CONTEXT_LINE_TYPE; + const isSameLine = this.hoveredLineCode === lineCode; + const isSameSection = position === this.hoveredSection; + + return { + [type]: type, + [LINE_HOVER_CLASS_NAME]: + this.isLoggedIn && isContextLine && isSameLine && isSameSection, + [LINE_UNFOLD_CLASS_NAME]: this.isLoggedIn && isMatchLine, + }; + }, + handleMouse(e, line, isHover) { + const cell = e.target.closest('td'); + + if (isHover) { + if (this.$refs.leftLines.indexOf(cell) > -1) { + this.hoveredLineCode = line.left.lineCode; + this.hoveredSection = 'left'; + } else if (this.$refs.rightLines.indexOf(cell) > -1) { + this.hoveredLineCode = line.right.lineCode; + this.hoveredSection = 'right'; + } + } else { + this.hoveredLineCode = null; + this.hoveredSection = null; + } + }, + }, +}; +</script> + +<template> + <div + :class="userColorScheme" + class="code diff-wrap-lines js-syntax-highlight text-file"> + <table> + <tbody> + <template + v-for="(line, index) in parallelDiffLines" + > + <tr + :key="index" + :class="getRowClass(line)" + class="line_holder parallel" + @mouseover="handleMouse($event, line, true)" + @mouseout="handleMouse($event, line, false)" + > + <td + :class="getClassName(line, 'left')" + ref="leftLines" + class="diff-line-num old_line" + > + <diff-line-gutter-content + :file-hash="fileHash" + :line-type="line.left.type" + :line-code="line.left.lineCode" + :line-number="line.left.oldLine" + :meta-data="line.left.metaData" + :show-comment-button="true" + :context-lines-path="diffFile.contextLinesPath" + :is-bottom="index + 1 === diffLinesLength" + line-position="left" + @showCommentForm="handleShowCommentForm" + /> + </td> + <td + :class="getClassName(line, 'left')" + ref="leftLines" + v-html="line.left.richText" + class="line_content parallel left-side" + > + </td> + <td + :class="getClassName(line, 'right')" + ref="rightLines" + class="diff-line-num new_line" + > + <diff-line-gutter-content + :file-hash="fileHash" + :line-type="line.right.type" + :line-code="line.right.lineCode" + :line-number="line.right.newLine" + :meta-data="line.right.metaData" + :show-comment-button="true" + :context-lines-path="diffFile.contextLinesPath" + :is-bottom="index + 1 === diffLinesLength" + line-position="right" + @showCommentForm="handleShowCommentForm" + /> + </td> + <td + :class="getClassName(line, 'right')" + ref="rightLines" + v-html="line.right.richText" + class="line_content parallel right-side" + > + </td> + </tr> + <tr + v-if="hasDiscussion(line)" + :key="line.left.lineCode || line.right.lineCode" + class="notes_holder" + > + <td class="notes_line old"></td> + <td class="notes_content parallel old"> + <div + v-if="discussionsByLineCode[line.left.lineCode]" + class="content" + > + <diff-discussions + :notes="discussionsByLineCode[line.left.lineCode]" + /> + </div> + </td> + <td class="notes_line new"></td> + <td class="notes_content parallel new"> + <div + v-if="discussionsByLineCode[line.right.lineCode] && line.right.type" + class="content" + > + <diff-discussions + :notes="discussionsByLineCode[line.right.lineCode]" + /> + </div> + </td> + </tr> + <tr + v-if="line.left.type === 'commentForm' || line.right.type === 'commentForm'" + :key="line.id" + class="notes_holder js-temp-notes-holder"> + <td class="notes_line old"></td> + <td class="notes_content parallel old"> + <diff-line-note-form + v-if="line.left.type === 'commentForm'" + :diff-file="diffFile" + :diff-lines="diffLines" + :line="line.left" + :note-target-line="diffLines[index - 1].left" + position="left" + /> + </td> + <td class="notes_line new"></td> + <td class="notes_content parallel new"> + <diff-line-note-form + v-if="line.right.type === 'commentForm'" + :diff-file="diffFile" + :diff-lines="diffLines" + :line="line.right" + :note-target-line="diffLines[index - 1].right" + position="right" + /> + </td> + </tr> + </template> + </tbody> + </table> + </div> +</template> diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js new file mode 100644 index 00000000000..f93e270575f --- /dev/null +++ b/app/assets/javascripts/diffs/constants.js @@ -0,0 +1,17 @@ +export const INLINE_DIFF_VIEW_TYPE = 'inline'; +export const PARALLEL_DIFF_VIEW_TYPE = 'parallel'; +export const MATCH_LINE_TYPE = 'match'; +export const CONTEXT_LINE_TYPE = 'context'; +export const DIFF_VIEW_COOKIE_NAME = 'diff_view'; +export const EMPTY_CELL_TYPE = 'empty-cell'; +export const LINE_HOVER_CLASS_NAME = 'is-over'; +export const LINE_UNFOLD_CLASS_NAME = 'unfold js-unfold'; +export const CONTEXT_LINE_CLASS_NAME = 'diff-expanded'; +export const COMMENT_FORM_TYPE = 'commentForm'; +export const LINE_POSITION_LEFT = 'left'; +export const LINE_POSITION_RIGHT = 'right'; +export const TEXT_DIFF_POSITION_TYPE = 'text'; +export const DIFF_NOTE_TYPE = 'DiffNote'; +export const NEW_LINE_TYPE = 'new'; +export const OLD_LINE_TYPE = 'old'; +export const UNFOLD_COUNT = 20; diff --git a/app/assets/javascripts/diffs/index.js b/app/assets/javascripts/diffs/index.js new file mode 100644 index 00000000000..f0038091aa5 --- /dev/null +++ b/app/assets/javascripts/diffs/index.js @@ -0,0 +1,27 @@ +import Vue from 'vue'; +import diffsApp from './components/app.vue'; + +document.addEventListener( + 'DOMContentLoaded', + () => + new Vue({ + el: '#js-diffs-app', + components: { + diffsApp, + }, + data() { + const dataset = document.querySelector(this.$options.el).dataset; + + return { + endpoint: dataset.path, + }; + }, + render(createElement) { + return createElement('diffs-app', { + props: { + endpoint: this.endpoint, + }, + }); + }, + }), +); diff --git a/app/assets/javascripts/diffs/mixins/diff_content.js b/app/assets/javascripts/diffs/mixins/diff_content.js new file mode 100644 index 00000000000..bb2ece68bc0 --- /dev/null +++ b/app/assets/javascripts/diffs/mixins/diff_content.js @@ -0,0 +1,93 @@ +import { mapGetters, mapActions } 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 { CONTEXT_LINE_TYPE, CONTEXT_LINE_CLASS_NAME } from '../constants'; + +export default { + props: { + diffFile: { + type: Object, + required: true, + }, + diffLines: { + type: Array, + required: true, + }, + }, + data() { + return { + hoveredLineCode: undefined, + hoveredSection: undefined, + }; + }, + components: { + diffDiscussions, + diffLineNoteForm, + diffLineGutterContent, + }, + computed: { + ...mapGetters(['discussionsByLineCode', 'isLoggedIn']), + userColorScheme() { + return window.gon.user_color_scheme; + }, + normalizedDiffLines() { + return this.diffLines.map(line => { + if (line.richText) { + return this.trimFirstChar(line); + } + + if (line.left) { + Object.assign(line, { left: this.trimFirstChar(line.left) }); + } + + if (line.right) { + Object.assign(line, { right: this.trimFirstChar(line.right) }); + } + + return line; + }); + }, + diffLinesLength() { + return this.normalizedDiffLines.length; + }, + fileHash() { + return this.diffFile.fileHash; + }, + }, + methods: { + ...mapActions(['showCommentForm', 'cancelCommentForm']), + getRowClass(line) { + const isContextLine = line.left + ? line.left.type === CONTEXT_LINE_TYPE + : line.type === CONTEXT_LINE_TYPE; + + return { + [line.type]: line.type, + [CONTEXT_LINE_CLASS_NAME]: isContextLine, + }; + }, + trimFirstChar(line) { + if (!line.richText) { + return line; + } + + const firstChar = line.richText.charAt(0); + + if (firstChar === ' ' || firstChar === '+' || firstChar === '-') { + Object.assign(line, { + richText: line.richText.substring(1), + }); + } + + return line; + }, + handleShowCommentForm({ lineCode, linePosition }) { + this.showCommentForm({ + diffLines: this.diffLines, + lineCode, + linePosition, + }); + }, + }, +}; diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js new file mode 100644 index 00000000000..3185c90cecb --- /dev/null +++ b/app/assets/javascripts/diffs/store/actions.js @@ -0,0 +1,64 @@ +import Vue from 'vue'; +import axios from '~/lib/utils/axios_utils'; +import Cookies from 'js-cookie'; +import { handleLocationHash } from '~/lib/utils/common_utils'; +import * as types from './mutation_types'; +import { + PARALLEL_DIFF_VIEW_TYPE, + INLINE_DIFF_VIEW_TYPE, + DIFF_VIEW_COOKIE_NAME, +} from '../constants'; + +export const setEndpoint = ({ commit }, endpoint) => { + commit(types.SET_ENDPOINT, endpoint); +}; + +export const setLoadingState = ({ commit }, state) => { + commit(types.SET_LOADING, state); +}; + +export const fetchDiffFiles = ({ state, commit }) => { + commit(types.SET_LOADING, true); + + return axios + .get(state.endpoint) + .then(res => { + commit(types.SET_LOADING, false); + commit(types.SET_DIFF_FILES, res.data.diff_files); + return Vue.nextTick(); + }) + .then(handleLocationHash); +}; + +export const setInlineDiffViewType = ({ commit }) => { + commit(types.SET_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE); + Cookies.set(DIFF_VIEW_COOKIE_NAME, INLINE_DIFF_VIEW_TYPE); +}; + +export const setParallelDiffViewType = ({ commit }) => { + commit(types.SET_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE); + Cookies.set(DIFF_VIEW_COOKIE_NAME, PARALLEL_DIFF_VIEW_TYPE); +}; + +export const showCommentForm = ({ commit }, params) => { + commit(types.ADD_COMMENT_FORM_LINE, params); +}; + +export const cancelCommentForm = ({ commit }, params) => { + commit(types.REMOVE_COMMENT_FORM_LINE, params); +}; + +export const loadMoreLines = ({ commit }, options) => { + const { endpoint, params, lineNumbers, fileHash } = options; + + return axios.get(endpoint, { params }).then(res => { + const contextLines = res.data || []; + + commit(types.ADD_CONTEXT_LINES, { + lineNumbers, + contextLines, + params, + fileHash, + }); + }); +}; diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js new file mode 100644 index 00000000000..ec4ea070209 --- /dev/null +++ b/app/assets/javascripts/diffs/store/getters.js @@ -0,0 +1,10 @@ +import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '../constants'; + +export default { + isParallelView(state) { + return state.diffViewType === PARALLEL_DIFF_VIEW_TYPE; + }, + isInlineView(state) { + return state.diffViewType === INLINE_DIFF_VIEW_TYPE; + }, +}; diff --git a/app/assets/javascripts/diffs/store/index.js b/app/assets/javascripts/diffs/store/index.js new file mode 100644 index 00000000000..a25851e2f79 --- /dev/null +++ b/app/assets/javascripts/diffs/store/index.js @@ -0,0 +1,17 @@ +import Cookies from 'js-cookie'; +import * as actions from './actions'; +import getters from './getters'; +import mutations from './mutations'; +import { INLINE_DIFF_VIEW_TYPE, DIFF_VIEW_COOKIE_NAME } from '../constants'; + +export default { + state: { + isLoading: true, + endpoint: '', + diffFiles: [], + diffViewType: Cookies.get(DIFF_VIEW_COOKIE_NAME) || INLINE_DIFF_VIEW_TYPE, + }, + getters, + actions, + mutations, +}; diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js new file mode 100644 index 00000000000..805fa510a56 --- /dev/null +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -0,0 +1,7 @@ +export const SET_ENDPOINT = 'SET_ENDPOINT'; +export const SET_LOADING = 'SET_LOADING'; +export const SET_DIFF_FILES = 'SET_DIFF_FILES'; +export const SET_DIFF_VIEW_TYPE = 'SET_DIFF_VIEW_TYPE'; +export const ADD_COMMENT_FORM_LINE = 'ADD_COMMENT_FORM_LINE'; +export const REMOVE_COMMENT_FORM_LINE = 'REMOVE_COMMENT_FORM_LINE'; +export const ADD_CONTEXT_LINES = 'ADD_CONTEXT_LINES'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js new file mode 100644 index 00000000000..8d7c26ed212 --- /dev/null +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -0,0 +1,129 @@ +import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; +import * as utils from './utils'; +import * as types from './mutation_types'; +import * as constants from '../constants'; + +export default { + [types.SET_ENDPOINT](state, endpoint) { + Object.assign(state, { endpoint }); + }, + + [types.SET_LOADING](state, isLoading) { + Object.assign(state, { isLoading }); + }, + + [types.SET_DIFF_FILES](state, diffFiles) { + Object.assign(state, { + diffFiles: convertObjectPropsToCamelCase(diffFiles, { + deep: true, + }), + }); + }, + + [types.SET_DIFF_VIEW_TYPE](state, diffViewType) { + Object.assign(state, { diffViewType }); + }, + + [types.ADD_COMMENT_FORM_LINE](state, { diffLines, lineCode, linePosition }) { + const index = utils.findDiffLineIndex({ + diffLines, + lineCode, + linePosition, + }); + const commentFormType = constants.COMMENT_FORM_TYPE; + + if (!diffLines[index]) { + return; + } + + const item = linePosition + ? diffLines[index][linePosition] + : diffLines[index]; + + if (!item) { + return; + } + + // We add forms as another diff line so they have to have a unique id + // We later use this id to remove the form from diff lines + const id = `${item.lineCode}_CommentForm_${linePosition || ''}`; + const targetIndex = index + 1; + const targetLine = diffLines[targetIndex]; + const atTargetIndex = linePosition ? targetLine[linePosition] : targetLine; + + // We already have comment form for target line + if (atTargetIndex && atTargetIndex.id === id) { + return; + } + + // Unique comment form object as a diff line + const formObj = { + id, + type: commentFormType, + }; + + if (linePosition) { + // linePosition is only valid for Parallel mode + // Create the final lineObj which will represent the forms as a line + // Restore old form in opposite position so we can rerender it + const reversePosition = utils.getReversePosition(linePosition); + const reverseObj = targetLine[reversePosition]; + const lineObj = { + [linePosition]: formObj, + [reversePosition]: + reverseObj.type === commentFormType ? reverseObj : {}, + }; + + // Check if there is any comment form on the target position + // If we have, we should to remove it because above lineObj should be final version + const { left, right } = targetLine; + const hasAlreadyForm = + left.type === commentFormType || right.type === commentFormType; + const spliceCount = hasAlreadyForm ? 1 : 0; + + diffLines.splice(targetIndex, spliceCount, lineObj); + } else { + diffLines.splice(targetIndex, 0, formObj); + } + }, + + [types.REMOVE_COMMENT_FORM_LINE](state, { diffLines, formId, linePosition }) { + const index = utils.findDiffLineIndex({ diffLines, formId, linePosition }); + + if (index > -1) { + if (linePosition) { + const reversePosition = utils.getReversePosition(linePosition); + const line = diffLines[index]; + const reverse = line[reversePosition]; + const shouldRemove = reverse.type !== constants.COMMENT_FORM_TYPE; + + if (shouldRemove) { + diffLines.splice(index, 1); + } else { + Object.assign(line, { + [linePosition]: {}, + }); + } + } else { + diffLines.splice(index, 1); + } + } + }, + + [types.ADD_CONTEXT_LINES](state, options) { + const { lineNumbers, contextLines, fileHash } = options; + const { bottom } = options.params; + const diffFile = utils.findDiffFile(state.diffFiles, fileHash); + const { highlightedDiffLines, parallelDiffLines } = diffFile; + + utils.removeMatchLine(diffFile, lineNumbers, bottom); + const lines = utils.addLineReferences(contextLines, lineNumbers, bottom); + utils.addContextLines({ + inlineLines: highlightedDiffLines, + parallelLines: parallelDiffLines, + contextLines: lines, + bottom, + lineNumbers, + }); + }, +}; diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js new file mode 100644 index 00000000000..f7a4eddf60d --- /dev/null +++ b/app/assets/javascripts/diffs/store/utils.js @@ -0,0 +1,186 @@ +import _ from 'underscore'; +import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; +import { + LINE_POSITION_LEFT, + LINE_POSITION_RIGHT, + TEXT_DIFF_POSITION_TYPE, + DIFF_NOTE_TYPE, + NEW_LINE_TYPE, + OLD_LINE_TYPE, + MATCH_LINE_TYPE, +} from '../constants'; + +export const findDiffLineIndex = options => { + const { diffLines, lineCode, linePosition, formId } = options; + + return _.findIndex(diffLines, l => { + const line = linePosition ? l[linePosition] : l; + + if (!line) { + return null; + } + + if (formId) { + return line.id === formId; + } + + return line.lineCode === lineCode; + }); +}; + +export const findDiffFile = (files, hash) => + files.filter(file => file.fileHash === hash)[0]; + +export const getReversePosition = linePosition => { + if (linePosition === LINE_POSITION_RIGHT) { + return LINE_POSITION_LEFT; + } + + return LINE_POSITION_RIGHT; +}; + +export const getNoteFormData = params => { + const { + note, + noteableType, + noteableData, + diffFile, + noteTargetLine, + diffViewType, + linePosition, + } = params; + + // TODO: Discuss with @felipe_arthur to remove this JSON.stringify + const position = JSON.stringify({ + base_sha: diffFile.diffRefs.baseSha, + start_sha: diffFile.diffRefs.startSha, + head_sha: diffFile.diffRefs.headSha, + old_path: diffFile.oldPath, + new_path: diffFile.newPath, + position_type: TEXT_DIFF_POSITION_TYPE, + old_line: noteTargetLine.oldLine, + new_line: noteTargetLine.newLine, + }); + + // TODO: @fatihacet - Double check empty strings + const postData = { + view: diffViewType, + line_type: + linePosition === LINE_POSITION_RIGHT ? NEW_LINE_TYPE : OLD_LINE_TYPE, + merge_request_diff_head_sha: diffFile.diffRefs.headSha, + in_reply_to_discussion_id: '', + note_project_id: '', + target_type: noteableType, + target_id: noteableData.id, + 'note[noteable_type]': noteableType, + 'note[noteable_id]': noteableData.id, + 'note[commit_id]': '', + 'note[type]': DIFF_NOTE_TYPE, + 'note[line_code]': noteTargetLine.lineCode, + 'note[note]': note, + 'note[position]': position, + }; + + return { + endpoint: noteableData.create_note_path, + data: postData, + }; +}; + +export const findIndexInInlineLines = (lines, lineNumbers) => { + const { oldLineNumber, newLineNumber } = lineNumbers; + + return _.findIndex( + lines, + line => line.oldLine === oldLineNumber && line.newLine === newLineNumber, + ); +}; + +export const findIndexInParallelLines = (lines, lineNumbers) => { + const { oldLineNumber, newLineNumber } = lineNumbers; + + return _.findIndex( + lines, + line => + line.left && + line.right && + line.left.oldLine === oldLineNumber && + line.right.newLine === newLineNumber, + ); +}; + +export const removeMatchLine = (diffFile, lineNumbers, bottom) => { + const indexForInline = findIndexInInlineLines( + diffFile.highlightedDiffLines, + lineNumbers, + ); + + const indexForParallel = findIndexInParallelLines( + diffFile.parallelDiffLines, + lineNumbers, + ); + + const factor = bottom ? 1 : -1; + + diffFile.highlightedDiffLines.splice(indexForInline + factor, 1); + diffFile.parallelDiffLines.splice(indexForParallel + factor, 1); +}; + +export const addLineReferences = (lines, lineNumbers, bottom) => { + const { oldLineNumber, newLineNumber } = lineNumbers; + const lineCount = lines.length; + let matchLineIndex = -1; + + const linesWithNumbers = lines.map((l, index) => { + const line = convertObjectPropsToCamelCase(l); + + if (line.type === MATCH_LINE_TYPE) { + matchLineIndex = index; + } else { + Object.assign(line, { + oldLine: bottom + ? oldLineNumber + index + 1 + : oldLineNumber + index - lineCount, + newLine: bottom + ? newLineNumber + index + 1 + : newLineNumber + index - lineCount, + }); + } + + return line; + }); + + if (matchLineIndex > -1) { + const line = linesWithNumbers[matchLineIndex]; + const targetLine = bottom + ? linesWithNumbers[matchLineIndex - 1] + : linesWithNumbers[matchLineIndex + 1]; + + Object.assign(line, { + metaData: { + oldPos: targetLine.oldLine, + newPos: targetLine.newLine, + }, + }); + } + + return linesWithNumbers; +}; + +export const addContextLines = options => { + const { inlineLines, parallelLines, contextLines, lineNumbers } = options; + const inlineIndex = findIndexInInlineLines(inlineLines, lineNumbers); + const parallelIndex = findIndexInParallelLines(parallelLines, lineNumbers); + const normalizedParallelLines = contextLines.map(line => ({ + left: line, + right: line, + })); + + if (options.bottom) { + inlineLines.push(...contextLines); + parallelLines.push(...normalizedParallelLines); + } else { + inlineLines.splice(inlineIndex, 0, ...contextLines); + parallelLines.splice(parallelIndex, 0, ...normalizedParallelLines); + } +}; diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index 0830ebe9e4e..606c836acee 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -3,8 +3,12 @@ import Cookies from 'js-cookie'; import axios from './axios_utils'; import { getLocationHash } from './url_utility'; import { convertToCamelCase } from './text_utility'; +import { isObject } from './type_utility'; -export const getPagePath = (index = 0) => $('body').attr('data-page').split(':')[index]; +export const getPagePath = (index = 0) => + $('body') + .attr('data-page') + .split(':')[index]; export const isInGroupsPage = () => getPagePath() === 'groups'; @@ -34,32 +38,39 @@ export const checkPageAndAction = (page, action) => { export const isInIssuePage = () => checkPageAndAction('issues', 'show'); export const isInMRPage = () => checkPageAndAction('merge_requests', 'show'); export const isInNoteablePage = () => isInIssuePage() || isInMRPage(); -export const hasVueMRDiscussionsCookie = () => Cookies.get('vue_mr_discussions'); -export const ajaxGet = url => axios.get(url, { - params: { format: 'js' }, - responseType: 'text', -}).then(({ data }) => { - $.globalEval(data); -}); +export const ajaxGet = url => + axios + .get(url, { + params: { format: 'js' }, + responseType: 'text', + }) + .then(({ data }) => { + $.globalEval(data); + }); -export const rstrip = (val) => { +export const rstrip = val => { if (val) { return val.replace(/\s+$/, ''); } return val; }; -export const updateTooltipTitle = ($tooltipEl, newTitle) => $tooltipEl.attr('title', newTitle).tooltip('fixTitle'); +export const updateTooltipTitle = ($tooltipEl, newTitle) => + $tooltipEl.attr('title', newTitle).tooltip('fixTitle'); -export const disableButtonIfEmptyField = (fieldSelector, buttonSelector, eventName = 'input') => { +export const disableButtonIfEmptyField = ( + fieldSelector, + buttonSelector, + eventName = 'input', +) => { const field = $(fieldSelector); const closestSubmit = field.closest('form').find(buttonSelector); if (rstrip(field.val()) === '') { closestSubmit.disable(); } // eslint-disable-next-line func-names - return field.on(eventName, function () { + return field.on(eventName, function() { if (rstrip($(this).val()) === '') { return closestSubmit.disable(); } @@ -76,9 +87,11 @@ export const handleLocationHash = () => { // This is required to handle non-unicode characters in hash hash = decodeURIComponent(hash); - const target = document.getElementById(hash) || document.getElementById(`user-content-${hash}`); + const target = + document.getElementById(hash) || + document.getElementById(`user-content-${hash}`); const fixedTabs = document.querySelector('.js-tabs-affix'); - const fixedDiffStats = document.querySelector('.js-diff-files-changed.is-stuck'); + const fixedDiffStats = document.querySelector('.js-diff-files-changed'); const fixedNav = document.querySelector('.navbar-gitlab'); let adjustment = 0; @@ -101,7 +114,7 @@ export const handleLocationHash = () => { // Check if element scrolled into viewport from above or below // Courtesy http://stackoverflow.com/a/7557433/414749 -export const isInViewport = (el) => { +export const isInViewport = el => { const rect = el.getBoundingClientRect(); return ( @@ -112,25 +125,31 @@ export const isInViewport = (el) => { ); }; -export const parseUrl = (url) => { +export const parseUrl = url => { const parser = document.createElement('a'); parser.href = url; return parser; }; -export const parseUrlPathname = (url) => { +export const parseUrlPathname = url => { const parsedUrl = parseUrl(url); // parsedUrl.pathname will return an absolute path for Firefox and a relative path for IE11 // We have to make sure we always have an absolute path. - return parsedUrl.pathname.charAt(0) === '/' ? parsedUrl.pathname : `/${parsedUrl.pathname}`; + return parsedUrl.pathname.charAt(0) === '/' + ? parsedUrl.pathname + : `/${parsedUrl.pathname}`; }; // We can trust that each param has one & since values containing & will be encoded // Remove the first character of search as it is always ? -export const getUrlParamsArray = () => window.location.search.slice(1).split('&').map((param) => { - const split = param.split('='); - return [decodeURI(split[0]), split[1]].join('='); -}); +export const getUrlParamsArray = () => + window.location.search + .slice(1) + .split('&') + .map(param => { + const split = param.split('='); + return [decodeURI(split[0]), split[1]].join('='); + }); export const isMetaKey = e => e.metaKey || e.ctrlKey || e.altKey || e.shiftKey; @@ -140,7 +159,7 @@ export const isMetaKey = e => e.metaKey || e.ctrlKey || e.altKey || e.shiftKey; // 3) Middle-click or Mouse Wheel Click (e.which is 2) export const isMetaClick = e => e.metaKey || e.ctrlKey || e.which === 2; -export const scrollToElement = (element) => { +export const scrollToElement = element => { let $el = element; if (!(element instanceof $)) { $el = $(element); @@ -149,9 +168,12 @@ export const scrollToElement = (element) => { const mrTabsHeight = $('.merge-request-tabs').height() || 0; const headerHeight = $('.navbar-gitlab').height() || 0; - return $('body, html').animate({ - scrollTop: top - mrTabsHeight - headerHeight, - }, 200); + return $('body, html').animate( + { + scrollTop: top - mrTabsHeight - headerHeight, + }, + 200, + ); }; /** @@ -190,13 +212,15 @@ export const insertText = (target, text) => { const textBefore = value.substring(0, selectionStart); const textAfter = value.substring(selectionEnd, value.length); - const insertedText = text instanceof Function ? text(textBefore, textAfter) : text; + const insertedText = + text instanceof Function ? text(textBefore, textAfter) : text; const newText = textBefore + insertedText + textAfter; // eslint-disable-next-line no-param-reassign target.value = newText; // eslint-disable-next-line no-param-reassign - target.selectionStart = target.selectionEnd = selectionStart + insertedText.length; + target.selectionStart = target.selectionEnd = + selectionStart + insertedText.length; // Trigger autosave target.dispatchEvent(new Event('input')); @@ -208,7 +232,8 @@ export const insertText = (target, text) => { }; export const nodeMatchesSelector = (node, selector) => { - const matches = Element.prototype.matches || + const matches = + Element.prototype.matches || Element.prototype.matchesSelector || Element.prototype.mozMatchesSelector || Element.prototype.msMatchesSelector || @@ -237,10 +262,10 @@ export const nodeMatchesSelector = (node, selector) => { this will take in the headers from an API response and normalize them this way we don't run into production issues when nginx gives us lowercased header keys */ -export const normalizeHeaders = (headers) => { +export const normalizeHeaders = headers => { const upperCaseHeaders = {}; - Object.keys(headers || {}).forEach((e) => { + Object.keys(headers || {}).forEach(e => { upperCaseHeaders[e.toUpperCase()] = headers[e]; }); @@ -251,11 +276,11 @@ export const normalizeHeaders = (headers) => { this will take in the getAllResponseHeaders result and normalize them this way we don't run into production issues when nginx gives us lowercased header keys */ -export const normalizeCRLFHeaders = (headers) => { +export const normalizeCRLFHeaders = headers => { const headersObject = {}; const headersArray = headers.split('\n'); - headersArray.forEach((header) => { + headersArray.forEach(header => { const keyValue = header.split(': '); headersObject[keyValue[0]] = keyValue[1]; }); @@ -291,15 +316,13 @@ export const parseIntPagination = paginationInformation => ({ export const parseQueryStringIntoObject = (query = '') => { if (query === '') return {}; - return query - .split('&') - .reduce((acc, element) => { - const val = element.split('='); - Object.assign(acc, { - [val[0]]: decodeURIComponent(val[1]), - }); - return acc; - }, {}); + return query.split('&').reduce((acc, element) => { + const val = element.split('='); + Object.assign(acc, { + [val[0]]: decodeURIComponent(val[1]), + }); + return acc; + }, {}); }; /** @@ -308,9 +331,13 @@ export const parseQueryStringIntoObject = (query = '') => { * * @param {Object} params */ -export const objectToQueryString = (params = {}) => Object.keys(params).map(param => `${param}=${params[param]}`).join('&'); +export const objectToQueryString = (params = {}) => + Object.keys(params) + .map(param => `${param}=${params[param]}`) + .join('&'); -export const buildUrlWithCurrentLocation = param => (param ? `${window.location.pathname}${param}` : window.location.pathname); +export const buildUrlWithCurrentLocation = param => + param ? `${window.location.pathname}${param}` : window.location.pathname; /** * Based on the current location and the string parameters provided @@ -318,7 +345,7 @@ export const buildUrlWithCurrentLocation = param => (param ? `${window.location. * * @param {String} param */ -export const historyPushState = (newUrl) => { +export const historyPushState = newUrl => { window.history.pushState({}, document.title, newUrl); }; @@ -367,7 +394,7 @@ export const backOff = (fn, timeout = 60000) => { let timeElapsed = 0; return new Promise((resolve, reject) => { - const stop = arg => ((arg instanceof Error) ? reject(arg) : resolve(arg)); + const stop = arg => (arg instanceof Error ? reject(arg) : resolve(arg)); const next = () => { if (timeElapsed < timeout) { @@ -383,7 +410,7 @@ export const backOff = (fn, timeout = 60000) => { }); }; -export const setFavicon = (faviconPath) => { +export const setFavicon = faviconPath => { const faviconEl = document.getElementById('favicon'); if (faviconEl && faviconPath) { faviconEl.setAttribute('href', faviconPath); @@ -399,7 +426,8 @@ export const resetFavicon = () => { }; export const setCiStatusFavicon = pageUrl => - axios.get(pageUrl) + axios + .get(pageUrl) .then(({ data }) => { if (data && data.favicon) { setFavicon(data.favicon); @@ -412,7 +440,9 @@ export const setCiStatusFavicon = pageUrl => export const spriteIcon = (icon, className = '') => { const classAttribute = className.length > 0 ? `class="${className}"` : ''; - return `<svg ${classAttribute}><use xlink:href="${gon.sprite_icons}#${icon}" /></svg>`; + return `<svg ${classAttribute}><use xlink:href="${ + gon.sprite_icons + }#${icon}" /></svg>`; }; /** @@ -422,28 +452,41 @@ export const spriteIcon = (icon, className = '') => { * Reasoning for this method is to ensure consistent property * naming conventions across JS code. */ -export const convertObjectPropsToCamelCase = (obj = {}) => { +export const convertObjectPropsToCamelCase = (obj = {}, options = {}) => { if (obj === null) { return {}; } + const initial = Array.isArray(obj) ? [] : {}; + return Object.keys(obj).reduce((acc, prop) => { const result = acc; - - result[convertToCamelCase(prop)] = obj[prop]; + const val = obj[prop]; + + if (options.deep && (isObject(val) || Array.isArray(val))) { + result[convertToCamelCase(prop)] = convertObjectPropsToCamelCase( + val, + options, + ); + } else { + result[convertToCamelCase(prop)] = obj[prop]; + } return acc; - }, {}); + }, initial); }; -export const imagePath = imgUrl => `${gon.asset_host || ''}${gon.relative_url_root || ''}/assets/${imgUrl}`; +export const imagePath = imgUrl => + `${gon.asset_host || ''}${gon.relative_url_root || ''}/assets/${imgUrl}`; export const addSelectOnFocusBehaviour = (selector = '.js-select-on-focus') => { // Click a .js-select-on-focus field, select the contents // Prevent a mouseup event from deselecting the input $(selector).on('focusin', function selectOnFocusCallback() { - $(this).select().one('mouseup', (e) => { - e.preventDefault(); - }); + $(this) + .select() + .one('mouseup', e => { + e.preventDefault(); + }); }); }; diff --git a/app/assets/javascripts/merge_request.js b/app/assets/javascripts/merge_request.js index d8222ebec63..d8960c212b4 100644 --- a/app/assets/javascripts/merge_request.js +++ b/app/assets/javascripts/merge_request.js @@ -49,6 +49,7 @@ MergeRequest.prototype.initTabs = function() { if (window.mrTabs) { window.mrTabs.unbindEvents(); } + window.mrTabs = new MergeRequestTabs(this.opts); }; diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index e77318fef46..9f6af8c7a0d 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -1,6 +1,7 @@ /* eslint-disable no-new, class-methods-use-this */ import $ from 'jquery'; +import Vue from 'vue'; import Cookies from 'js-cookie'; import axios from './lib/utils/axios_utils'; import flash from './flash'; @@ -69,17 +70,18 @@ import Notes from './notes'; let location = window.location; export default class MergeRequestTabs { - constructor({ action, setUrl, stubLocation } = {}) { const mergeRequestTabs = document.querySelector('.js-tabs-affix'); const navbar = document.querySelector('.navbar-gitlab'); const peek = document.getElementById('peek'); const paddingTop = 16; + this.commitsTab = document.querySelector('.tab-content .commits.tab-pane'); this.diffsLoaded = false; this.pipelinesLoaded = false; this.commitsLoaded = false; this.fixedLayoutPref = null; + this.eventHub = new Vue(); this.setUrl = setUrl !== undefined ? setUrl : true; this.setCurrentAction = this.setCurrentAction.bind(this); @@ -106,21 +108,27 @@ export default class MergeRequestTabs { bindEvents() { $(document) - .on('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown) + .on( + 'shown.bs.tab', + '.merge-request-tabs a[data-toggle="tab"]', + this.tabShown, + ) .on('click', '.js-show-tab', this.showTab); - $('.merge-request-tabs a[data-toggle="tab"]') - .on('click', this.clickTab); + $('.merge-request-tabs a[data-toggle="tab"]').on('click', this.clickTab); } // Used in tests unbindEvents() { $(document) - .off('shown.bs.tab', '.merge-request-tabs a[data-toggle="tab"]', this.tabShown) + .off( + 'shown.bs.tab', + '.merge-request-tabs a[data-toggle="tab"]', + this.tabShown, + ) .off('click', '.js-show-tab', this.showTab); - $('.merge-request-tabs a[data-toggle="tab"]') - .off('click', this.clickTab); + $('.merge-request-tabs a[data-toggle="tab"]').off('click', this.clickTab); } destroyPipelinesView() { @@ -156,7 +164,6 @@ export default class MergeRequestTabs { this.resetViewContainer(); this.destroyPipelinesView(); } else if (this.isDiffAction(action)) { - this.loadDiff($target.attr('href')); if (bp.getBreakpointSize() !== 'lg') { this.shrinkView(); } @@ -164,6 +171,7 @@ export default class MergeRequestTabs { this.expandViewContainer(); } this.destroyPipelinesView(); + this.commitsTab.classList.remove('active'); } else if (action === 'pipelines') { this.resetViewContainer(); this.mountPipelinesView(); @@ -179,14 +187,15 @@ export default class MergeRequestTabs { if (this.setUrl) { this.setCurrentAction(action); } + + this.eventHub.$emit('MergeRequestTabChange', this.getCurrentAction()); } scrollToElement(container) { if (location.hash) { - const offset = 0 - ( - $('.navbar-gitlab').outerHeight() + - $('.js-tabs-affix').outerHeight() - ); + const offset = + 0 - + ($('.navbar-gitlab').outerHeight() + $('.js-tabs-affix').outerHeight()); const $el = $(`${container} ${location.hash}:not(.match)`); if ($el.length) { $.scrollTo($el[0], { offset }); @@ -224,7 +233,10 @@ export default class MergeRequestTabs { this.currentAction = action; // Remove a trailing '/commits' '/diffs' '/pipelines' - let newState = location.pathname.replace(/\/(commits|diffs|pipelines)(\.html)?\/?$/, ''); + let newState = location.pathname.replace( + /\/(commits|diffs|pipelines)(\.html)?\/?$/, + '', + ); // Append the new action if we're on a tab other than 'notes' if (this.currentAction !== 'show' && this.currentAction !== 'new') { @@ -240,9 +252,13 @@ export default class MergeRequestTabs { // Turbolinks' history. // // See https://github.com/rails/turbolinks/issues/363 - window.history.replaceState({ - url: newState, - }, document.title, newState); + window.history.replaceState( + { + url: newState, + }, + document.title, + newState, + ); return newState; } @@ -258,7 +274,8 @@ export default class MergeRequestTabs { this.toggleLoading(true); - axios.get(`${source}.json`) + axios + .get(`${source}.json`) .then(({ data }) => { document.querySelector('div#commits').innerHTML = data.html; localTimeAgo($('.js-timeago', 'div#commits')); @@ -274,7 +291,9 @@ export default class MergeRequestTabs { } mountPipelinesView() { - const pipelineTableViewEl = document.querySelector('#commit-pipeline-table-view'); + const pipelineTableViewEl = document.querySelector( + '#commit-pipeline-table-view', + ); const CommitPipelinesTable = gl.CommitPipelinesTable; this.commitPipelinesTable = new CommitPipelinesTable({ propsData: { @@ -291,6 +310,8 @@ export default class MergeRequestTabs { pipelineTableViewEl.appendChild(this.commitPipelinesTable.$el); } + // TODO: @fatihacet + // Delete this method later. It's not being called anymore but here for reference for refactor. loadDiff(source) { if (this.diffsLoaded) { document.dispatchEvent(new CustomEvent('scroll')); @@ -303,7 +324,8 @@ export default class MergeRequestTabs { this.toggleLoading(true); - axios.get(`${urlPathname}.json${location.search}`) + axios + .get(`${urlPathname}.json${location.search}`) .then(({ data }) => { const $container = $('#diffs'); $container.html(data.html); @@ -317,7 +339,10 @@ export default class MergeRequestTabs { localTimeAgo($('.js-timeago', 'div#diffs')); syntaxHighlight($('#diffs .js-syntax-highlight')); - if (this.diffViewType() === 'parallel' && this.isDiffAction(this.currentAction)) { + if ( + this.diffViewType() === 'parallel' && + this.isDiffAction(this.currentAction) + ) { this.expandViewContainer(); } this.diffsLoaded = true; @@ -331,9 +356,10 @@ export default class MergeRequestTabs { forkButtons: $(el).find('.js-fork-suggestion-button'), cancelButtons: $(el).find('.js-cancel-fork-suggestion-button'), suggestionSections: $(el).find('.js-file-fork-suggestion-section'), - actionTextPieces: $(el).find('.js-file-fork-suggestion-section-action'), - }) - .init(); + actionTextPieces: $(el).find( + '.js-file-fork-suggestion-section-action', + ), + }).init(); }); // Scroll any linked note into view @@ -388,8 +414,10 @@ export default class MergeRequestTabs { resetViewContainer() { if (this.fixedLayoutPref !== null) { - $('.content-wrapper .container-fluid') - .toggleClass('container-limited', this.fixedLayoutPref); + $('.content-wrapper .container-fluid').toggleClass( + 'container-limited', + this.fixedLayoutPref, + ); } } @@ -438,12 +466,12 @@ export default class MergeRequestTabs { const $diffTabs = $('#diff-notes-app'); - $tabs.off('affix.bs.affix affix-top.bs.affix') + $tabs + .off('affix.bs.affix affix-top.bs.affix') .affix({ offset: { - top: () => ( - $diffTabs.offset().top - $tabs.height() - $fixedNav.height() - ), + top: () => + $diffTabs.offset().top - $tabs.height() - $fixedNav.height(), }, }) .on('affix.bs.affix', () => $diffTabs.css({ marginTop: $tabs.height() })) diff --git a/app/assets/javascripts/mr_notes/index.js b/app/assets/javascripts/mr_notes/index.js index 096c4ef5f31..c09952364f4 100644 --- a/app/assets/javascripts/mr_notes/index.js +++ b/app/assets/javascripts/mr_notes/index.js @@ -1,30 +1,70 @@ import Vue from 'vue'; +import Vuex, { mapActions, mapState } from 'vuex'; import notesApp from '../notes/components/notes_app.vue'; +import diffsApp from '../diffs/components/app.vue'; import discussionCounter from '../notes/components/discussion_counter.vue'; -import store from '../notes/stores'; +import notesStoreConfig from '../notes/stores'; +import diffsStoreConfig from '../diffs/store'; +import mrPageStoreConfig from './stores'; +import MergeRequest from '../merge_request'; + +Vue.use(Vuex); + +const store = new Vuex.Store({ + modules: { + page: mrPageStoreConfig, + notes: notesStoreConfig, + diffs: diffsStoreConfig, + }, +}); export default function initMrNotes() { + const mrShowNode = document.querySelector('.merge-request'); + // eslint-disable-next-line no-new + new MergeRequest({ + action: mrShowNode.dataset.mrAction, + }); + // eslint-disable-next-line no-new new Vue({ el: '#js-vue-mr-discussions', + name: 'MergeRequestDiscussions', components: { notesApp, }, + store, data() { const notesDataset = document.getElementById('js-vue-mr-discussions') .dataset; + return { noteableData: JSON.parse(notesDataset.noteableData), currentUserData: JSON.parse(notesDataset.currentUserData), notesData: JSON.parse(notesDataset.notesData), }; }, + computed: { + ...mapState({ + activeTab: state => state.page.activeTab, + }), + }, + mounted() { + this.setActiveTab(window.mrTabs.getCurrentAction()); + + window.mrTabs.eventHub.$on('MergeRequestTabChange', tab => { + this.setActiveTab(tab); + }); + }, + methods: { + ...mapActions(['setActiveTab']), + }, render(createElement) { return createElement('notes-app', { props: { noteableData: this.noteableData, notesData: this.notesData, userData: this.currentUserData, + shouldShow: this.activeTab === 'show', }, }); }, @@ -33,6 +73,7 @@ export default function initMrNotes() { // eslint-disable-next-line no-new new Vue({ el: '#js-vue-discussion-counter', + name: 'DiscussionCounter', components: { discussionCounter, }, @@ -41,4 +82,34 @@ export default function initMrNotes() { return createElement('discussion-counter'); }, }); + + // eslint-disable-next-line no-new + new Vue({ + el: '#js-diffs-app', + name: 'DiffsApp', + components: { + diffsApp, + }, + store, + data() { + const { dataset } = document.querySelector(this.$options.el); + + return { + endpoint: dataset.endpoint, + }; + }, + computed: { + ...mapState({ + activeTab: state => state.page.activeTab, + }), + }, + render(createElement) { + return createElement('diffs-app', { + props: { + endpoint: this.endpoint, + shouldShow: this.activeTab === 'diffs', + }, + }); + }, + }); } diff --git a/app/assets/javascripts/mr_notes/stores/actions.js b/app/assets/javascripts/mr_notes/stores/actions.js new file mode 100644 index 00000000000..426c6a00d5e --- /dev/null +++ b/app/assets/javascripts/mr_notes/stores/actions.js @@ -0,0 +1,7 @@ +import types from './mutation_types'; + +export default { + setActiveTab({ commit }, tab) { + commit(types.SET_ACTIVE_TAB, tab); + }, +}; diff --git a/app/assets/javascripts/mr_notes/stores/getters.js b/app/assets/javascripts/mr_notes/stores/getters.js new file mode 100644 index 00000000000..b10e9f9f9f1 --- /dev/null +++ b/app/assets/javascripts/mr_notes/stores/getters.js @@ -0,0 +1,5 @@ +export default { + isLoggedIn(state, getters) { + return !!getters.getUserData.id; + }, +}; diff --git a/app/assets/javascripts/mr_notes/stores/index.js b/app/assets/javascripts/mr_notes/stores/index.js new file mode 100644 index 00000000000..3284bc08c70 --- /dev/null +++ b/app/assets/javascripts/mr_notes/stores/index.js @@ -0,0 +1,12 @@ +import actions from './actions'; +import getters from './getters'; +import mutations from './mutations'; + +export default { + state: { + activeTab: null, + }, + actions, + getters, + mutations, +}; diff --git a/app/assets/javascripts/mr_notes/stores/mutation_types.js b/app/assets/javascripts/mr_notes/stores/mutation_types.js new file mode 100644 index 00000000000..105104361cf --- /dev/null +++ b/app/assets/javascripts/mr_notes/stores/mutation_types.js @@ -0,0 +1,3 @@ +export default { + SET_ACTIVE_TAB: 'SET_ACTIVE_TAB', +}; diff --git a/app/assets/javascripts/mr_notes/stores/mutations.js b/app/assets/javascripts/mr_notes/stores/mutations.js new file mode 100644 index 00000000000..8175aa9488f --- /dev/null +++ b/app/assets/javascripts/mr_notes/stores/mutations.js @@ -0,0 +1,7 @@ +import types from './mutation_types'; + +export default { + [types.SET_ACTIVE_TAB](state, tab) { + Object.assign(state, { activeTab: tab }); + }, +}; diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index b0573510ff9..e2b3f6b1806 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -33,7 +33,7 @@ import { getPagePath, scrollToElement, isMetaKey, - hasVueMRDiscussionsCookie, + isInMRPage, } from './lib/utils/common_utils'; import imageDiffHelper from './image_diff/helpers/index'; import { localTimeAgo } from './lib/utils/datetime_utility'; @@ -48,21 +48,9 @@ const MAX_VISIBLE_COMMIT_LIST_COUNT = 3; const REGEX_QUICK_ACTIONS = /^\/\w+.*$/gm; export default class Notes { - static initialize( - notes_url, - note_ids, - last_fetched_at, - view, - enableGFM = true, - ) { + static initialize(notes_url, note_ids, last_fetched_at, view, enableGFM = true) { if (!this.instance) { - this.instance = new Notes( - notes_url, - note_ids, - last_fetched_at, - view, - enableGFM, - ); + this.instance = new Notes(notes_url, note_ids, last_fetched_at, view, enableGFM); } } @@ -105,9 +93,7 @@ export default class Notes { this.basePollingInterval = 15000; this.maxPollingSteps = 4; - this.$wrapperEl = hasVueMRDiscussionsCookie() - ? $(document).find('.diffs') - : $(document); + this.$wrapperEl = isInMRPage() ? $(document).find('.diffs') : $(document); this.cleanBinding(); this.addBinding(); this.setPollingInterval(); @@ -147,55 +133,27 @@ export default class Notes { // Reopen and close actions for Issue/MR combined with note form submit this.$wrapperEl.on('click', '.js-comment-submit-button', this.postComment); this.$wrapperEl.on('click', '.js-comment-save-button', this.updateComment); - this.$wrapperEl.on( - 'keyup input', - '.js-note-text', - this.updateTargetButtons, - ); + this.$wrapperEl.on('keyup input', '.js-note-text', this.updateTargetButtons); // resolve a discussion this.$wrapperEl.on('click', '.js-comment-resolve-button', this.postComment); // remove a note (in general) this.$wrapperEl.on('click', '.js-note-delete', this.removeNote); // delete note attachment - this.$wrapperEl.on( - 'click', - '.js-note-attachment-delete', - this.removeAttachment, - ); + this.$wrapperEl.on('click', '.js-note-attachment-delete', this.removeAttachment); // reset main target form when clicking discard this.$wrapperEl.on('click', '.js-note-discard', this.resetMainTargetForm); // update the file name when an attachment is selected - this.$wrapperEl.on( - 'change', - '.js-note-attachment-input', - this.updateFormAttachment, - ); + this.$wrapperEl.on('change', '.js-note-attachment-input', this.updateFormAttachment); // reply to diff/discussion notes - this.$wrapperEl.on( - 'click', - '.js-discussion-reply-button', - this.onReplyToDiscussionNote, - ); + this.$wrapperEl.on('click', '.js-discussion-reply-button', this.onReplyToDiscussionNote); // add diff note this.$wrapperEl.on('click', '.js-add-diff-note-button', this.onAddDiffNote); // add diff note for images - this.$wrapperEl.on( - 'click', - '.js-add-image-diff-note-button', - this.onAddImageDiffNote, - ); + this.$wrapperEl.on('click', '.js-add-image-diff-note-button', this.onAddImageDiffNote); // hide diff note form - this.$wrapperEl.on( - 'click', - '.js-close-discussion-note-form', - this.cancelDiscussionForm, - ); + this.$wrapperEl.on('click', '.js-close-discussion-note-form', this.cancelDiscussionForm); // toggle commit list - this.$wrapperEl.on( - 'click', - '.system-note-commit-list-toggler', - this.toggleCommitList, - ); + this.$wrapperEl.on('click', '.system-note-commit-list-toggler', this.toggleCommitList); this.$wrapperEl.on('click', '.js-toggle-lazy-diff', this.loadLazyDiff); // fetch notes when tab becomes visible @@ -204,16 +162,8 @@ export default class Notes { this.$wrapperEl.on('issuable:change', this.refresh); // ajax:events that happen on Form when actions like Reopen, Close are performed on Issues and MRs. this.$wrapperEl.on('ajax:success', '.js-main-target-form', this.addNote); - this.$wrapperEl.on( - 'ajax:success', - '.js-discussion-note-form', - this.addDiscussionNote, - ); - this.$wrapperEl.on( - 'ajax:success', - '.js-main-target-form', - this.resetMainTargetForm, - ); + this.$wrapperEl.on('ajax:success', '.js-discussion-note-form', this.addDiscussionNote); + this.$wrapperEl.on('ajax:success', '.js-main-target-form', this.resetMainTargetForm); this.$wrapperEl.on( 'ajax:complete', '.js-main-target-form', @@ -223,8 +173,6 @@ export default class Notes { this.$wrapperEl.on('keydown', '.js-note-text', this.keydownNoteText); // When the URL fragment/hash has changed, `#note_xxx` $(window).on('hashchange', this.onHashChange); - this.boundGetContent = this.getContent.bind(this); - document.addEventListener('refreshLegacyNotes', this.boundGetContent); } cleanBinding() { @@ -247,21 +195,14 @@ export default class Notes { this.$wrapperEl.off('ajax:success', '.js-main-target-form'); this.$wrapperEl.off('ajax:success', '.js-discussion-note-form'); this.$wrapperEl.off('ajax:complete', '.js-main-target-form'); - document.removeEventListener('refreshLegacyNotes', this.boundGetContent); $(window).off('hashchange', this.onHashChange); } static initCommentTypeToggle(form) { - const dropdownTrigger = form.querySelector( - '.js-comment-type-dropdown .dropdown-toggle', - ); - const dropdownList = form.querySelector( - '.js-comment-type-dropdown .dropdown-menu', - ); + const dropdownTrigger = form.querySelector('.js-comment-type-dropdown .dropdown-toggle'); + const dropdownList = form.querySelector('.js-comment-type-dropdown .dropdown-menu'); const noteTypeInput = form.querySelector('#note_type'); - const submitButton = form.querySelector( - '.js-comment-type-dropdown .js-comment-submit-button', - ); + const submitButton = form.querySelector('.js-comment-type-dropdown .js-comment-submit-button'); const closeButton = form.querySelector('.js-note-target-close'); const reopenButton = form.querySelector('.js-note-target-reopen'); @@ -297,9 +238,7 @@ export default class Notes { return; } myLastNote = $( - `li.note[data-author-id='${ - gon.current_user_id - }'][data-editable]:last`, + `li.note[data-author-id='${gon.current_user_id}'][data-editable]:last`, $textarea.closest('.note, .notes_holder, #notes'), ); if (myLastNote.length) { @@ -312,9 +251,7 @@ export default class Notes { discussionNoteForm = $textarea.closest('.js-discussion-note-form'); if (discussionNoteForm.length) { if ($textarea.val() !== '') { - if ( - !confirm('Are you sure you want to cancel creating this comment?') - ) { + if (!confirm('Are you sure you want to cancel creating this comment?')) { return; } } @@ -326,9 +263,7 @@ export default class Notes { originalText = $textarea.closest('form').data('originalNote'); newText = $textarea.val(); if (originalText !== newText) { - if ( - !confirm('Are you sure you want to cancel editing this comment?') - ) { + if (!confirm('Are you sure you want to cancel editing this comment?')) { return; } } @@ -396,8 +331,7 @@ export default class Notes { if (shouldReset == null) { shouldReset = true; } - nthInterval = - this.basePollingInterval * Math.pow(2, this.maxPollingSteps - 1); + nthInterval = this.basePollingInterval * Math.pow(2, this.maxPollingSteps - 1); if (shouldReset) { this.pollingInterval = this.basePollingInterval; } else if (this.pollingInterval < nthInterval) { @@ -418,10 +352,7 @@ export default class Notes { loadAwardsHandler() .then(awardsHandler => { - awardsHandler.addAwardToEmojiBar( - votesBlock, - noteEntity.commands_changes.emoji_award, - ); + awardsHandler.addAwardToEmojiBar(votesBlock, noteEntity.commands_changes.emoji_award); awardsHandler.scrollToAwards(); }) .catch(() => { @@ -471,17 +402,10 @@ export default class Notes { if (!noteEntity.valid) { if (noteEntity.errors && noteEntity.errors.commands_only) { - if ( - noteEntity.commands_changes && - Object.keys(noteEntity.commands_changes).length > 0 - ) { + if (noteEntity.commands_changes && Object.keys(noteEntity.commands_changes).length > 0) { $notesList.find('.system-note.being-posted').remove(); } - this.addFlash( - noteEntity.errors.commands_only, - 'notice', - this.parentTimeline.get(0), - ); + this.addFlash(noteEntity.errors.commands_only, 'notice', this.parentTimeline.get(0)); this.refresh(); } return; @@ -489,7 +413,7 @@ export default class Notes { const $note = $notesList.find(`#note_${noteEntity.id}`); if (Notes.isNewNote(noteEntity, this.note_ids)) { - if (hasVueMRDiscussionsCookie()) { + if (isInMRPage()) { return; } @@ -517,8 +441,7 @@ export default class Notes { // There can be CRLF vs LF mismatches if we don't sanitize and compare the same way const sanitizedNoteNote = normalizeNewlines(noteEntity.note); const isTextareaUntouched = - currentContent === initialContent || - currentContent === sanitizedNoteNote; + currentContent === initialContent || currentContent === sanitizedNoteNote; if (isEditing && isTextareaUntouched) { $textarea.val(noteEntity.note); @@ -531,8 +454,6 @@ export default class Notes { this.setupNewNote($updatedNote); } } - - Notes.refreshVueNotes(); } isParallelView() { @@ -550,13 +471,7 @@ export default class Notes { } this.note_ids.push(noteEntity.id); - form = - $form || - $( - `.js-discussion-note-form[data-discussion-id="${ - noteEntity.discussion_id - }"]`, - ); + form = $form || $(`.js-discussion-note-form[data-discussion-id="${noteEntity.discussion_id}"]`); row = form.length || !noteEntity.discussion_line_code ? form.closest('tr') @@ -572,9 +487,7 @@ export default class Notes { .first() .find('.js-avatar-container.' + lineType + '_line'); // is this the first note of discussion? - discussionContainer = $( - `.notes[data-discussion-id="${noteEntity.discussion_id}"]`, - ); + discussionContainer = $(`.notes[data-discussion-id="${noteEntity.discussion_id}"]`); if (!discussionContainer.length) { discussionContainer = form.closest('.discussion').find('.notes'); } @@ -582,18 +495,12 @@ export default class Notes { if (noteEntity.diff_discussion_html) { var $discussion = $(noteEntity.diff_discussion_html).renderGFM(); - if ( - !this.isParallelView() || - row.hasClass('js-temp-notes-holder') || - noteEntity.on_image - ) { + if (!this.isParallelView() || row.hasClass('js-temp-notes-holder') || noteEntity.on_image) { // insert the note and the reply button after the temp row row.after($discussion); } else { // Merge new discussion HTML in - var $notes = $discussion.find( - `.notes[data-discussion-id="${noteEntity.discussion_id}"]`, - ); + var $notes = $discussion.find(`.notes[data-discussion-id="${noteEntity.discussion_id}"]`); var contentContainerClass = '.' + $notes @@ -607,28 +514,12 @@ export default class Notes { .append($notes.closest('.content').children()); } } - // Init discussion on 'Discussion' page if it is merge request page - const page = $('body').attr('data-page'); - if ( - (page && page.indexOf('projects:merge_request') !== -1) || - !noteEntity.diff_discussion_html - ) { - if (!hasVueMRDiscussionsCookie()) { - Notes.animateAppendNote( - noteEntity.discussion_html, - $('.main-notes-list'), - ); - } - } } else { // append new note to all matching discussions Notes.animateAppendNote(noteEntity.html, discussionContainer); } - if ( - typeof gl.diffNotesCompileComponents !== 'undefined' && - noteEntity.discussion_resolvable - ) { + if (typeof gl.diffNotesCompileComponents !== 'undefined' && noteEntity.discussion_resolvable) { gl.diffNotesCompileComponents(); this.renderDiscussionAvatar(diffAvatarContainer, noteEntity); @@ -937,9 +828,7 @@ export default class Notes { form.removeClass('current-note-edit-form'); form.find('.js-finish-edit-warning').hide(); // Replace markdown textarea text with original note text. - return form - .find('.js-note-text') - .val(form.find('form.edit-note').data('originalNote')); + return form.find('.js-note-text').val(form.find('form.edit-note').data('originalNote')); } /** @@ -987,21 +876,15 @@ export default class Notes { // The notes tr can contain multiple lists of notes, like on the parallel diff // notesTr does not exist for image diffs - if ( - notesTr.find('.discussion-notes').length > 1 || - notesTr.length === 0 - ) { + if (notesTr.find('.discussion-notes').length > 1 || notesTr.length === 0) { const $diffFile = $notes.closest('.diff-file'); if ($diffFile.length > 0) { - const removeBadgeEvent = new CustomEvent( - 'removeBadge.imageDiff', - { - detail: { - // badgeNumber's start with 1 and index starts with 0 - badgeNumber: $notes.index() + 1, - }, + const removeBadgeEvent = new CustomEvent('removeBadge.imageDiff', { + detail: { + // badgeNumber's start with 1 and index starts with 0 + badgeNumber: $notes.index() + 1, }, - ); + }); $diffFile[0].dispatchEvent(removeBadgeEvent); } @@ -1015,7 +898,6 @@ export default class Notes { })(this), ); - Notes.refreshVueNotes(); Notes.checkMergeRequestStatus(); return this.updateNotesCount(-1); } @@ -1031,7 +913,7 @@ export default class Notes { $note.find('.note-attachment').remove(); $note.find('.note-body > .note-text').show(); $note.find('.note-header').show(); - return $note.find('.current-note-edit-form').remove(); + return $note.find('.diffs .current-note-edit-form').remove(); } /** @@ -1105,9 +987,7 @@ export default class Notes { form.find('.js-note-new-discussion').remove(); this.setupNoteForm(form); - form - .removeClass('js-main-target-form') - .addClass('discussion-form js-discussion-note-form'); + form.removeClass('js-main-target-form').addClass('discussion-form js-discussion-note-form'); if (typeof gl.diffNotesCompileComponents !== 'undefined') { var $commentBtn = form.find('comment-and-resolve-btn'); @@ -1117,9 +997,7 @@ export default class Notes { } form.find('.js-note-text').focus(); - form - .find('.js-comment-resolve-button') - .attr('data-discussion-id', discussionID); + form.find('.js-comment-resolve-button').attr('data-discussion-id', discussionID); } /** @@ -1152,9 +1030,7 @@ export default class Notes { // Setup comment form let newForm; - const $noteContainer = $link - .closest('.diff-viewer') - .find('.note-container'); + const $noteContainer = $link.closest('.diff-viewer').find('.note-container'); const $form = $noteContainer.find('> .discussion-form'); if ($form.length === 0) { @@ -1223,9 +1099,7 @@ export default class Notes { notesContent = targetRow.find(notesContentSelector); addForm = true; } else { - const isCurrentlyShown = targetRow - .find('.content:not(:empty)') - .is(':visible'); + const isCurrentlyShown = targetRow.find('.content:not(:empty)').is(':visible'); const isForced = forceShow === true || forceShow === false; const showNow = forceShow === true || (!isCurrentlyShown && !isForced); @@ -1390,9 +1264,7 @@ export default class Notes { if ($note.find('.js-conflict-edit-warning').length === 0) { const $alert = $(`<div class="js-conflict-edit-warning alert alert-danger"> This comment has changed since you started editing, please review the - <a href="#note_${ - noteEntity.id - }" target="_blank" rel="noopener noreferrer"> + <a href="#note_${noteEntity.id}" target="_blank" rel="noopener noreferrer"> updated comment </a> to ensure information is not lost @@ -1402,9 +1274,7 @@ export default class Notes { } updateNotesCount(updateCount) { - return this.notesCountBadge.text( - parseInt(this.notesCountBadge.text(), 10) + updateCount, - ); + return this.notesCountBadge.text(parseInt(this.notesCountBadge.text(), 10) + updateCount); } static renderPlaceholderComponent($container) { @@ -1467,9 +1337,7 @@ export default class Notes { toggleCommitList(e) { const $element = $(e.currentTarget); - const $closestSystemCommitList = $element.siblings( - '.system-note-commit-list', - ); + const $closestSystemCommitList = $element.siblings('.system-note-commit-list'); $element .find('.fa') @@ -1502,9 +1370,7 @@ export default class Notes { $systemNote.find('.note-text').addClass('system-note-commit-list'); $systemNote.find('.system-note-commit-list-toggler').show(); } else { - $systemNote - .find('.note-text') - .addClass('system-note-commit-list hide-shade'); + $systemNote.find('.note-text').addClass('system-note-commit-list hide-shade'); } }); } @@ -1575,10 +1441,6 @@ export default class Notes { return $updatedNote; } - static refreshVueNotes() { - document.dispatchEvent(new CustomEvent('refreshVueNotes')); - } - /** * Get data from Form attributes to use for saving/submitting comment. */ @@ -1659,12 +1521,8 @@ export default class Notes { <div class="note-header"> <div class="note-header-info"> <a href="/${_.escape(currentUsername)}"> - <span class="hidden-xs">${_.escape( - currentUsername, - )}</span> - <span class="note-headline-light">${_.escape( - currentUsername, - )}</span> + <span class="hidden-xs">${_.escape(currentUsername)}</span> + <span class="note-headline-light">${_.escape(currentUsername)}</span> </a> </div> </div> @@ -1679,9 +1537,7 @@ export default class Notes { ); $tempNote.find('.hidden-xs').text(_.escape(currentUserFullname)); - $tempNote - .find('.note-headline-light') - .text(`@${_.escape(currentUsername)}`); + $tempNote.find('.note-headline-light').text(`@${_.escape(currentUsername)}`); return $tempNote; } @@ -1737,15 +1593,8 @@ export default class Notes { .attr('id') === 'discussion'; const isMainForm = $form.hasClass('js-main-target-form'); const isDiscussionForm = $form.hasClass('js-discussion-note-form'); - const isDiscussionResolve = $submitBtn.hasClass( - 'js-comment-resolve-button', - ); - const { - formData, - formContent, - formAction, - formContentOriginal, - } = this.getFormData($form); + const isDiscussionResolve = $submitBtn.hasClass('js-comment-resolve-button'); + const { formData, formContent, formAction, formContentOriginal } = this.getFormData($form); let noteUniqueId; let systemNoteUniqueId; let hasQuickActions = false; @@ -1833,9 +1682,7 @@ export default class Notes { // Reset cached commands list when command is applied if (hasQuickActions) { - $form - .find('textarea.js-note-text') - .trigger('clear-commands-cache.atwho'); + $form.find('textarea.js-note-text').trigger('clear-commands-cache.atwho'); } // Clear previous form errors @@ -1880,12 +1727,8 @@ export default class Notes { // append flash-container to the Notes list if ($notesContainer.length) { - $notesContainer.append( - '<div class="flash-container" style="display: none;"></div>', - ); + $notesContainer.append('<div class="flash-container" style="display: none;"></div>'); } - - Notes.refreshVueNotes(); } else if (isMainForm) { // Check if this was main thread comment // Show final note element on UI and perform form and action buttons cleanup @@ -1919,9 +1762,7 @@ export default class Notes { // Show form again on UI on failure if (isDiscussionForm && $notesContainer.length) { - const replyButton = $notesContainer - .parent() - .find('.js-discussion-reply-button'); + const replyButton = $notesContainer.parent().find('.js-discussion-reply-button'); this.replyToDiscussionNote(replyButton[0]); $form = $notesContainer.parent().find('form'); } @@ -1964,9 +1805,7 @@ export default class Notes { // Show updated comment content temporarily $noteBodyText.html(formContent); - $editingNote - .removeClass('is-editing fade-in-full') - .addClass('being-posted fade-in-half'); + $editingNote.removeClass('is-editing fade-in-full').addClass('being-posted fade-in-half'); $editingNote .find('.note-headline-meta a') .html( diff --git a/app/assets/javascripts/notes/components/diff_file_header.vue b/app/assets/javascripts/notes/components/diff_file_header.vue index 94d9dc69964..5cc1367600e 100644 --- a/app/assets/javascripts/notes/components/diff_file_header.vue +++ b/app/assets/javascripts/notes/components/diff_file_header.vue @@ -12,17 +12,71 @@ export default { type: Object, required: true, }, + collapsible: { + type: Boolean, + required: false, + default: false, + }, + addMergeRequestButtons: { + type: Boolean, + required: false, + default: false, + }, }, computed: { titleTag() { - return this.diffFile.discussionPath ? 'a' : 'span'; + return this.diffFile.fileHash ? 'a' : 'span'; + }, + baseSha() { + return this.diffFile.diffRefs.baseSha; + }, + contentSha() { + return this.diffFile.contentSha; + }, + truncatedBaseSha() { + return this.truncate(this.baseSha); + }, + truncatedContentSha() { + return this.truncate(this.contentSha); + }, + imageDiff() { + return !this.diffFile.text; + }, + replacedFile() { + return !(this.diffFile.newFile || this.diffFile.deletedFile); + }, + }, + methods: { + handleToggle(e, checkTarget) { + if (checkTarget) { + if (e.target === this.$refs.header) { + this.$emit('toggleFile'); + } + } else { + this.$emit('toggleFile'); + } + }, + truncate(sha) { + return sha.slice(0, 8); }, }, }; </script> <template> - <div class="file-header-content"> + <div + @click="handleToggle($event, true)" + ref="header" + class="file-header-content" + > + <icon + v-if="collapsible" + @click.stop="handleToggle" + name="chevron-down" + aria-hidden="true" + :size="16" + class="diff-toggle-caret" + /> <div v-if="diffFile.submodule" > @@ -33,8 +87,8 @@ export default { class="file-title-name" ></strong> <clipboard-button - title="Copy file path to clipboard" :text="diffFile.submoduleLink" + title="Copy file path to clipboard" css-class="btn-default btn-transparent btn-clipboard" /> </span> @@ -43,9 +97,9 @@ export default { <component ref="titleWrapper" :is="titleTag" - :href="diffFile.discussionPath" + :href="`#${diffFile.fileHash}`" > - <span v-html="diffFile.blobIcon"></span> + <i class="fa fa-fw" :class="`fa-${diffFile.blob.icon}`"></i> <span v-if="diffFile.renamedFile"> <strong class="file-title-name has-tooltip" @@ -90,5 +144,54 @@ export default { {{ diffFile.aMode }} → {{ diffFile.bMode }} </small> </template> + + + <div + v-if="!diffFile.submodule" + class="file-actions hidden-xs" + > + <template + v-if="diffFile.blob && diffFile.blob.readableText" + > + <button + class="js-toggle-diff-comments btn" + title="Toggle comments for this file" + type="button" + :class="{ + active: 'todo' + }" + > + <icon name="comment" /> + </button> + + <a + :href="diffFile.editPath" + class="btn btn-default js-edit-blob" + > + Edit + </a> + </template> + + <a + v-if="imageDiff && replacedFile" + class="btn view-file js-view-file" + :href="baseSha" + > + View replaced file @ <span class="commit-sha">{{ truncatedBaseSha }}</span> + </a> + <a + class="btn view-file js-view-file" + :href="contentSha" + > + View file @ <span class="commit-sha">{{ truncatedContentSha }}</span> + </a> + + <button + v-if="diffFile.environment" + > + View on environment + <!-- = view_on_environment_button(diff_file.content_sha, diff_file.file_path, environment) if environment --> + </button> + </div> </div> </template> diff --git a/app/assets/javascripts/notes/components/discussion_counter.vue b/app/assets/javascripts/notes/components/discussion_counter.vue index d492d1cd001..cbe4774a360 100644 --- a/app/assets/javascripts/notes/components/discussion_counter.vue +++ b/app/assets/javascripts/notes/components/discussion_counter.vue @@ -86,7 +86,7 @@ export default { v-html="resolveSvg" ></span> </span> - <span class=".line-resolve-text"> + <span class="line-resolve-text"> {{ resolvedDiscussionCount }}/{{ discussionCount }} {{ countText }} resolved </span> </div> diff --git a/app/assets/javascripts/notes/components/note_body.vue b/app/assets/javascripts/notes/components/note_body.vue index 069f94c5845..1bd0a1bd8f3 100644 --- a/app/assets/javascripts/notes/components/note_body.vue +++ b/app/assets/javascripts/notes/components/note_body.vue @@ -72,7 +72,7 @@ export default { this.$emit('handleFormUpdate', note, parentElement, callback); }, formCancelHandler(shouldConfirm, isDirty) { - this.$emit('cancelFormEdition', shouldConfirm, isDirty); + this.$emit('cancelForm', shouldConfirm, isDirty); }, }, }; @@ -90,7 +90,7 @@ export default { v-if="isEditing" ref="noteForm" @handleFormUpdate="handleFormUpdate" - @cancelFormEdition="formCancelHandler" + @cancelForm="formCancelHandler" :is-editing="isEditing" :note-body="noteBody" :note-id="note.id" diff --git a/app/assets/javascripts/notes/components/note_form.vue b/app/assets/javascripts/notes/components/note_form.vue index c59a2e7a406..da50fcb70e8 100644 --- a/app/assets/javascripts/notes/components/note_form.vue +++ b/app/assets/javascripts/notes/components/note_form.vue @@ -124,7 +124,7 @@ export default { cancelHandler(shouldConfirm = false) { // Sends information about confirm message and if the textarea has changed this.$emit( - 'cancelFormEdition', + 'cancelForm', shouldConfirm, this.noteBody !== this.updatedNoteBody, ); diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index cf579c5d4dc..b4698becce7 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -40,6 +40,21 @@ export default { type: Object, required: true, }, + renderHeader: { + type: Boolean, + required: false, + default: true, + }, + renderDiffFile: { + type: Boolean, + required: false, + default: true, + }, + alwaysExpanded: { + type: Boolean, + required: false, + default: false, + }, }, data() { return { @@ -95,12 +110,17 @@ export default { return this.unresolvedDiscussions.length > 0; }, wrapperComponent() { - return this.discussion.diffDiscussion && this.discussion.diffFile - ? diffWithNote - : 'div'; + const shouldRenderDiffs = + this.discussion.diffDiscussion && + this.discussion.diffFile && + this.renderDiffFile; + + return shouldRenderDiffs ? diffWithNote : 'div'; }, wrapperClass() { - return this.isDiffDiscussion ? '' : 'panel panel-default'; + return this.isDiffDiscussion + ? '' + : 'panel panel-default discussion-wrapper'; }, }, mounted() { @@ -149,10 +169,10 @@ export default { }, cancelReplyForm(shouldConfirm) { if (shouldConfirm && this.$refs.noteForm.isDirty) { - const msg = 'Are you sure you want to cancel creating this comment?'; - // eslint-disable-next-line no-alert - if (!confirm(msg)) { + if ( + !confirm('Are you sure you want to cancel creating this comment?') + ) { return; } } @@ -222,7 +242,10 @@ Please check your network connection and try again.`; </div> <div class="timeline-content"> <div class="discussion"> - <div class="discussion-header"> + <div + v-if="renderHeader" + class="discussion-header" + > <note-header :author="author" :created-at="discussion.created_at" @@ -242,7 +265,7 @@ Please check your network connection and try again.`; /> </div> <div - v-if="note.expanded" + v-if="note.expanded || alwaysExpanded" class="discussion-body"> <component :is="wrapperComponent" @@ -332,7 +355,7 @@ Please check your network connection and try again.`; :note="note" :is-editing="false" @handleFormUpdate="saveReply" - @cancelFormEdition="cancelReplyForm" + @cancelForm="cancelReplyForm" ref="noteForm" /> <note-signed-out-widget v-if="!canReply" /> </div> diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index 3554027d2b4..730983ca1fd 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -154,7 +154,8 @@ export default { class="note timeline-entry" :id="noteAnchorId" :class="classNameBindings" - :data-award-url="note.toggle_award_path"> + :data-award-url="note.toggle_award_path" + > <div class="timeline-entry-inner"> <div class="timeline-icon"> <user-avatar-link @@ -194,7 +195,7 @@ export default { :can-edit="note.current_user.can_edit" :is-editing="isEditing" @handleFormUpdate="formUpdateHandler" - @cancelFormEdition="formCancelHandler" + @cancelForm="formCancelHandler" ref="noteBody" /> </div> diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue index a90c6d6381d..908a79bcee1 100644 --- a/app/assets/javascripts/notes/components/notes_app.vue +++ b/app/assets/javascripts/notes/components/notes_app.vue @@ -3,7 +3,6 @@ import $ from 'jquery'; import { mapGetters, mapActions } from 'vuex'; import { getLocationHash } from '../../lib/utils/url_utility'; import Flash from '../../flash'; -import store from '../stores/'; import * as constants from '../constants'; import noteableNote from './noteable_note.vue'; import noteableDiscussion from './noteable_discussion.vue'; @@ -39,23 +38,24 @@ export default { required: false, default: () => ({}), }, + shouldShow: { + type: Boolean, + required: false, + default: true, + }, }, - store, data() { return { isLoading: true, }; }, computed: { - ...mapGetters(['notes', 'getNotesDataByProp', 'discussionCount']), - noteableType() { - // FIXME -- @fatihacet Get this from JSON data. - const { ISSUE_NOTEABLE_TYPE, MERGE_REQUEST_NOTEABLE_TYPE } = constants; - - return this.noteableData.merge_params - ? MERGE_REQUEST_NOTEABLE_TYPE - : ISSUE_NOTEABLE_TYPE; - }, + ...mapGetters([ + 'notes', + 'getNotesDataByProp', + 'discussionCount', + 'noteableType', + ]), allNotes() { if (this.isLoading) { const totalNotes = parseInt(this.notesData.totalNotes, 10) || 0; @@ -86,10 +86,6 @@ export default { this.actionToggleAward({ awardName, noteId }); }); } - document.addEventListener('refreshVueNotes', this.fetchNotes); - }, - beforeDestroy() { - document.removeEventListener('refreshVueNotes', this.fetchNotes); }, methods: { ...mapActions({ @@ -160,7 +156,9 @@ export default { </script> <template> - <div id="notes"> + <div + v-if="shouldShow" + id="notes"> <ul id="notes-list" class="notes main-notes-list timeline"> diff --git a/app/assets/javascripts/notes/index.js b/app/assets/javascripts/notes/index.js index f90775d0157..66897e6a0db 100644 --- a/app/assets/javascripts/notes/index.js +++ b/app/assets/javascripts/notes/index.js @@ -1,5 +1,15 @@ import Vue from 'vue'; +import Vuex from 'vuex'; import notesApp from './components/notes_app.vue'; +import storeConfig from './stores'; + +Vue.use(Vuex); + +const store = new Vuex.Store({ + modules: { + notes: storeConfig, + }, +}); document.addEventListener( 'DOMContentLoaded', @@ -9,11 +19,11 @@ document.addEventListener( components: { notesApp, }, + store, data() { const notesDataset = document.getElementById('js-vue-notes').dataset; const parsedUserData = JSON.parse(notesDataset.currentUserData); let currentUserData = {}; - if (parsedUserData) { currentUserData = { id: parsedUserData.id, diff --git a/app/assets/javascripts/notes/mixins/resolvable.js b/app/assets/javascripts/notes/mixins/resolvable.js index f79049b85f6..a7684cc5659 100644 --- a/app/assets/javascripts/notes/mixins/resolvable.js +++ b/app/assets/javascripts/notes/mixins/resolvable.js @@ -35,9 +35,13 @@ export default { methods: { resolveHandler(resolvedState = false) { this.isResolving = true; - const endpoint = this.note.resolve_path || `${this.note.path}/resolve`; const isResolved = this.discussionResolved || resolvedState; const discussion = this.resolveAsThread; + let endpoint = `${this.note.path}/resolve`; + + if (discussion) { + endpoint = this.note.resolve_path; + } this.toggleResolveNote({ endpoint, isResolved, discussion }) .then(() => { diff --git a/app/assets/javascripts/notes/stores/actions.js b/app/assets/javascripts/notes/stores/actions.js index 244a6980b5a..b2e27f6dd3f 100644 --- a/app/assets/javascripts/notes/stores/actions.js +++ b/app/assets/javascripts/notes/stores/actions.js @@ -14,16 +14,22 @@ let eTagPoll; export const setNotesData = ({ commit }, data) => commit(types.SET_NOTES_DATA, data); + export const setNoteableData = ({ commit }, data) => commit(types.SET_NOTEABLE_DATA, data); + export const setUserData = ({ commit }, data) => commit(types.SET_USER_DATA, data); + export const setLastFetchedAt = ({ commit }, data) => commit(types.SET_LAST_FETCHED_AT, data); + export const setInitialNotes = ({ commit }, data) => commit(types.SET_INITIAL_NOTES, data); + export const setTargetNoteHash = ({ commit }, data) => commit(types.SET_TARGET_NOTE_HASH, data); + export const toggleDiscussion = ({ commit }, data) => commit(types.TOGGLE_DISCUSSION, data); @@ -134,7 +140,8 @@ export const toggleIssueLocalState = ({ commit }, newState) => { }; export const saveNote = ({ commit, dispatch }, noteData) => { - const { note } = noteData.data.note; + // For MR discussuions we need to post as `note[note]` and issue we use `note.note`. + const note = noteData.data['note[note]'] || noteData.data.note.note; let placeholderText = note; const hasQuickActions = utils.hasQuickActions(placeholderText); const replyId = noteData.data.in_reply_to_discussion_id; diff --git a/app/assets/javascripts/notes/stores/getters.js b/app/assets/javascripts/notes/stores/getters.js index f89591a54d6..82f31bee174 100644 --- a/app/assets/javascripts/notes/stores/getters.js +++ b/app/assets/javascripts/notes/stores/getters.js @@ -1,16 +1,22 @@ import _ from 'underscore'; +import * as constants from '../constants'; export const notes = state => state.notes; + export const targetNoteHash = state => state.targetNoteHash; export const getNotesData = state => state.notesData; + export const getNotesDataByProp = state => prop => state.notesData[prop]; export const getNoteableData = state => state.noteableData; + export const getNoteableDataByProp = state => prop => state.noteableData[prop]; + export const openState = state => state.noteableData.state; export const getUserData = state => state.userData || {}; + export const getUserDataByProp = state => prop => state.userData && state.userData[prop]; @@ -20,7 +26,28 @@ export const notesById = state => return acc; }, {}); +export const discussionsByLineCode = state => + state.notes.reduce((acc, note) => { + if (note.diff_discussion && note.line_code) { + // For context about line notes: there might be multiple notes with the same line code + const items = acc[note.line_code] || []; + items.push(note); + + Object.assign(acc, { [note.line_code]: items }); + } + return acc; + }, {}); + +export const noteableType = state => { + const { ISSUE_NOTEABLE_TYPE, MERGE_REQUEST_NOTEABLE_TYPE } = constants; + + return state.noteableData.merge_params + ? MERGE_REQUEST_NOTEABLE_TYPE + : ISSUE_NOTEABLE_TYPE; +}; + const reverseNotes = array => array.slice(0).reverse(); + const isLastNote = (note, state) => !note.system && state.userData && diff --git a/app/assets/javascripts/notes/stores/index.js b/app/assets/javascripts/notes/stores/index.js index 9ed19bf171e..f9805bc4d1e 100644 --- a/app/assets/javascripts/notes/stores/index.js +++ b/app/assets/javascripts/notes/stores/index.js @@ -1,12 +1,8 @@ -import Vue from 'vue'; -import Vuex from 'vuex'; import * as actions from './actions'; import * as getters from './getters'; import mutations from './mutations'; -Vue.use(Vuex); - -export default new Vuex.Store({ +export default { state: { notes: [], targetNoteHash: null, @@ -23,4 +19,4 @@ export default new Vuex.Store({ actions, getters, mutations, -}); +}; diff --git a/app/assets/javascripts/notes/stores/mutations.js b/app/assets/javascripts/notes/stores/mutations.js index c8edc06349f..356f261bea5 100644 --- a/app/assets/javascripts/notes/stores/mutations.js +++ b/app/assets/javascripts/notes/stores/mutations.js @@ -26,7 +26,6 @@ export default { } state.notes.push(noteData); - document.dispatchEvent(new CustomEvent('refreshLegacyNotes')); } }, @@ -35,7 +34,6 @@ export default { if (noteObj) { noteObj.notes.push(note); - document.dispatchEvent(new CustomEvent('refreshLegacyNotes')); } }, @@ -52,8 +50,6 @@ export default { state.notes.splice(state.notes.indexOf(noteObj), 1); } } - - document.dispatchEvent(new CustomEvent('refreshLegacyNotes')); }, [types.REMOVE_PLACEHOLDER_NOTES](state) { @@ -161,8 +157,6 @@ export default { user: { id, name, username }, }); } - - document.dispatchEvent(new CustomEvent('refreshLegacyNotes')); }, [types.TOGGLE_DISCUSSION](state, { discussionId }) { @@ -180,8 +174,6 @@ export default { const comment = utils.findNoteObjectById(noteObj.notes, note.id); noteObj.notes.splice(noteObj.notes.indexOf(comment), 1, note); } - - // document.dispatchEvent(new CustomEvent('refreshLegacyNotes')); }, [types.UPDATE_DISCUSSION](state, noteData) { @@ -196,8 +188,6 @@ export default { note.expanded = true; // override expand flag to prevent collapse state.notes.splice(index, 1, note); - - document.dispatchEvent(new CustomEvent('refreshLegacyNotes')); }, [types.CLOSE_ISSUE](state) { diff --git a/app/assets/javascripts/pages/projects/merge_requests/init_merge_request_show.js b/app/assets/javascripts/pages/projects/merge_requests/init_merge_request_show.js index 28d8761b502..26ead75cec4 100644 --- a/app/assets/javascripts/pages/projects/merge_requests/init_merge_request_show.js +++ b/app/assets/javascripts/pages/projects/merge_requests/init_merge_request_show.js @@ -1,30 +1,15 @@ -import MergeRequest from '~/merge_request'; import ZenMode from '~/zen_mode'; -import initNotes from '~/init_notes'; import initIssuableSidebar from '~/init_issuable_sidebar'; -import initDiffNotes from '~/diff_notes/diff_notes_bundle'; import ShortcutsIssuable from '~/shortcuts_issuable'; -import Diff from '~/diff'; import { handleLocationHash } from '~/lib/utils/common_utils'; import howToMerge from '~/how_to_merge'; import initPipelines from '~/commit/pipelines/pipelines_bundle'; import initWidget from '../../../vue_merge_request_widget'; -export default function () { - new Diff(); // eslint-disable-line no-new +export default function() { new ZenMode(); // eslint-disable-line no-new - initIssuableSidebar(); - initNotes(); - initDiffNotes(); initPipelines(); - - const mrShowNode = document.querySelector('.merge-request'); - - window.mergeRequest = new MergeRequest({ - action: mrShowNode.dataset.mrAction, - }); - new ShortcutsIssuable(true); // eslint-disable-line no-new handleLocationHash(); howToMerge(); diff --git a/app/assets/javascripts/pages/projects/merge_requests/show/index.js b/app/assets/javascripts/pages/projects/merge_requests/show/index.js index e5b2827b50c..f61f4db78d5 100644 --- a/app/assets/javascripts/pages/projects/merge_requests/show/index.js +++ b/app/assets/javascripts/pages/projects/merge_requests/show/index.js @@ -1,4 +1,3 @@ -import { hasVueMRDiscussionsCookie } from '~/lib/utils/common_utils'; import initMrNotes from '~/mr_notes'; import initSidebarBundle from '~/sidebar/sidebar_bundle'; import initShow from '../init_merge_request_show'; @@ -6,8 +5,5 @@ import initShow from '../init_merge_request_show'; document.addEventListener('DOMContentLoaded', () => { initShow(); initSidebarBundle(); - - if (hasVueMRDiscussionsCookie()) { - initMrNotes(); - } + initMrNotes(); }); diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 7f037582ca0..7a94e5d27bc 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -71,6 +71,12 @@ span { white-space: pre-wrap; + + &.context-cell { + display: inline-block; + width: 100%; + height: 100%; + } } .line { diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 81e98f358a8..35ecd05e34e 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -3,9 +3,15 @@ */ @-webkit-keyframes targe3-note { - from { background: $note-targe3-outside; } - 50% { background: $note-targe3-inside; } - to { background: $note-targe3-outside; } + from { + background: $note-targe3-outside; + } + 50% { + background: $note-targe3-inside; + } + to { + background: $note-targe3-outside; + } } ul.notes { @@ -36,7 +42,8 @@ ul.notes { } } - > li { // .timeline-entry + > li { + // .timeline-entry padding: 0; display: block; position: relative; @@ -153,7 +160,6 @@ ul.notes { } .note-header { - @include notes-media('max', $screen-xs-min) { .inline { display: block; @@ -282,7 +288,10 @@ ul.notes { position: absolute; left: 0; bottom: 0; - background: linear-gradient(rgba($white-light, 0.1) -100px, $white-light 100%); + background: linear-gradient( + rgba($white-light, 0.1) -100px, + $white-light 100% + ); } } } @@ -666,7 +675,6 @@ ul.notes { background-color: $white-light; } - a { color: $gl-link-color; } @@ -745,7 +753,8 @@ ul.notes { border: 0; outline: 0; color: $gray-darkest; - transition: color $general-hover-transition-duration $general-hover-transition-curve; + transition: color $general-hover-transition-duration + $general-hover-transition-curve; &.is-disabled { cursor: default; @@ -776,3 +785,39 @@ ul.notes { .line-resolve-text { vertical-align: middle; } + +// Vue refactored diff discussion adjustments +.files { + .diff-discussions { + .note-discussion.timeline-entry { + padding-left: 0; + + & > .timeline-entry-inner { + padding: 0; + + & > .timeline-content { + margin-left: 0; + } + + & > .timeline-icon { + display: none; + } + } + .discussion-body { + padding-top: 0; + + .discussion-wrapper { + border-color: transparent; + } + } + } + } + + .diff-comment-form { + display: block; + } + + .add-diff-note svg { + margin-top: 4px; + } +} diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 0c1c286a0a4..a366559ae1d 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -3,8 +3,8 @@ class Projects::BlobController < Projects::ApplicationController include ExtractsPath include CreatesCommit include RendersBlob + include NotesHelper include ActionView::Helpers::SanitizeHelper - prepend_before_action :authenticate_user!, only: [:edit] before_action :require_non_empty_project, except: [:new, :create] @@ -92,6 +92,7 @@ class Projects::BlobController < Projects::ApplicationController @lines = Gitlab::Highlight.highlight(@blob.path, @blob.data, repository: @repository).lines @form = UnfoldForm.new(params) + @lines = @lines[@form.since - 1..@form.to - 1].map(&:html_safe) if @form.bottom? @@ -102,11 +103,45 @@ class Projects::BlobController < Projects::ApplicationController @match_line = "@@ -#{line}+#{line} @@" end - render layout: false + if has_vue_discussions_cookie? + render_diff_lines + else + render layout: false + end end private + # Converts a String array to Gitlab::Diff::Line array + def render_diff_lines + @lines.map! do |line| + diff_line = Gitlab::Diff::Line.new(line ,'context', nil, nil, nil) + diff_line.rich_text = line + diff_line + end + + add_match_line + + render json: @lines + end + + def add_match_line + return unless @form.unfold? + + if @form.bottom? && @form.to < @blob.lines.size + old_pos = @form.to - @form.offset + new_pos = @form.to + elsif @form.since != 1 + old_pos = new_pos = @form.since + end + + return unless new_pos + + @match_line = Gitlab::Diff::Line.new(@match_line ,'match', nil, old_pos, new_pos) + + @form.bottom? ? @lines.push(@match_line) : @lines.unshift(@match_line) + end + def blob @blob ||= @repository.blob_at(@commit.id, @path) diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index fe8525a488c..242bcfb574d 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -1,6 +1,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::ApplicationController include DiffForPath include DiffHelper + include NotesHelper include RendersNotes before_action :apply_diff_view_cookie! @@ -11,7 +12,11 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic def show @environment = @merge_request.environments_for(current_user).last - render json: { html: view_to_html_string("projects/merge_requests/diffs/_diffs") } + if has_vue_discussions_cookie? + render json: DiffsSerializer.new.represent(@diffs, merge_request: @merge_request) + else + render json: { html: view_to_html_string("projects/merge_requests/diffs/_diffs") } + end end def diff_for_path diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index 20aed60cb7a..a30ba37c568 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -179,7 +179,7 @@ module NotesHelper end def has_vue_discussions_cookie? - cookies[:vue_mr_discussions] == 'true' + true end def serialize_notes? diff --git a/app/serializers/blob_entity.rb b/app/serializers/blob_entity.rb index ad039a2623d..b501fd5e964 100644 --- a/app/serializers/blob_entity.rb +++ b/app/serializers/blob_entity.rb @@ -3,11 +3,13 @@ class BlobEntity < Grape::Entity expose :id, :path, :name, :mode + expose :readable_text?, as: :readable_text + expose :icon do |blob| IconsHelper.file_type_icon_class('file', blob.mode, blob.name) end - expose :url do |blob| + expose :url, if: -> (*) { request.respond_to?(:ref) } do |blob| project_blob_path(request.project, File.join(request.ref, blob.path)) end end diff --git a/app/serializers/diff_file_entity.rb b/app/serializers/diff_file_entity.rb index 6e68d275047..793a28a7beb 100644 --- a/app/serializers/diff_file_entity.rb +++ b/app/serializers/diff_file_entity.rb @@ -1,7 +1,9 @@ class DiffFileEntity < Grape::Entity + include RequestAwareEntity include DiffHelper include SubmoduleHelper include BlobHelper + include TreeHelper include IconsHelper include ActionView::Helpers::TagHelper @@ -11,15 +13,26 @@ class DiffFileEntity < Grape::Entity submodule_links(diff_file.blob, diff_file.content_sha, diff_file.repository).first end + expose :blob, using: BlobEntity + expose :blob_path do |diff_file| diff_file.blob.path end + expose :blob_name do |diff_file| + diff_file.blob.name + end + expose :blob_icon do |diff_file| blob_icon(diff_file.b_mode, diff_file.file_path) end + expose :file_hash do |diff_file| + Digest::SHA1.hexdigest(diff_file.file_path) + end + expose :file_path + expose :new_file?, as: :new_file expose :deleted_file?, as: :deleted_file expose :renamed_file?, as: :renamed_file expose :old_path @@ -28,6 +41,10 @@ class DiffFileEntity < Grape::Entity expose :a_mode expose :b_mode expose :text?, as: :text + expose :added_lines + expose :removed_lines + expose :diff_refs + expose :content_sha expose :old_path_html do |diff_file| old_path = mark_inline_diffs(diff_file.old_path, diff_file.new_path) @@ -38,4 +55,57 @@ class DiffFileEntity < Grape::Entity _, new_path = mark_inline_diffs(diff_file.old_path, diff_file.new_path) new_path end + + # TODO check if these are not creating a n+1 call + # we should probably also pass project as parameter + expose :edit_path, if: -> (_, options) { options[:merge_request] } do |diff_file| + merge_request = options[:merge_request] + + edit_blob_path(merge_request.source_project, merge_request.source_branch, diff_file.new_path) + end + + expose :view_path, if: -> (_, options) { options[:merge_request] } do |diff_file| + merge_request = options[:merge_request] + + project_blob_path(merge_request.source_project, tree_join(merge_request.source_branch, diff_file.new_path)) + end + + expose :context_lines_path, if: -> (diff_file, _) { diff_file.text? } do |diff_file| + project_blob_diff_path(diff_file.repository.project, tree_join(diff_file.content_sha, diff_file.file_path)) + end + + # Used for inline diffs + expose :highlighted_diff_lines, if: -> (diff_file, _) { diff_file.text? } do |diff_file| + lines = diff_file.highlighted_diff_lines + + add_bottom_match_line(lines, diff_file) + + lines + end + + # Used for parallel diffs + expose :parallel_diff_lines, if: -> (diff_file, _) { diff_file.text? } + + private + + # This adds the bottom line to the array which contains + # the data to load more context lines. + def add_bottom_match_line(lines, diff_file) + return if lines.empty? + + last_line = lines.last + + return unless last_line.new_pos < total_lines(diff_file.blob) + + match_line = Gitlab::Diff::Line.new("" ,'match', nil, last_line.old_pos, last_line.new_pos) + lines.push(match_line) + end + + def total_lines(blob) + @total_lines ||= begin + line_count = blob.lines.size + line_count -= 1 if line_count > 0 && blob.lines.last.blank? + line_count + end + end end diff --git a/app/serializers/diff_file_serializer.rb b/app/serializers/diff_file_serializer.rb new file mode 100644 index 00000000000..8fcb7d67e34 --- /dev/null +++ b/app/serializers/diff_file_serializer.rb @@ -0,0 +1,3 @@ +class DiffFileSerializer < BaseSerializer + entity DiffFileEntity +end diff --git a/app/serializers/diffs_entity.rb b/app/serializers/diffs_entity.rb new file mode 100644 index 00000000000..fb8027059cb --- /dev/null +++ b/app/serializers/diffs_entity.rb @@ -0,0 +1,13 @@ +class DiffsEntity < Grape::Entity + expose :real_size + + expose :added_lines do |diffs| + diffs.diff_files.sum(&:added_lines) + end + + expose :removed_lines do |diffs| + diffs.diff_files.sum(&:removed_lines) + end + + expose :diff_files, using: DiffFileEntity +end diff --git a/app/serializers/diffs_serializer.rb b/app/serializers/diffs_serializer.rb new file mode 100644 index 00000000000..6771e10c5ac --- /dev/null +++ b/app/serializers/diffs_serializer.rb @@ -0,0 +1,3 @@ +class DiffsSerializer < BaseSerializer + entity DiffsEntity +end diff --git a/app/serializers/discussion_entity.rb b/app/serializers/discussion_entity.rb index bbbcf6a97c1..6af0973c1dd 100644 --- a/app/serializers/discussion_entity.rb +++ b/app/serializers/discussion_entity.rb @@ -2,6 +2,8 @@ class DiscussionEntity < Grape::Entity include RequestAwareEntity expose :id, :reply_id + expose :position, if: -> (d, _) { defined? d.diff_file } + expose :line_code, if: -> (d, _) { defined? d.diff_file } expose :expanded?, as: :expanded expose :notes, using: NoteEntity diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml index 9866cc716ee..6029738f437 100644 --- a/app/views/projects/merge_requests/show.html.haml +++ b/app/views/projects/merge_requests/show.html.haml @@ -76,19 +76,22 @@ %section.col-md-12 %script.js-notes-data{ type: "application/json" }= initial_notes_data(true).to_json.html_safe .issuable-discussion.js-vue-notes-event - = render "projects/merge_requests/discussion" - if has_vue_discussions_cookie? #js-vue-mr-discussions{ data: { notes_data: notes_data(@merge_request), noteable_data: serialize_issuable(@merge_request), current_user_data: UserSerializer.new.represent(current_user).to_json} } + - else + = render "projects/merge_requests/discussion" #commits.commits.tab-pane -# This tab is always loaded via AJAX #pipelines.pipelines.tab-pane - if @pipelines.any? = render 'projects/commit/pipelines_list', disable_initialization: true, endpoint: pipelines_project_merge_request_path(@project, @merge_request) - #diffs.diffs.tab-pane{ data: { "is-locked" => @merge_request.discussion_locked? } } - -# This tab is always loaded via AJAX + #js-diffs-app.diffs.tab-pane{ data: { "is-locked" => @merge_request.discussion_locked?, endpoint: diffs_project_merge_request_path(@project, @merge_request, 'json') } } + + - unless has_vue_discussions_cookie? # TODO: @fatihacet This should be deleted after refactor + #diffs .mr-loading-status = spinner diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb index 014854da55c..30eab47575c 100644 --- a/lib/gitlab/diff/file.rb +++ b/lib/gitlab/diff/file.rb @@ -120,11 +120,13 @@ module Gitlab # Array of Gitlab::Diff::Line objects def diff_lines - @diff_lines ||= Gitlab::Diff::Parser.new.parse(raw_diff.each_line).to_a + @diff_lines ||= + Gitlab::Diff::Parser.new.parse(raw_diff.each_line, diff_file: self).to_a end def highlighted_diff_lines - @highlighted_diff_lines ||= Gitlab::Diff::Highlight.new(self, repository: self.repository).highlight + @highlighted_diff_lines ||= + Gitlab::Diff::Highlight.new(self, repository: self.repository).highlight end # Array[<Hash>] with right/left keys that contains Gitlab::Diff::Line objects which text is hightlighted diff --git a/lib/gitlab/diff/highlight.rb b/lib/gitlab/diff/highlight.rb index 269016daac2..16da773bb4a 100644 --- a/lib/gitlab/diff/highlight.rb +++ b/lib/gitlab/diff/highlight.rb @@ -5,7 +5,7 @@ module Gitlab delegate :old_path, :new_path, :old_sha, :new_sha, to: :diff_file, prefix: :diff - def initialize(diff_lines, repository: nil) + def initialize(diff_lines, since: nil, from: nil, repository: nil) @repository = repository if diff_lines.is_a?(Gitlab::Diff::File) diff --git a/lib/gitlab/diff/line.rb b/lib/gitlab/diff/line.rb index 0603141e441..b8966110f5a 100644 --- a/lib/gitlab/diff/line.rb +++ b/lib/gitlab/diff/line.rb @@ -1,22 +1,26 @@ module Gitlab module Diff class Line - attr_reader :type, :index, :old_pos, :new_pos + attr_reader :line_code, :type, :index, :old_pos, :new_pos, :meta_data attr_writer :rich_text attr_accessor :text - def initialize(text, type, index, old_pos, new_pos, parent_file: nil) + def initialize(text, type, index, old_pos, new_pos, parent_file: nil, line_code: nil) @text, @type, @index = text, type, index @old_pos, @new_pos = old_pos, new_pos @parent_file = parent_file + + # When line code is not provided from cache store we build it + # using the parent_file(Diff::File or Conflict::File). + @line_code = line_code || set_line_code end def self.init_from_hash(hash) - new(hash[:text], hash[:type], hash[:index], hash[:old_pos], hash[:new_pos]) + new(hash[:text], hash[:type], hash[:index], hash[:old_pos], hash[:new_pos], line_code: hash[:line_code]) end def serialize_keys - @serialize_keys ||= %i(text type index old_pos new_pos) + @serialize_keys ||= %i(line_code text type index old_pos new_pos) end def to_hash @@ -58,20 +62,39 @@ module Gitlab end def rich_text - @parent_file.highlight_lines! if @parent_file && !@rich_text + @parent_file.try(:highlight_lines!) if @parent_file && !@rich_text @rich_text end + def meta_data + return unless meta? + + { + old_pos: old_pos, + new_pos: new_pos + } + end + def as_json(opts = nil) { + line_code: line_code, type: type, old_line: old_line, new_line: new_line, text: text, - rich_text: rich_text || text + rich_text: rich_text || text, + meta_data: meta_data } end + + private + + def set_line_code + return nil unless @parent_file + + @parent_file.line_code(self) + end end end end diff --git a/lib/gitlab/diff/parser.rb b/lib/gitlab/diff/parser.rb index 8302f30a0a2..7ae7ed286ed 100644 --- a/lib/gitlab/diff/parser.rb +++ b/lib/gitlab/diff/parser.rb @@ -3,7 +3,7 @@ module Gitlab class Parser include Enumerable - def parse(lines) + def parse(lines, diff_file: nil) return [] if lines.blank? @lines = lines @@ -31,17 +31,17 @@ module Gitlab next if line_old <= 1 && line_new <= 1 # top of file - yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new) + yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new, parent_file: diff_file) line_obj_index += 1 next elsif line[0] == '\\' type = "#{context}-nonewline" - yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new) + yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new, parent_file: diff_file) line_obj_index += 1 else type = identification_type(line) - yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new) + yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new, parent_file: diff_file) line_obj_index += 1 end diff --git a/spec/javascripts/diffs/components/changed_files_spec.js b/spec/javascripts/diffs/components/changed_files_spec.js new file mode 100644 index 00000000000..2510b123886 --- /dev/null +++ b/spec/javascripts/diffs/components/changed_files_spec.js @@ -0,0 +1,69 @@ +import Vue from 'vue'; +import { mountComponent } from 'spec/helpers'; +import ChangedFiles from '~/diffs/components/changed_files.vue'; + +const vueMatchers = { + toContainText() { + return { + compare(vm, text) { + const result = { + pass: vm.$el.innerText.includes(text), + }; + return result; + }, + }; + }, + toRender() { + return { + compare(vm) { + const result = { + pass: vm.$el.nodeType !== Node.COMMENT_NODE, + }; + return result; + }, + }; + }, +}; + +describe('ChangedFiles', () => { + const Component = Vue.extend(ChangedFiles); + + beforeEach(() => { + jasmine.addMatchers(vueMatchers); + }); + + describe('with no changed files', () => { + const props = { + diffFiles: [], + }; + + it('does not render', () => { + const vm = mountComponent(Component, props); + + expect(vm).not.toRender(); + }); + }); + + describe('with single file added', () => { + let vm; + const props = { + diffFiles: [{ + addedLines: 10, + removedLines: 20, + }], + }; + + beforeEach(() => { + vm = mountComponent(Component, props); + }); + + it('shows files changes', () => { + expect(vm).toContainText('1 changed file'); + }); + + it('shows file additions and deletions', () => { + expect(vm).toContainText('10 additions'); + expect(vm).toContainText('20 deletions'); + }); + }); +}); diff --git a/spec/javascripts/helpers/index.js b/spec/javascripts/helpers/index.js new file mode 100644 index 00000000000..f15b0532a2b --- /dev/null +++ b/spec/javascripts/helpers/index.js @@ -0,0 +1,9 @@ +import mountComponent from './vue_mount_component_helper'; + +export { + mountComponent, +}; + +export default { + mountComponent, +}; diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js index 27f06573432..bf18d9eff69 100644 --- a/spec/javascripts/lib/utils/common_utils_spec.js +++ b/spec/javascripts/lib/utils/common_utils_spec.js @@ -523,5 +523,75 @@ describe('common_utils', () => { expect(Object.keys(commonUtils.convertObjectPropsToCamelCase()).length).toBe(0); expect(Object.keys(commonUtils.convertObjectPropsToCamelCase({})).length).toBe(0); }); + + it('does not deep-convert by default', () => { + const obj = { + snake_key: { + child_snake_key: 'value', + }, + }; + + expect( + commonUtils.convertObjectPropsToCamelCase(obj), + ).toEqual({ + snakeKey: { + child_snake_key: 'value', + }, + }); + }); + + describe('deep: true', () => { + it('converts object with child objects', () => { + const obj = { + snake_key: { + child_snake_key: 'value', + }, + }; + + expect( + commonUtils.convertObjectPropsToCamelCase(obj, { deep: true }), + ).toEqual({ + snakeKey: { + childSnakeKey: 'value', + }, + }); + }); + + it('converts array with child objects', () => { + const arr = [ + { + child_snake_key: 'value', + }, + ]; + + expect( + commonUtils.convertObjectPropsToCamelCase(arr, { deep: true }), + ).toEqual([ + { + childSnakeKey: 'value', + }, + ]); + }); + + it('converts array with child arrays', () => { + const arr = [ + [ + { + child_snake_key: 'value', + }, + ], + ]; + + expect( + commonUtils.convertObjectPropsToCamelCase(arr, { deep: true }), + ).toEqual([ + [ + { + childSnakeKey: 'value', + }, + ], + ]); + }); + }); }); }); diff --git a/spec/serializers/discussion_entity_spec.rb b/spec/serializers/discussion_entity_spec.rb index 7ee8e38af1c..c0db71c693f 100644 --- a/spec/serializers/discussion_entity_spec.rb +++ b/spec/serializers/discussion_entity_spec.rb @@ -30,7 +30,7 @@ describe DiscussionEntity do let(:note) { create(:diff_note_on_merge_request) } it 'exposes diff file attributes' do - expect(subject).to include(:diff_file, :truncated_diff_lines, :image_diff_html) + expect(subject).to include(:diff_file, :truncated_diff_lines, :image_diff_html, :diff_lines, :highlighted_diff_lines) end end end |