diff options
Diffstat (limited to 'app/assets/javascripts/notes')
10 files changed, 222 insertions, 86 deletions
diff --git a/app/assets/javascripts/notes/components/discussion_counter.vue b/app/assets/javascripts/notes/components/discussion_counter.vue index 831e6dd8f92..33819c78c0f 100644 --- a/app/assets/javascripts/notes/components/discussion_counter.vue +++ b/app/assets/javascripts/notes/components/discussion_counter.vue @@ -78,8 +78,8 @@ export default { v-if="resolveAllDiscussionsIssuePath && !allResolved" v-gl-tooltip :href="resolveAllDiscussionsIssuePath" - :title="s__('Create issue to resolve all threads')" - :aria-label="s__('Create issue to resolve all threads')" + :title="__('Create issue to resolve all threads')" + :aria-label="__('Create issue to resolve all threads')" class="new-issue-for-discussion discussion-create-issue-btn" icon="issue-new" /> diff --git a/app/assets/javascripts/notes/components/discussion_notes.vue b/app/assets/javascripts/notes/components/discussion_notes.vue index 6fcfa66ea49..d1df4eb848b 100644 --- a/app/assets/javascripts/notes/components/discussion_notes.vue +++ b/app/assets/javascripts/notes/components/discussion_notes.vue @@ -1,5 +1,6 @@ <script> import { mapGetters, mapActions } from 'vuex'; +import { GlIntersectionObserver } from '@gitlab/ui'; import { __ } from '~/locale'; import PlaceholderNote from '~/vue_shared/components/notes/placeholder_note.vue'; import PlaceholderSystemNote from '~/vue_shared/components/notes/placeholder_system_note.vue'; @@ -16,7 +17,9 @@ export default { ToggleRepliesWidget, NoteEditedText, DiscussionNotesRepliesWrapper, + GlIntersectionObserver, }, + inject: ['discussionObserverHandler'], props: { discussion: { type: Object, @@ -54,7 +57,11 @@ export default { }, }, computed: { - ...mapGetters(['userCanReply']), + ...mapGetters([ + 'userCanReply', + 'previousUnresolvedDiscussionId', + 'firstUnresolvedDiscussionId', + ]), hasReplies() { return Boolean(this.replies.length); }, @@ -77,9 +84,20 @@ export default { url: this.discussion.discussion_path, }; }, + isFirstUnresolved() { + return this.firstUnresolvedDiscussionId === this.discussion.id; + }, + }, + observerOptions: { + threshold: 0, + rootMargin: '0px 0px -50% 0px', }, methods: { - ...mapActions(['toggleDiscussion', 'setSelectedCommentPositionHover']), + ...mapActions([ + 'toggleDiscussion', + 'setSelectedCommentPositionHover', + 'setCurrentDiscussionId', + ]), componentName(note) { if (note.isPlaceholderNote) { if (note.placeholderType === SYSTEM_NOTE) { @@ -110,6 +128,18 @@ export default { this.setSelectedCommentPositionHover(); } }, + observerTriggered(entry) { + this.discussionObserverHandler({ + entry, + isFirstUnresolved: this.isFirstUnresolved, + currentDiscussion: { ...this.discussion }, + isDiffsPage: !this.isOverviewTab, + functions: { + setCurrentDiscussionId: this.setCurrentDiscussionId, + getPreviousUnresolvedDiscussionId: this.previousUnresolvedDiscussionId, + }, + }); + }, }, }; </script> @@ -122,33 +152,35 @@ export default { @mouseleave="handleMouseLeave(discussion)" > <template v-if="shouldGroupReplies"> - <component - :is="componentName(firstNote)" - :note="componentData(firstNote)" - :line="line || diffLine" - :discussion-file="discussion.diff_file" - :commit="commit" - :help-page-path="helpPagePath" - :show-reply-button="userCanReply" - :discussion-root="true" - :discussion-resolve-path="discussion.resolve_path" - :is-overview-tab="isOverviewTab" - @handleDeleteNote="$emit('deleteNote')" - @startReplying="$emit('startReplying')" - > - <template #discussion-resolved-text> - <note-edited-text - v-if="discussion.resolved" - :edited-at="discussion.resolved_at" - :edited-by="discussion.resolved_by" - :action-text="resolvedText" - class-name="discussion-headline-light js-discussion-headline discussion-resolved-text" - /> - </template> - <template #avatar-badge> - <slot name="avatar-badge"></slot> - </template> - </component> + <gl-intersection-observer :options="$options.observerOptions" @update="observerTriggered"> + <component + :is="componentName(firstNote)" + :note="componentData(firstNote)" + :line="line || diffLine" + :discussion-file="discussion.diff_file" + :commit="commit" + :help-page-path="helpPagePath" + :show-reply-button="userCanReply" + :discussion-root="true" + :discussion-resolve-path="discussion.resolve_path" + :is-overview-tab="isOverviewTab" + @handleDeleteNote="$emit('deleteNote')" + @startReplying="$emit('startReplying')" + > + <template #discussion-resolved-text> + <note-edited-text + v-if="discussion.resolved" + :edited-at="discussion.resolved_at" + :edited-by="discussion.resolved_by" + :action-text="resolvedText" + class-name="discussion-headline-light js-discussion-headline discussion-resolved-text" + /> + </template> + <template #avatar-badge> + <slot name="avatar-badge"></slot> + </template> + </component> + </gl-intersection-observer> <discussion-notes-replies-wrapper :is-diff-discussion="discussion.diff_discussion"> <toggle-replies-widget v-if="hasReplies" diff --git a/app/assets/javascripts/notes/components/multiline_comment_form.vue b/app/assets/javascripts/notes/components/multiline_comment_form.vue index 6ad565567be..1633b79c3be 100644 --- a/app/assets/javascripts/notes/components/multiline_comment_form.vue +++ b/app/assets/javascripts/notes/components/multiline_comment_form.vue @@ -1,6 +1,6 @@ <script> import { GlFormSelect, GlSprintf } from '@gitlab/ui'; -import { mapActions, mapState } from 'vuex'; +import { mapActions } from 'vuex'; import { getSymbol, getLineClasses } from './multiline_comment_utils'; export default { @@ -27,13 +27,12 @@ export default { }; }, computed: { - ...mapState({ selectedCommentPosition: ({ notes }) => notes.selectedCommentPosition }), lineNumber() { return this.commentLineOptions[this.commentLineOptions.length - 1].text; }, }, created() { - const line = this.selectedCommentPosition?.start || this.lineRange?.start || this.line; + const line = this.lineRange?.start || this.line; this.commentLineStart = { line_code: line.line_code, @@ -42,7 +41,6 @@ export default { new_line: line.new_line, }; - if (this.selectedCommentPosition) return; this.highlightSelection(); }, destroyed() { diff --git a/app/assets/javascripts/notes/components/note_body.vue b/app/assets/javascripts/notes/components/note_body.vue index 1ce1696e332..c09582d6287 100644 --- a/app/assets/javascripts/notes/components/note_body.vue +++ b/app/assets/javascripts/notes/components/note_body.vue @@ -1,5 +1,6 @@ <script> import $ from 'jquery'; +import { GlSafeHtmlDirective } from '@gitlab/ui'; import { escape } from 'lodash'; import { mapActions, mapGetters, mapState } from 'vuex'; @@ -19,6 +20,9 @@ export default { noteForm, Suggestions, }, + directives: { + SafeHtml: GlSafeHtmlDirective, + }, mixins: [autosave], props: { note: { @@ -144,6 +148,9 @@ export default { this.removeSuggestionInfoFromBatch(suggestionId); }, }, + safeHtmlConfig: { + ADD_TAGS: ['use', 'gl-emoji'], + }, }; </script> @@ -163,11 +170,7 @@ export default { @addToBatch="addSuggestionToBatch" @removeFromBatch="removeSuggestionFromBatch" /> - <div - v-else - class="note-text md" - v-html="note.note_html /* eslint-disable-line vue/no-v-html */" - ></div> + <div v-else v-safe-html:[$options.safeHtmlConfig]="note.note_html" class="note-text md"></div> <note-form v-if="isEditing" ref="noteForm" diff --git a/app/assets/javascripts/notes/components/note_form.vue b/app/assets/javascripts/notes/components/note_form.vue index b05643e5e13..d6b65ed0e8b 100644 --- a/app/assets/javascripts/notes/components/note_form.vue +++ b/app/assets/javascripts/notes/components/note_form.vue @@ -1,9 +1,9 @@ <script> -import { GlButton } from '@gitlab/ui'; +import { GlButton, GlSprintf, GlLink } from '@gitlab/ui'; import { mapGetters, mapActions, mapState } from 'vuex'; import { getDraft, updateDraft } from '~/lib/utils/autosave'; import { mergeUrlParams } from '~/lib/utils/url_utility'; -import { __, sprintf } from '~/locale'; +import { __ } from '~/locale'; import markdownField from '~/vue_shared/components/markdown/field.vue'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import eventHub from '../event_hub'; @@ -17,6 +17,8 @@ export default { markdownField, CommentFieldLayout, GlButton, + GlSprintf, + GlLink, }, mixins: [glFeatureFlagsMixin(), issuableStateMixin, resolvable], props: { @@ -203,16 +205,12 @@ export default { ); }, changedCommentText() { - return sprintf( - __( + return { + text: __( 'This comment changed after you started editing it. Review the %{startTag}updated comment%{endTag} to ensure information is not lost.', ), - { - startTag: `<a href="${this.noteHash}" target="_blank" rel="noopener noreferrer">`, - endTag: '</a>', - }, - false, - ); + placeholder: { link: ['startTag', 'endTag'] }, + }; }, }, watch: { @@ -318,11 +316,13 @@ export default { <template> <div ref="editNoteForm" class="note-edit-form current-note-edit-form js-discussion-note-form"> - <div - v-if="conflictWhileEditing" - class="js-conflict-edit-warning alert alert-danger" - v-html="changedCommentText /* eslint-disable-line vue/no-v-html */" - ></div> + <div v-if="conflictWhileEditing" class="js-conflict-edit-warning alert alert-danger"> + <gl-sprintf :message="changedCommentText.text" :placeholders="changedCommentText.placeholder"> + <template #link="{ content }"> + <gl-link :href="noteHash" target="_blank">{{ content }}</gl-link> + </template> + </gl-sprintf> + </div> <div class="flash-container timeline-content"></div> <form :data-line-code="lineCode" class="edit-note common-note-form js-quick-submit gfm-form"> <comment-field-layout @@ -334,13 +334,13 @@ export default { :markdown-docs-path="markdownDocsPath" :quick-actions-docs-path="quickActionsDocsPath" :line="line" + :lines="lines" :note="discussionNote" :can-suggest="canSuggest" :add-spacing-classes="false" :help-page-path="helpPagePath" :show-suggest-popover="showSuggestPopover" :textarea-value="updatedNoteBody" - :lines="lines" @handleSuggestDismissed="() => $emit('handleSuggestDismissed')" > <template #textarea> diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue index 58570e76795..3ab3e7a20d4 100644 --- a/app/assets/javascripts/notes/components/notes_app.vue +++ b/app/assets/javascripts/notes/components/notes_app.vue @@ -8,6 +8,7 @@ import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item import OrderedLayout from '~/vue_shared/components/ordered_layout.vue'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import draftNote from '../../batch_comments/components/draft_note.vue'; +import { discussionIntersectionObserverHandlerFactory } from '../../diffs/utils/discussions'; import { getLocationHash, doesHashExistInUrl } from '../../lib/utils/url_utility'; import placeholderNote from '../../vue_shared/components/notes/placeholder_note.vue'; import placeholderSystemNote from '../../vue_shared/components/notes/placeholder_system_note.vue'; @@ -38,6 +39,9 @@ export default { TimelineEntryItem, }, mixins: [glFeatureFlagsMixin()], + provide: { + discussionObserverHandler: discussionIntersectionObserverHandlerFactory(), + }, props: { noteableData: { type: Object, @@ -94,15 +98,17 @@ export default { return this.noteableData.noteableType; }, allDiscussions() { + let skeletonNotes = []; + if (this.renderSkeleton || this.isLoading) { const prerenderedNotesCount = parseInt(this.notesData.prerenderedNotesCount, 10) || 0; - return new Array(prerenderedNotesCount).fill({ + skeletonNotes = new Array(prerenderedNotesCount).fill({ isSkeletonNote: true, }); } - return this.discussions; + return this.discussions.concat(skeletonNotes); }, canReply() { return this.userCanReply && !this.commentsDisabled && !this.timelineEnabled; @@ -258,7 +264,13 @@ export default { getFetchDiscussionsConfig() { const defaultConfig = { path: this.getNotesDataByProp('discussionsPath') }; - if (doesHashExistInUrl(constants.NOTE_UNDERSCORE)) { + const currentFilter = + this.getNotesDataByProp('notesFilter') || constants.DISCUSSION_FILTERS_DEFAULT_VALUE; + + if ( + doesHashExistInUrl(constants.NOTE_UNDERSCORE) && + currentFilter !== constants.DISCUSSION_FILTERS_DEFAULT_VALUE + ) { return { ...defaultConfig, filter: constants.DISCUSSION_FILTERS_DEFAULT_VALUE, diff --git a/app/assets/javascripts/notes/mixins/discussion_navigation.js b/app/assets/javascripts/notes/mixins/discussion_navigation.js index 96974c4fa2d..ad529eb99b6 100644 --- a/app/assets/javascripts/notes/mixins/discussion_navigation.js +++ b/app/assets/javascripts/notes/mixins/discussion_navigation.js @@ -1,7 +1,10 @@ import { mapGetters, mapActions, mapState } from 'vuex'; import { scrollToElementWithContext, scrollToElement } from '~/lib/utils/common_utils'; +import { updateHistory } from '../../lib/utils/url_utility'; import eventHub from '../event_hub'; +const isDiffsVirtualScrollingEnabled = () => window.gon?.features?.diffsVirtualScrolling; + /** * @param {string} selector * @returns {boolean} @@ -11,20 +14,52 @@ function scrollTo(selector, { withoutContext = false } = {}) { const scrollFunction = withoutContext ? scrollToElement : scrollToElementWithContext; if (el) { - scrollFunction(el); + scrollFunction(el, { + behavior: isDiffsVirtualScrollingEnabled() ? 'auto' : 'smooth', + }); return true; } return false; } +function updateUrlWithNoteId(noteId) { + const newHistoryEntry = { + state: null, + title: window.title, + url: `#note_${noteId}`, + replace: true, + }; + + if (noteId && isDiffsVirtualScrollingEnabled()) { + // Temporarily mask the ID to avoid the browser default + // scrolling taking over which is broken with virtual + // scrolling enabled. + const note = document.querySelector(`#note_${noteId}`); + note?.setAttribute('id', `masked::${note.id}`); + + // Update the hash now that the ID "doesn't exist" in the page + updateHistory(newHistoryEntry); + + // Unmask the note's ID + note?.setAttribute('id', `note_${noteId}`); + } else if (noteId) { + updateHistory(newHistoryEntry); + } +} + /** * @param {object} self Component instance with mixin applied * @param {string} id Discussion id we are jumping to */ -function diffsJump({ expandDiscussion }, id) { +function diffsJump({ expandDiscussion }, id, firstNoteId) { const selector = `ul.notes[data-discussion-id="${id}"]`; - eventHub.$once('scrollToDiscussion', () => scrollTo(selector)); + + eventHub.$once('scrollToDiscussion', () => { + scrollTo(selector); + // Wait for the discussion scroll before updating to the more specific ID + setTimeout(() => updateUrlWithNoteId(firstNoteId), 0); + }); expandDiscussion({ discussionId: id }); } @@ -56,12 +91,13 @@ function switchToDiscussionsTabAndJumpTo(self, id) { * @param {object} discussion Discussion we are jumping to */ function jumpToDiscussion(self, discussion) { - const { id, diff_discussion: isDiffDiscussion } = discussion; + const { id, diff_discussion: isDiffDiscussion, notes } = discussion; + const firstNoteId = notes?.[0]?.id; if (id) { const activeTab = window.mrTabs.currentAction; if (activeTab === 'diffs' && isDiffDiscussion) { - diffsJump(self, id); + diffsJump(self, id, firstNoteId); } else if (activeTab === 'show') { discussionJump(self, id); } else { @@ -79,10 +115,18 @@ function handleDiscussionJump(self, fn, discussionId = self.currentDiscussionId) const isDiffView = window.mrTabs.currentAction === 'diffs'; const targetId = fn(discussionId, isDiffView); const discussion = self.getDiscussion(targetId); + const setHash = !isDiffView && !isDiffsVirtualScrollingEnabled(); const discussionFilePath = discussion?.diff_file?.file_path; + if (isDiffsVirtualScrollingEnabled()) { + window.location.hash = ''; + } + if (discussionFilePath) { - self.scrollToFile(discussionFilePath); + self.scrollToFile({ + path: discussionFilePath, + setHash, + }); } self.$nextTick(() => { diff --git a/app/assets/javascripts/notes/stores/actions.js b/app/assets/javascripts/notes/stores/actions.js index 7eb10f647a0..c862a29ad9c 100644 --- a/app/assets/javascripts/notes/stores/actions.js +++ b/app/assets/javascripts/notes/stores/actions.js @@ -1,4 +1,3 @@ -/* eslint-disable @gitlab/require-string-literal-i18n-helpers */ import $ from 'jquery'; import Visibility from 'visibilityjs'; import Vue from 'vue'; @@ -71,7 +70,7 @@ export const setUserData = ({ commit }, data) => commit(types.SET_USER_DATA, dat export const setLastFetchedAt = ({ commit }, data) => commit(types.SET_LAST_FETCHED_AT, data); export const setInitialNotes = ({ commit }, discussions) => - commit(types.SET_INITIAL_DISCUSSIONS, discussions); + commit(types.ADD_OR_UPDATE_DISCUSSIONS, discussions); export const setTargetNoteHash = ({ commit }, data) => commit(types.SET_TARGET_NOTE_HASH, data); @@ -90,14 +89,51 @@ export const fetchDiscussions = ({ commit, dispatch }, { path, filter, persistFi ? { params: { notes_filter: filter, persist_filter: persistFilter } } : null; + if (window.gon?.features?.paginatedIssueDiscussions) { + return dispatch('fetchDiscussionsBatch', { path, config, perPage: 20 }); + } + return axios.get(path, config).then(({ data }) => { - commit(types.SET_INITIAL_DISCUSSIONS, data); + commit(types.ADD_OR_UPDATE_DISCUSSIONS, data); commit(types.SET_FETCHING_DISCUSSIONS, false); dispatch('updateResolvableDiscussionsCounts'); }); }; +export const fetchDiscussionsBatch = ({ commit, dispatch }, { path, config, cursor, perPage }) => { + const params = { ...config?.params, per_page: perPage }; + + if (cursor) { + params.cursor = cursor; + } + + return axios.get(path, { params }).then(({ data, headers }) => { + commit(types.ADD_OR_UPDATE_DISCUSSIONS, data); + + if (headers['x-next-page-cursor']) { + const nextConfig = { ...config }; + + if (config?.params?.persist_filter) { + delete nextConfig.params.notes_filter; + delete nextConfig.params.persist_filter; + } + + return dispatch('fetchDiscussionsBatch', { + path, + config: nextConfig, + cursor: headers['x-next-page-cursor'], + perPage: Math.min(Math.round(perPage * 1.5), 100), + }); + } + + commit(types.SET_FETCHING_DISCUSSIONS, false); + dispatch('updateResolvableDiscussionsCounts'); + + return undefined; + }); +}; + export const updateDiscussion = ({ commit, state }, discussion) => { commit(types.UPDATE_DISCUSSION, discussion); @@ -621,7 +657,7 @@ export const submitSuggestion = ( const flashMessage = errorMessage || defaultMessage; createFlash({ - message: __(flashMessage), + message: flashMessage, parent: flashContainer, }); }) @@ -657,7 +693,7 @@ export const submitSuggestionBatch = ({ commit, dispatch, state }, { message, fl const flashMessage = errorMessage || defaultMessage; createFlash({ - message: __(flashMessage), + message: flashMessage, parent: flashContainer, }); }) diff --git a/app/assets/javascripts/notes/stores/mutation_types.js b/app/assets/javascripts/notes/stores/mutation_types.js index 2e8b728e013..fcd2846ff0d 100644 --- a/app/assets/javascripts/notes/stores/mutation_types.js +++ b/app/assets/javascripts/notes/stores/mutation_types.js @@ -1,11 +1,11 @@ export const ADD_NEW_NOTE = 'ADD_NEW_NOTE'; export const ADD_NEW_REPLY_TO_DISCUSSION = 'ADD_NEW_REPLY_TO_DISCUSSION'; +export const ADD_OR_UPDATE_DISCUSSIONS = 'ADD_OR_UPDATE_DISCUSSIONS'; export const DELETE_NOTE = 'DELETE_NOTE'; export const REMOVE_PLACEHOLDER_NOTES = 'REMOVE_PLACEHOLDER_NOTES'; export const SET_NOTES_DATA = 'SET_NOTES_DATA'; export const SET_NOTEABLE_DATA = 'SET_NOTEABLE_DATA'; export const SET_USER_DATA = 'SET_USER_DATA'; -export const SET_INITIAL_DISCUSSIONS = 'SET_INITIAL_DISCUSSIONS'; export const SET_LAST_FETCHED_AT = 'SET_LAST_FETCHED_AT'; export const SET_TARGET_NOTE_HASH = 'SET_TARGET_NOTE_HASH'; export const SHOW_PLACEHOLDER_NOTE = 'SHOW_PLACEHOLDER_NOTE'; diff --git a/app/assets/javascripts/notes/stores/mutations.js b/app/assets/javascripts/notes/stores/mutations.js index c5fa34dfedd..1a99750ddb3 100644 --- a/app/assets/javascripts/notes/stores/mutations.js +++ b/app/assets/javascripts/notes/stores/mutations.js @@ -129,8 +129,8 @@ export default { Object.assign(state, { userData: data }); }, - [types.SET_INITIAL_DISCUSSIONS](state, discussionsData) { - const discussions = discussionsData.reduce((acc, d) => { + [types.ADD_OR_UPDATE_DISCUSSIONS](state, discussionsData) { + discussionsData.forEach((d) => { const discussion = { ...d }; const diffData = {}; @@ -145,27 +145,38 @@ export default { // To support legacy notes, should be very rare case. if (discussion.individual_note && discussion.notes.length > 1) { discussion.notes.forEach((n) => { - acc.push({ + const newDiscussion = { ...discussion, ...diffData, notes: [n], // override notes array to only have one item to mimick individual_note - }); + }; + const oldDiscussion = state.discussions.find( + (existingDiscussion) => + existingDiscussion.id === discussion.id && existingDiscussion.notes[0].id === n.id, + ); + + if (oldDiscussion) { + state.discussions.splice(state.discussions.indexOf(oldDiscussion), 1, newDiscussion); + } else { + state.discussions.push(newDiscussion); + } }); } else { - const oldNote = utils.findNoteObjectById(state.discussions, discussion.id); + const oldDiscussion = utils.findNoteObjectById(state.discussions, discussion.id); - acc.push({ - ...discussion, - ...diffData, - expanded: oldNote ? oldNote.expanded : discussion.expanded, - }); + if (oldDiscussion) { + state.discussions.splice(state.discussions.indexOf(oldDiscussion), 1, { + ...discussion, + ...diffData, + expanded: oldDiscussion.expanded, + }); + } else { + state.discussions.push({ ...discussion, ...diffData }); + } } - - return acc; - }, []); - - Object.assign(state, { discussions }); + }); }, + [types.SET_LAST_FETCHED_AT](state, fetchedAt) { Object.assign(state, { lastFetchedAt: fetchedAt }); }, |