From c15d02c3440959e281196261f6b9d3f929d40940 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Tue, 18 Apr 2017 15:22:59 +0530 Subject: [PATCH 01/35] [WIP] Add support for creating comments instantly --- app/assets/javascripts/notes.js | 64 +++++++++++++++++++ app/assets/stylesheets/pages/notes.scss | 4 ++ .../shared/notes/_comment_button.html.haml | 2 + 3 files changed, 70 insertions(+) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 87f03a40eba3fb..b4fbfc405a71e4 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -47,6 +47,7 @@ const normalizeNewlines = function(str) { this.refresh = bind(this.refresh, this); this.keydownNoteText = bind(this.keydownNoteText, this); this.toggleCommitList = bind(this.toggleCommitList, this); + this.postComment = bind(this.postComment, this); this.notes_url = notes_url; this.note_ids = note_ids; @@ -94,6 +95,7 @@ const normalizeNewlines = function(str) { $(document).on("click", ".note-edit-cancel", this.cancelEdit); // Reopen and close actions for Issue/MR combined with note form submit $(document).on("click", ".js-comment-button", this.updateCloseButton); + $(document).on("click", ".js-insta-comment-button", this.postComment); $(document).on("keyup input", ".js-note-text", this.updateTargetButtons); // resolve a discussion $(document).on('click', '.js-comment-resolve-button', this.resolveDiscussion); @@ -1150,6 +1152,68 @@ const normalizeNewlines = function(str) { return $updatedNote; }; + Notes.prototype.createTemporaryNote = function($sampleNote, formContent, uniqueId) { + const $tempNote = $sampleNote.clone(); + + // Set unique ID for later reference. + $tempNote.attr('id', uniqueId); + $tempNote.addClass('being-posted'); + $tempNote.find('.note-headline-meta a').html(''); + + // Check if current user is same as what tempLi meta holds + if ($tempNote.data('author-id') !== gon.current_user_id) { + $tempNote.data('author-id', gon.current_user_id); + $tempNote.find('.timeline-icon > a, .note-header-info > a').each((i, anchor) => { + $(anchor).attr('href', `/${gon.current_username}`); + }); + $tempNote.find('.note-header-info a .hidden-xs').text(gon.current_user_fullname); + $tempNote.find('.note-header-info a .note-headline-light').text(`@${gon.current_user_fullname}`); + } + + // Update note body + const $noteBody = $tempNote.find('.js-task-list-container'); + $noteBody.find('.md.note-text').html(formContent); // If we ever preview it, use preview instead! + $noteBody.find('.original-note-content').text(formContent); + $noteBody.find('.js-task-list-field').text(formContent); + + return $tempNote; + }; + + Notes.prototype.postComment = function(e) { + const self = this; + const $form = $(e.target).parents('form'); + const formData = $form.serialize(); + const formContent = $form.find('.js-note-text').val(); + // const formContentMD = $form.find('.js-md-preview').html(); + const uniqueId = Date.now(); + let $notesContainer; + let $lastLi; + + if ($form.hasClass('js-discussion-note-form')) { + $notesContainer = $form.parent('.discussion-notes').find('.notes'); + $lastLi = $notesContainer.find('.note:not(.system-note)').last(); + } else if ($form.hasClass('js-main-target-form')) { + $notesContainer = $('ul.main-notes-list'); + $lastLi = $notesContainer.find('.note:not(.system-note)').last(); + } + + $notesContainer.append(this.createTemporaryNote($lastLi, formContent, uniqueId)); + $form.find('.js-note-text').val(''); + + $.ajax({ + type: 'POST', + url: $form.attr('action'), + data: formData, + success(note) { + $notesContainer.find(`#${uniqueId}`).remove(); + self.note_ids.push(note.id); + $notesContainer.append(note.html).syntaxHighlight(); + gl.utils.localTimeAgo($notesContainer.find("#note_" + note.id + " .js-timeago"), false); + self.updateNotesCount(1); + } + }); + }; + return Notes; })(); }).call(window); diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index f89150ebead7ed..470c5c29ca6376 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -57,6 +57,10 @@ ul.notes { position: relative; border-bottom: 1px solid $white-normal; + &.being-posted { + opacity: 0.5; + } + &.note-discussion { &.timeline-entry { padding: 14px 10px; diff --git a/app/views/shared/notes/_comment_button.html.haml b/app/views/shared/notes/_comment_button.html.haml index 29cf5825292e33..ab4570378d7c2f 100644 --- a/app/views/shared/notes/_comment_button.html.haml +++ b/app/views/shared/notes/_comment_button.html.haml @@ -28,3 +28,5 @@ Discuss a specific suggestion or question - if @note.noteable.supports_resolvable_notes? that needs to be resolved + +%input.btn.btn-nr.btn-create.comment-btn.js-insta-comment-button{ type: 'button', value: 'Insta Comment' } -- GitLab From a202bea0d4c30463c8e51d0c9c316c6ade85ba90 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Wed, 19 Apr 2017 14:47:02 +0530 Subject: [PATCH 02/35] [ci skip] [WIP] Add support for updating comments instantly --- app/assets/javascripts/notes.js | 33 +++++++++++++++++++-- app/views/shared/notes/_edit_form.html.haml | 2 ++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index b4fbfc405a71e4..957fad00f4db12 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -96,6 +96,7 @@ const normalizeNewlines = function(str) { // Reopen and close actions for Issue/MR combined with note form submit $(document).on("click", ".js-comment-button", this.updateCloseButton); $(document).on("click", ".js-insta-comment-button", this.postComment); + $(document).on("click", ".js-insta-save-button", this.updateComment.bind(this)); $(document).on("keyup input", ".js-note-text", this.updateTargetButtons); // resolve a discussion $(document).on('click', '.js-comment-resolve-button', this.resolveDiscussion); @@ -1152,8 +1153,8 @@ const normalizeNewlines = function(str) { return $updatedNote; }; - Notes.prototype.createTemporaryNote = function($sampleNote, formContent, uniqueId) { - const $tempNote = $sampleNote.clone(); + Notes.prototype.createPlaceholderNote = function($baseNote, formContent, uniqueId) { + const $tempNote = $baseNote.clone(); // Set unique ID for later reference. $tempNote.attr('id', uniqueId); @@ -1197,7 +1198,7 @@ const normalizeNewlines = function(str) { $lastLi = $notesContainer.find('.note:not(.system-note)').last(); } - $notesContainer.append(this.createTemporaryNote($lastLi, formContent, uniqueId)); + $notesContainer.append(this.createPlaceholderNote($lastLi, formContent, uniqueId)); $form.find('.js-note-text').val(''); $.ajax({ @@ -1214,6 +1215,32 @@ const normalizeNewlines = function(str) { }); }; + Notes.prototype.updateComment = function(e) { + const self = this; + const $form = $(e.target).parents('form'); + const formData = $form.serialize(); + const formContent = $form.find('.js-note-text').val(); + const $editingNote = $form.parents('.note.is-editting'); + const $noteBody = $editingNote.find('.js-task-list-container'); + + $noteBody.find('.md.note-text').html(formContent); // If we ever preview it, use preview instead! + $noteBody.find('.original-note-content').text(formContent); + $noteBody.find('.js-task-list-field').text(formContent); + + $editingNote.removeClass('is-editting'); + $editingNote.addClass('being-posted'); + $editingNote.find('.note-headline-meta a').html(''); + + $.ajax({ + type: 'POST', + url: $form.attr('action'), + data: formData, + success(note) { + self.updateNote(null, note, null); + } + }); + }; + return Notes; })(); }).call(window); diff --git a/app/views/shared/notes/_edit_form.html.haml b/app/views/shared/notes/_edit_form.html.haml index fc57b92d4a6caf..9cd79b9a6ec853 100644 --- a/app/views/shared/notes/_edit_form.html.haml +++ b/app/views/shared/notes/_edit_form.html.haml @@ -10,5 +10,7 @@ .settings-message.note-edit-warning.js-finish-edit-warning Finish editing this message first! = submit_tag 'Save comment', class: 'btn btn-nr btn-save js-comment-button' + %button.btn.btn-nr.btn-save.js-insta-save-button{ type: 'button' } + Insta save comment %button.btn.btn-nr.btn-cancel.note-edit-cancel{ type: 'button' } Cancel -- GitLab From 18174477541c06361c40656cb45fb624bc7a11c3 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Tue, 25 Apr 2017 15:28:49 +0530 Subject: [PATCH 03/35] [ci skip][WIP] Retain fadeIn for comment create/update --- app/assets/javascripts/notes.js | 2 ++ app/assets/stylesheets/framework/animations.scss | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 957fad00f4db12..344542a7a330c7 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -1176,6 +1176,7 @@ const normalizeNewlines = function(str) { $noteBody.find('.md.note-text').html(formContent); // If we ever preview it, use preview instead! $noteBody.find('.original-note-content').text(formContent); $noteBody.find('.js-task-list-field').text(formContent); + $tempNote.addClass('fade-in'); return $tempNote; }; @@ -1230,6 +1231,7 @@ const normalizeNewlines = function(str) { $editingNote.removeClass('is-editting'); $editingNote.addClass('being-posted'); $editingNote.find('.note-headline-meta a').html(''); + $editingNote.addClass('fade-in'); $.ajax({ type: 'POST', diff --git a/app/assets/stylesheets/framework/animations.scss b/app/assets/stylesheets/framework/animations.scss index 7c50b80fd2bb56..08738aebcf0135 100644 --- a/app/assets/stylesheets/framework/animations.scss +++ b/app/assets/stylesheets/framework/animations.scss @@ -152,7 +152,7 @@ a { } 100% { - opacity: 1; + opacity: 0.5; } } -- GitLab From ea56c41bca297b7cd5da82fa768e6bf5a24fb6e8 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Wed, 26 Apr 2017 13:27:28 +0530 Subject: [PATCH 04/35] [ci skip][WIP] Enable full fadeIn support for create/edit comments. --- app/assets/javascripts/notes.js | 5 ++++- app/assets/stylesheets/framework/animations.scss | 14 ++++++++++++++ app/assets/stylesheets/framework/variables.scss | 2 +- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 344542a7a330c7..6a525b34c5e507 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -602,6 +602,7 @@ const normalizeNewlines = function(str) { var $html, $note_li; // Convert returned HTML to a jQuery object so we can modify it further $html = $(noteEntity.html); + $html.addClass('fade-in-complete'); this.revertNoteEditForm(); gl.utils.localTimeAgo($('.js-timeago', $html)); $html.renderGFM(); @@ -1209,7 +1210,9 @@ const normalizeNewlines = function(str) { success(note) { $notesContainer.find(`#${uniqueId}`).remove(); self.note_ids.push(note.id); - $notesContainer.append(note.html).syntaxHighlight(); + const $note = $(note.html); + $note.addClass('fade-in-complete'); + $notesContainer.append($note).syntaxHighlight(); gl.utils.localTimeAgo($notesContainer.find("#note_" + note.id + " .js-timeago"), false); self.updateNotesCount(1); } diff --git a/app/assets/stylesheets/framework/animations.scss b/app/assets/stylesheets/framework/animations.scss index 08738aebcf0135..5a24d761c95e9f 100644 --- a/app/assets/stylesheets/framework/animations.scss +++ b/app/assets/stylesheets/framework/animations.scss @@ -156,6 +156,20 @@ a { } } +@keyframes fadeInComplete { + 0% { + opacity: 0.5; + } + + 100% { + opacity: 1; + } +} + .fade-in { animation: fadeIn $fade-in-duration 1; } + +.fade-in-complete { + animation: fadeInComplete $fade-in-duration 1; +} diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 60f02921c0a632..cd9e83cdc6303d 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -464,7 +464,7 @@ $label-border-radius: 100px; /* * Animation */ -$fade-in-duration: 200ms; +$fade-in-duration: 300ms; /* * Lint -- GitLab From 54b0fb68cc56ba5cfed44441f1f9b87d6e8f7b5c Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Wed, 26 Apr 2017 13:50:59 +0530 Subject: [PATCH 05/35] [ci skip][WIP] Add `getFormData` to remove some repetition, instant editing now ready --- app/assets/javascripts/notes.js | 40 +++++++++++---------- app/views/shared/notes/_edit_form.html.haml | 2 -- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 6a525b34c5e507..9928aa69c813a2 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -31,7 +31,7 @@ const normalizeNewlines = function(str) { function Notes(notes_url, note_ids, last_fetched_at, view) { this.updateTargetButtons = bind(this.updateTargetButtons, this); - this.updateCloseButton = bind(this.updateCloseButton, this); + this.updateComment = bind(this.updateComment, this); this.visibilityChange = bind(this.visibilityChange, this); this.cancelDiscussionForm = bind(this.cancelDiscussionForm, this); this.addDiffNote = bind(this.addDiffNote, this); @@ -94,9 +94,8 @@ const normalizeNewlines = function(str) { $(document).on("click", ".js-note-edit", this.showEditForm.bind(this)); $(document).on("click", ".note-edit-cancel", this.cancelEdit); // Reopen and close actions for Issue/MR combined with note form submit - $(document).on("click", ".js-comment-button", this.updateCloseButton); + $(document).on("click", ".js-comment-button", this.updateComment); $(document).on("click", ".js-insta-comment-button", this.postComment); - $(document).on("click", ".js-insta-save-button", this.updateComment.bind(this)); $(document).on("keyup input", ".js-note-text", this.updateTargetButtons); // resolve a discussion $(document).on('click', '.js-comment-resolve-button', this.resolveDiscussion); @@ -986,14 +985,6 @@ const normalizeNewlines = function(str) { return this.refresh(); }; - Notes.prototype.updateCloseButton = function(e) { - var closebtn, form, textarea; - textarea = $(e.target); - form = textarea.parents('form'); - closebtn = form.find('.js-note-target-close'); - return closebtn.text(closebtn.data('original-text')); - }; - Notes.prototype.updateTargetButtons = function(e) { var closebtn, closetext, discardbtn, form, reopenbtn, reopentext, textarea; textarea = $(e.target); @@ -1154,6 +1145,17 @@ const normalizeNewlines = function(str) { return $updatedNote; }; + Notes.prototype.getFormData = function(target) { + const $form = $(target).parents('form'); + + return { + $form, + formData: $form.serialize(), + formContent: $form.find('.js-note-text').val(), + formAction: $form.attr('action'), + }; + }; + Notes.prototype.createPlaceholderNote = function($baseNote, formContent, uniqueId) { const $tempNote = $baseNote.clone(); @@ -1184,9 +1186,7 @@ const normalizeNewlines = function(str) { Notes.prototype.postComment = function(e) { const self = this; - const $form = $(e.target).parents('form'); - const formData = $form.serialize(); - const formContent = $form.find('.js-note-text').val(); + const { $form, formData, formContent, formAction } = self.getFormData(e.target); // const formContentMD = $form.find('.js-md-preview').html(); const uniqueId = Date.now(); let $notesContainer; @@ -1205,7 +1205,7 @@ const normalizeNewlines = function(str) { $.ajax({ type: 'POST', - url: $form.attr('action'), + url: formAction, data: formData, success(note) { $notesContainer.find(`#${uniqueId}`).remove(); @@ -1220,10 +1220,10 @@ const normalizeNewlines = function(str) { }; Notes.prototype.updateComment = function(e) { + e.preventDefault(); const self = this; - const $form = $(e.target).parents('form'); - const formData = $form.serialize(); - const formContent = $form.find('.js-note-text').val(); + const { $form, formData, formContent, formAction } = self.getFormData(e.target); + const $closeBtn = $form.find('.js-note-target-close'); const $editingNote = $form.parents('.note.is-editting'); const $noteBody = $editingNote.find('.js-task-list-container'); @@ -1238,12 +1238,14 @@ const normalizeNewlines = function(str) { $.ajax({ type: 'POST', - url: $form.attr('action'), + url: formAction, data: formData, success(note) { self.updateNote(null, note, null); } }); + + return $closeBtn.text($closeBtn.data('original-text')); }; return Notes; diff --git a/app/views/shared/notes/_edit_form.html.haml b/app/views/shared/notes/_edit_form.html.haml index 9cd79b9a6ec853..fc57b92d4a6caf 100644 --- a/app/views/shared/notes/_edit_form.html.haml +++ b/app/views/shared/notes/_edit_form.html.haml @@ -10,7 +10,5 @@ .settings-message.note-edit-warning.js-finish-edit-warning Finish editing this message first! = submit_tag 'Save comment', class: 'btn btn-nr btn-save js-comment-button' - %button.btn.btn-nr.btn-save.js-insta-save-button{ type: 'button' } - Insta save comment %button.btn.btn-nr.btn-cancel.note-edit-cancel{ type: 'button' } Cancel -- GitLab From 0c9e324b7a32eb0bae3795efa9277ed6676cc8d4 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Wed, 26 Apr 2017 18:08:10 +0530 Subject: [PATCH 06/35] [ci skip][WIP] Create and update comments is now fully functional --- app/assets/javascripts/notes.js | 141 ++++++++++-------- app/assets/stylesheets/pages/notes.scss | 14 ++ app/views/discussions/_notes.html.haml | 1 + .../shared/notes/_comment_button.html.haml | 2 - app/views/shared/notes/_edit_form.html.haml | 2 +- 5 files changed, 92 insertions(+), 68 deletions(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 9928aa69c813a2..c86fd649d1c5aa 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -83,19 +83,12 @@ const normalizeNewlines = function(str) { }; Notes.prototype.addBinding = function() { - // add note to UI after creation - $(document).on("ajax:success", ".js-main-target-form", this.addNote); - $(document).on("ajax:success", ".js-discussion-note-form", this.addDiscussionNote); - // catch note ajax errors - $(document).on("ajax:error", ".js-main-target-form", this.addNoteError); - // change note in UI after update - $(document).on("ajax:success", "form.edit-note", this.updateNote); // Edit note link $(document).on("click", ".js-note-edit", this.showEditForm.bind(this)); $(document).on("click", ".note-edit-cancel", this.cancelEdit); // Reopen and close actions for Issue/MR combined with note form submit - $(document).on("click", ".js-comment-button", this.updateComment); - $(document).on("click", ".js-insta-comment-button", this.postComment); + $(document).on("click", ".js-comment-submit-button", this.postComment); + $(document).on("click", ".js-comment-save-button", this.updateComment); $(document).on("keyup input", ".js-note-text", this.updateTargetButtons); // resolve a discussion $(document).on('click', '.js-comment-resolve-button', this.resolveDiscussion); @@ -103,9 +96,6 @@ const normalizeNewlines = function(str) { $(document).on("click", ".js-note-delete", this.removeNote); // delete note attachment $(document).on("click", ".js-note-attachment-delete", this.removeAttachment); - // reset main target form after submit - $(document).on("ajax:complete", ".js-main-target-form", this.reenableTargetFormSubmitButton); - $(document).on("ajax:success", ".js-main-target-form", this.resetMainTargetForm); // reset main target form when clicking discard $(document).on("click", ".js-note-discard", this.resetMainTargetForm); // update the file name when an attachment is selected @@ -127,9 +117,6 @@ const normalizeNewlines = function(str) { }; Notes.prototype.cleanBinding = function() { - $(document).off("ajax:success", ".js-main-target-form"); - $(document).off("ajax:success", ".js-discussion-note-form"); - $(document).off("ajax:success", "form.edit-note"); $(document).off("click", ".js-note-edit"); $(document).off("click", ".note-edit-cancel"); $(document).off("click", ".js-note-delete"); @@ -558,13 +545,23 @@ const normalizeNewlines = function(str) { Adds new note to list. */ - Notes.prototype.addNote = function(xhr, note, status) { + Notes.prototype.addNote = function($form, note) { this.handleCreateChanges(note); return this.renderNote(note); }; - Notes.prototype.addNoteError = function(xhr, note, status) { - return new Flash('Your comment could not be submitted! Please check your network connection and try again.', 'alert', this.parentTimeline); + Notes.prototype.addNoteError = function($form) { + let formParentTimeline; + if ($form.hasClass('js-main-target-form')) { + formParentTimeline = $form.parents('.timeline'); + } else if ($form.hasClass('js-discussion-note-form')) { + formParentTimeline = $form.closest('.discussion-notes').find('.notes'); + } + return new Flash('Your comment could not be submitted! Please check your network connection and try again.', 'alert', formParentTimeline); + }; + + Notes.prototype.updateNoteError = function($parentTimeline) { + return new Flash('Your comment could not be updated! Please check your network connection and try again.'); }; /* @@ -573,9 +570,7 @@ const normalizeNewlines = function(str) { Adds new note to list. */ - Notes.prototype.addDiscussionNote = function(xhr, note, status) { - var $form = $(xhr.target); - + Notes.prototype.addDiscussionNote = function($form, note) { if ($form.attr('data-resolve-all') != null) { var projectPath = $form.data('project-path'); var discussionId = $form.data('discussion-id'); @@ -701,7 +696,7 @@ const normalizeNewlines = function(str) { var $editForm = $(selector); $editForm.insertBefore('.notes-form'); - $editForm.find('.js-comment-button').enable(); + $editForm.find('.js-comment-save-button').enable(); $editForm.find('.js-finish-edit-warning').hide(); }; @@ -1156,51 +1151,53 @@ const normalizeNewlines = function(str) { }; }; - Notes.prototype.createPlaceholderNote = function($baseNote, formContent, uniqueId) { - const $tempNote = $baseNote.clone(); - - // Set unique ID for later reference. - $tempNote.attr('id', uniqueId); - $tempNote.addClass('being-posted'); - $tempNote.find('.note-headline-meta a').html(''); - - // Check if current user is same as what tempLi meta holds - if ($tempNote.data('author-id') !== gon.current_user_id) { - $tempNote.data('author-id', gon.current_user_id); - $tempNote.find('.timeline-icon > a, .note-header-info > a').each((i, anchor) => { - $(anchor).attr('href', `/${gon.current_username}`); - }); - $tempNote.find('.note-header-info a .hidden-xs').text(gon.current_user_fullname); - $tempNote.find('.note-header-info a .note-headline-light').text(`@${gon.current_user_fullname}`); - } - - // Update note body - const $noteBody = $tempNote.find('.js-task-list-container'); - $noteBody.find('.md.note-text').html(formContent); // If we ever preview it, use preview instead! - $noteBody.find('.original-note-content').text(formContent); - $noteBody.find('.js-task-list-field').text(formContent); - $tempNote.addClass('fade-in'); + Notes.prototype.createPlaceholderNote = function(formContent, uniqueId) { + const $tempNote = $( + `
  • +
    +
    + +
    +
    +
    +
    + + + @${gon.current_username} + + + + +
    +
    +
    +
    ${formContent}
    +
    +
    +
    +
    • ` + ); return $tempNote; }; Notes.prototype.postComment = function(e) { + e.preventDefault(); const self = this; const { $form, formData, formContent, formAction } = self.getFormData(e.target); - // const formContentMD = $form.find('.js-md-preview').html(); + const $closeBtn = $form.find('.js-note-target-close'); + const isMainForm = $form.hasClass('js-main-target-form'); + const isDiscussionForm = $form.hasClass('js-discussion-note-form'); const uniqueId = Date.now(); let $notesContainer; - let $lastLi; - if ($form.hasClass('js-discussion-note-form')) { + if (isDiscussionForm) { $notesContainer = $form.parent('.discussion-notes').find('.notes'); - $lastLi = $notesContainer.find('.note:not(.system-note)').last(); - } else if ($form.hasClass('js-main-target-form')) { + } else if (isMainForm) { $notesContainer = $('ul.main-notes-list'); - $lastLi = $notesContainer.find('.note:not(.system-note)').last(); } - $notesContainer.append(this.createPlaceholderNote($lastLi, formContent, uniqueId)); + $notesContainer.append(this.createPlaceholderNote(formContent, uniqueId)); $form.find('.js-note-text').val(''); $.ajax({ @@ -1209,14 +1206,21 @@ const normalizeNewlines = function(str) { data: formData, success(note) { $notesContainer.find(`#${uniqueId}`).remove(); - self.note_ids.push(note.id); - const $note = $(note.html); - $note.addClass('fade-in-complete'); - $notesContainer.append($note).syntaxHighlight(); - gl.utils.localTimeAgo($notesContainer.find("#note_" + note.id + " .js-timeago"), false); - self.updateNotesCount(1); + if (isDiscussionForm) { + self.addDiscussionNote($form, note); + } else if (isMainForm) { + self.addNote($form, note); + self.reenableTargetFormSubmitButton(e); + self.resetMainTargetForm(e); + } + }, + error() { + $notesContainer.find(`#${uniqueId}`).remove(); + self.addNoteError($form); } }); + + return $closeBtn.text($closeBtn.data('original-text')); }; Notes.prototype.updateComment = function(e) { @@ -1226,15 +1230,13 @@ const normalizeNewlines = function(str) { const $closeBtn = $form.find('.js-note-target-close'); const $editingNote = $form.parents('.note.is-editting'); const $noteBody = $editingNote.find('.js-task-list-container'); + const $noteBodyText = $noteBody.find('.note-text'); - $noteBody.find('.md.note-text').html(formContent); // If we ever preview it, use preview instead! - $noteBody.find('.original-note-content').text(formContent); - $noteBody.find('.js-task-list-field').text(formContent); + const cachedNoteBodyText = $noteBodyText.html(); + $noteBodyText.html(formContent); - $editingNote.removeClass('is-editting'); - $editingNote.addClass('being-posted'); + $editingNote.removeClass('is-editting').addClass('being-posted fade-in'); $editingNote.find('.note-headline-meta a').html(''); - $editingNote.addClass('fade-in'); $.ajax({ type: 'POST', @@ -1242,6 +1244,15 @@ const normalizeNewlines = function(str) { data: formData, success(note) { self.updateNote(null, note, null); + }, + error() { + // Revert back to original note + $noteBodyText.html(cachedNoteBodyText); + $editingNote.removeClass('being-posted fade-in'); + $editingNote.find('.fa.fa-spinner').remove(); + + // Show Flash message about failure + self.updateNoteError(); } }); diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 470c5c29ca6376..7977c5548151ea 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -59,6 +59,20 @@ ul.notes { &.being-posted { opacity: 0.5; + + .dummy-avatar { + display: inline-block; + height: 40px; + width: 40px; + border-radius: 50%; + background-color: $kdb-border; + border: 1px solid darken($kdb-border, 25%); + } + + .note-headline-light, + .fa-spinner { + margin-left: 3px; + } } &.note-discussion { diff --git a/app/views/discussions/_notes.html.haml b/app/views/discussions/_notes.html.haml index 964473ee3e08c0..7ba3f3f6c42a5d 100644 --- a/app/views/discussions/_notes.html.haml +++ b/app/views/discussions/_notes.html.haml @@ -1,6 +1,7 @@ .discussion-notes %ul.notes{ data: { discussion_id: discussion.id } } = render partial: "shared/notes/note", collection: discussion.notes, as: :note + .flash-container - if current_user .discussion-reply-holder diff --git a/app/views/shared/notes/_comment_button.html.haml b/app/views/shared/notes/_comment_button.html.haml index ab4570378d7c2f..29cf5825292e33 100644 --- a/app/views/shared/notes/_comment_button.html.haml +++ b/app/views/shared/notes/_comment_button.html.haml @@ -28,5 +28,3 @@ Discuss a specific suggestion or question - if @note.noteable.supports_resolvable_notes? that needs to be resolved - -%input.btn.btn-nr.btn-create.comment-btn.js-insta-comment-button{ type: 'button', value: 'Insta Comment' } diff --git a/app/views/shared/notes/_edit_form.html.haml b/app/views/shared/notes/_edit_form.html.haml index fc57b92d4a6caf..8923e5602a41f5 100644 --- a/app/views/shared/notes/_edit_form.html.haml +++ b/app/views/shared/notes/_edit_form.html.haml @@ -9,6 +9,6 @@ .note-form-actions.clearfix .settings-message.note-edit-warning.js-finish-edit-warning Finish editing this message first! - = submit_tag 'Save comment', class: 'btn btn-nr btn-save js-comment-button' + = submit_tag 'Save comment', class: 'btn btn-nr btn-save js-comment-save-button' %button.btn.btn-nr.btn-cancel.note-edit-cancel{ type: 'button' } Cancel -- GitLab From 300e7be8fd1fbbc189a6b7ab422d7c48b793fc2c Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Wed, 26 Apr 2017 19:08:38 +0530 Subject: [PATCH 07/35] Fix `Ctrl+Enter` support for comment submit, styling updates --- app/assets/javascripts/behaviors/quick_submit.js | 2 +- app/assets/javascripts/notes.js | 4 ++-- app/assets/stylesheets/pages/notes.scss | 4 ++++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/behaviors/quick_submit.js b/app/assets/javascripts/behaviors/quick_submit.js index 3d162b244135c6..1f9e0448084511 100644 --- a/app/assets/javascripts/behaviors/quick_submit.js +++ b/app/assets/javascripts/behaviors/quick_submit.js @@ -43,8 +43,8 @@ $(document).on('keydown.quick_submit', '.js-quick-submit', (e) => { const $submitButton = $form.find('input[type=submit], button[type=submit]'); if (!$submitButton.attr('disabled')) { + $submitButton.trigger('click', [e]); $submitButton.disable(); - $form.submit(); } }); diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index c86fd649d1c5aa..20d144aa77c618 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -121,8 +121,6 @@ const normalizeNewlines = function(str) { $(document).off("click", ".note-edit-cancel"); $(document).off("click", ".js-note-delete"); $(document).off("click", ".js-note-attachment-delete"); - $(document).off("ajax:complete", ".js-main-target-form"); - $(document).off("ajax:success", ".js-main-target-form"); $(document).off("click", ".js-discussion-reply-button"); $(document).off("click", ".js-add-diff-note-button"); $(document).off("visibilitychange"); @@ -1207,7 +1205,9 @@ const normalizeNewlines = function(str) { success(note) { $notesContainer.find(`#${uniqueId}`).remove(); if (isDiscussionForm) { + $notesContainer.find('.flash-container').remove(); self.addDiscussionNote($form, note); + $notesContainer.append(''); } else if (isMainForm) { self.addNote($form, note); self.reenableTargetFormSubmitButton(e); diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 7977c5548151ea..453560c2986f78 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -705,6 +705,10 @@ ul.notes { } } +.discussion-notes .flash-container { + margin-bottom: 0; +} + // Merge request notes in diffs .diff-file { // Diff is side by side -- GitLab From 255a22fbf46d52ab5704b6d98628e0e131d6ee20 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 27 Apr 2017 15:54:36 +0530 Subject: [PATCH 08/35] Revert fade-in duration back to `200ms` --- app/assets/stylesheets/framework/variables.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index cd9e83cdc6303d..60f02921c0a632 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -464,7 +464,7 @@ $label-border-radius: 100px; /* * Animation */ -$fade-in-duration: 300ms; +$fade-in-duration: 200ms; /* * Lint -- GitLab From 2c5e589f1be795e2ac2e85c5aab5476510778363 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 27 Apr 2017 15:54:55 +0530 Subject: [PATCH 09/35] Add new animation classes for fadeInHalf and fadeInFull --- .../stylesheets/framework/animations.scss | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/app/assets/stylesheets/framework/animations.scss b/app/assets/stylesheets/framework/animations.scss index 5a24d761c95e9f..3cd7f81da4719c 100644 --- a/app/assets/stylesheets/framework/animations.scss +++ b/app/assets/stylesheets/framework/animations.scss @@ -151,12 +151,30 @@ a { opacity: 0; } + 100% { + opacity: 1; + } +} + +.fade-in { + animation: fadeIn $fade-in-duration 1; +} + +@keyframes fadeInHalf { + 0% { + opacity: 0; + } + 100% { opacity: 0.5; } } -@keyframes fadeInComplete { +.fade-in-half { + animation: fadeInHalf $fade-in-duration 1; +} + +@keyframes fadeInFull { 0% { opacity: 0.5; } @@ -166,10 +184,6 @@ a { } } -.fade-in { - animation: fadeIn $fade-in-duration 1; -} - -.fade-in-complete { - animation: fadeInComplete $fade-in-duration 1; +.fade-in-full { + animation: fadeInFull $fade-in-duration 1; } -- GitLab From edf90d29ba93522e3a3f5a05e20694efbdb8606a Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 27 Apr 2017 15:55:32 +0530 Subject: [PATCH 10/35] Add method to generate random unique GUID --- app/assets/javascripts/lib/utils/common_utils.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index 8058672eaa90e0..b9e9850be7e816 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -371,6 +371,14 @@ }); }; + /** + * Generates random unique GUID, eg; f4ccf724-4cc3-93c5-0912-e7b8d440a832 + */ + w.gl.utils.guid = () => { + const s4 = () => Math.floor((1 + Math.random()) * 0x10000).toString(16).substring(1); + return `${s4()}${s4()}-${s4()}-${s4()}-${s4()}-${s4()}${s4()}${s4()}`; + }; + w.gl.utils.setFavicon = (faviconPath) => { if (faviconEl && faviconPath) { faviconEl.setAttribute('href', faviconPath); -- GitLab From 98d6165690270b90b1d749ec42ce14d1331efbd9 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 27 Apr 2017 15:56:05 +0530 Subject: [PATCH 11/35] Fix discussion resolve, add meaningful comments --- app/assets/javascripts/notes.js | 146 +++++++++++++++++++++++--------- 1 file changed, 105 insertions(+), 41 deletions(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 20d144aa77c618..e16bc57e8068e1 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -91,7 +91,7 @@ const normalizeNewlines = function(str) { $(document).on("click", ".js-comment-save-button", this.updateComment); $(document).on("keyup input", ".js-note-text", this.updateTargetButtons); // resolve a discussion - $(document).on('click', '.js-comment-resolve-button', this.resolveDiscussion); + $(document).on('click', '.js-comment-resolve-button', this.postComment); // remove a note (in general) $(document).on("click", ".js-note-delete", this.removeNote); // delete note attachment @@ -594,7 +594,7 @@ const normalizeNewlines = function(str) { var $html, $note_li; // Convert returned HTML to a jQuery object so we can modify it further $html = $(noteEntity.html); - $html.addClass('fade-in-complete'); + $html.addClass('fade-in-full'); this.revertNoteEditForm(); gl.utils.localTimeAgo($('.js-timeago', $html)); $html.renderGFM(); @@ -1066,17 +1066,6 @@ const normalizeNewlines = function(str) { return this.notesCountBadge.text(parseInt(this.notesCountBadge.text(), 10) + updateCount); }; - Notes.prototype.resolveDiscussion = function() { - var $this = $(this); - var discussionId = $this.attr('data-discussion-id'); - - $this - .closest('form') - .attr('data-discussion-id', discussionId) - .attr('data-resolve-all', 'true') - .attr('data-project-path', $this.attr('data-project-path')); - }; - Notes.prototype.toggleCommitList = function(e) { const $element = $(e.currentTarget); const $closestSystemCommitList = $element.siblings('.system-note-commit-list'); @@ -1125,7 +1114,7 @@ const normalizeNewlines = function(str) { Notes.animateAppendNote = function(noteHtml, $notesList) { const $note = $(noteHtml); - $note.addClass('fade-in').renderGFM(); + $note.addClass('fade-in-full').renderGFM(); $notesList.append($note); return $note; }; @@ -1138,20 +1127,26 @@ const normalizeNewlines = function(str) { return $updatedNote; }; - Notes.prototype.getFormData = function(target) { - const $form = $(target).parents('form'); - + /** + * Get data from Form attributes to use for saving/submitting comment. + */ + Notes.prototype.getFormData = function($form) { return { - $form, formData: $form.serialize(), formContent: $form.find('.js-note-text').val(), formAction: $form.attr('action'), }; }; + /** + * Create placeholder note DOM element populated with comment body + * that we will show while comment is being posted. + * Once comment is _actually_ posted on server, we will have final element + * in response that we will show in place of this temporary element. + */ Notes.prototype.createPlaceholderNote = function(formContent, uniqueId) { const $tempNote = $( - `
  • + `
  • @@ -1164,7 +1159,7 @@ const normalizeNewlines = function(str) { @${gon.current_username} - +
    @@ -1179,80 +1174,149 @@ const normalizeNewlines = function(str) { return $tempNote; }; + /** + * This method does following tasks step-by-step whenever a new comment + * is submitted by user (both main thread comments as well as discussion comments). + * + * 1) Get Form metadata + * 2) Identify comment type; a) Main thread b) Discussion thread c) Discussion resolve + * 3) Build temporary placeholder element (using `createPlaceholderNote`) + * 4) Show placeholder note on UI + * 5) Perform network request to submit the note + * a) If request is successfully completed + * 1. Remove placeholder element + * 2. Show submitted Note element + * 3. Perform post-submit errands + * a. Mark discussion as resolved if comment submission was for resolve. + * b. Reset comment form to original state. + * b) If request failed + * 1. Remove placeholder element + * 2. Show error Flash message about failure + */ Notes.prototype.postComment = function(e) { e.preventDefault(); - const self = this; - const { $form, formData, formContent, formAction } = self.getFormData(e.target); + + // Get Form metadata + const $submitBtn = $(e.target); + const $form = $submitBtn.parents('form'); const $closeBtn = $form.find('.js-note-target-close'); const isMainForm = $form.hasClass('js-main-target-form'); const isDiscussionForm = $form.hasClass('js-discussion-note-form'); - const uniqueId = Date.now(); + const isDiscussionResolve = $submitBtn.hasClass('js-comment-resolve-button'); + const { formData, formContent, formAction } = this.getFormData($form); + const uniqueId = gl.utils.guid(); let $notesContainer; + // Get reference to notes container based on type of comment if (isDiscussionForm) { $notesContainer = $form.parent('.discussion-notes').find('.notes'); } else if (isMainForm) { $notesContainer = $('ul.main-notes-list'); } + // If comment is to resolve discussion, disable submit buttons while + // comment posting is finished. + if (isDiscussionResolve) { + $submitBtn.disable(); + $form.find('.js-comment-submit-button').disable(); + } + + // Show placeholder note and clear the form textarea $notesContainer.append(this.createPlaceholderNote(formContent, uniqueId)); $form.find('.js-note-text').val(''); + // Make request to submit comment on server $.ajax({ type: 'POST', url: formAction, data: formData, - success(note) { + success: (note) => { + // Submission successful! remove placeholder $notesContainer.find(`#${uniqueId}`).remove(); + + // Check if this was discussion comment if (isDiscussionForm) { + // Remove flash-container $notesContainer.find('.flash-container').remove(); - self.addDiscussionNote($form, note); + + // If comment intends to resolve discussion, do the same. + if (isDiscussionResolve) { + $form + .attr('data-discussion-id', $submitBtn.data('discussion-id')) + .attr('data-resolve-all', 'true') + .attr('data-project-path', $submitBtn.data('project-path')); + } + + // Show final note element on UI and append flash-container after that + this.addDiscussionNote($form, note); $notesContainer.append(''); - } else if (isMainForm) { - self.addNote($form, note); - self.reenableTargetFormSubmitButton(e); - self.resetMainTargetForm(e); + } else if (isMainForm) { // Check if this was main thread comment + // Show final note element on UI and perform form and action buttons cleanup + this.addNote($form, note); + this.reenableTargetFormSubmitButton(e); + this.resetMainTargetForm(e); } }, - error() { + error: () => { + // Submission failed, remove placeholder note and show Flash error message $notesContainer.find(`#${uniqueId}`).remove(); - self.addNoteError($form); + this.addNoteError($form); } }); return $closeBtn.text($closeBtn.data('original-text')); }; + /** + * This method does following tasks step-by-step whenever an existing comment + * is updated by user (both main thread comments as well as discussion comments). + * + * 1) Get Form metadata + * 2) Update note element with new content + * 3) Perform network request to submit the updated note + * a) If request is successfully completed + * 1. Show submitted Note element + * b) If request failed + * 1. Revert Note element to original content + * 2. Show error Flash message about failure + */ Notes.prototype.updateComment = function(e) { e.preventDefault(); - const self = this; - const { $form, formData, formContent, formAction } = self.getFormData(e.target); + + // Get Form metadata + const $submitBtn = $(e.target); + const $form = $submitBtn.parents('form'); const $closeBtn = $form.find('.js-note-target-close'); const $editingNote = $form.parents('.note.is-editting'); const $noteBody = $editingNote.find('.js-task-list-container'); const $noteBodyText = $noteBody.find('.note-text'); + const { formData, formContent, formAction } = this.getFormData($form); + // Cache original comment content const cachedNoteBodyText = $noteBodyText.html(); - $noteBodyText.html(formContent); - $editingNote.removeClass('is-editting').addClass('being-posted fade-in'); - $editingNote.find('.note-headline-meta a').html(''); + // Show updated comment content temporarily + $noteBodyText.html(formContent); + $editingNote.removeClass('is-editting').addClass('being-posted fade-in-half'); + $editingNote.find('.note-headline-meta a').html(''); + // Make request to update comment on server $.ajax({ type: 'POST', url: formAction, data: formData, - success(note) { - self.updateNote(null, note, null); + success: (note) => { + // Submission successful! render final note element + this.updateNote(null, note, null); }, - error() { - // Revert back to original note + error: () => { + // Submission failed, revert back to original note $noteBodyText.html(cachedNoteBodyText); $editingNote.removeClass('being-posted fade-in'); $editingNote.find('.fa.fa-spinner').remove(); // Show Flash message about failure - self.updateNoteError(); + this.updateNoteError(); } }); -- GitLab From 62f54cd1682c9fb3be61d25043928ec24aa6030e Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Fri, 28 Apr 2017 14:04:06 +0530 Subject: [PATCH 12/35] [ci skip] Make call to `$.ajax` reusable --- app/assets/javascripts/notes.js | 44 ++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index e16bc57e8068e1..a0d2021d3e4b5b 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -1138,6 +1138,17 @@ const normalizeNewlines = function(str) { }; }; + /** + * Perform network request to submit form contents and return jQuery Defferred. + */ + Notes.prototype.submitComment = function(formAction, formData) { + return $.ajax({ + type: 'POST', + url: formAction, + data: formData, + }); + }; + /** * Create placeholder note DOM element populated with comment body * that we will show while comment is being posted. @@ -1182,7 +1193,7 @@ const normalizeNewlines = function(str) { * 2) Identify comment type; a) Main thread b) Discussion thread c) Discussion resolve * 3) Build temporary placeholder element (using `createPlaceholderNote`) * 4) Show placeholder note on UI - * 5) Perform network request to submit the note + * 5) Perform network request to submit the note using `submitComment` * a) If request is successfully completed * 1. Remove placeholder element * 2. Show submitted Note element @@ -1225,12 +1236,10 @@ const normalizeNewlines = function(str) { $notesContainer.append(this.createPlaceholderNote(formContent, uniqueId)); $form.find('.js-note-text').val(''); + /* eslint-disable promise/catch-or-return */ // Make request to submit comment on server - $.ajax({ - type: 'POST', - url: formAction, - data: formData, - success: (note) => { + this.submitComment(formAction, formData) + .then((note) => { // Submission successful! remove placeholder $notesContainer.find(`#${uniqueId}`).remove(); @@ -1256,13 +1265,11 @@ const normalizeNewlines = function(str) { this.reenableTargetFormSubmitButton(e); this.resetMainTargetForm(e); } - }, - error: () => { + }).fail(() => { // Submission failed, remove placeholder note and show Flash error message $notesContainer.find(`#${uniqueId}`).remove(); this.addNoteError($form); - } - }); + }); return $closeBtn.text($closeBtn.data('original-text')); }; @@ -1273,7 +1280,7 @@ const normalizeNewlines = function(str) { * * 1) Get Form metadata * 2) Update note element with new content - * 3) Perform network request to submit the updated note + * 3) Perform network request to submit the updated note using `submitComment` * a) If request is successfully completed * 1. Show submitted Note element * b) If request failed @@ -1300,16 +1307,14 @@ const normalizeNewlines = function(str) { $editingNote.removeClass('is-editting').addClass('being-posted fade-in-half'); $editingNote.find('.note-headline-meta a').html(''); + /* eslint-disable promise/catch-or-return */ // Make request to update comment on server - $.ajax({ - type: 'POST', - url: formAction, - data: formData, - success: (note) => { + this.submitComment(formAction, formData) + .then((note) => { // Submission successful! render final note element this.updateNote(null, note, null); - }, - error: () => { + }) + .fail(() => { // Submission failed, revert back to original note $noteBodyText.html(cachedNoteBodyText); $editingNote.removeClass('being-posted fade-in'); @@ -1317,8 +1322,7 @@ const normalizeNewlines = function(str) { // Show Flash message about failure this.updateNoteError(); - } - }); + }); return $closeBtn.text($closeBtn.data('original-text')); }; -- GitLab From 853b4b82a88ff3f210db8cd02ad776d08faf7a44 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Fri, 28 Apr 2017 17:50:16 +0530 Subject: [PATCH 13/35] Add `ajaxPost` util method --- app/assets/javascripts/lib/utils/common_utils.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index b9e9850be7e816..c57beff68a0ddf 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -35,6 +35,14 @@ }); }; + w.gl.utils.ajaxPost = function(url, data) { + return $.ajax({ + type: 'POST', + url: url, + data: data, + }); + }; + w.gl.utils.extractLast = function(term) { return this.split(term).pop(); }; -- GitLab From f9a1e76dc3d1f76f4109f9579c5298e7843695fe Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Fri, 28 Apr 2017 17:50:34 +0530 Subject: [PATCH 14/35] Replace `submitComment` with `gl.utils.ajaxPost` --- app/assets/javascripts/notes.js | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index a0d2021d3e4b5b..2706ef998281ee 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -1138,17 +1138,6 @@ const normalizeNewlines = function(str) { }; }; - /** - * Perform network request to submit form contents and return jQuery Defferred. - */ - Notes.prototype.submitComment = function(formAction, formData) { - return $.ajax({ - type: 'POST', - url: formAction, - data: formData, - }); - }; - /** * Create placeholder note DOM element populated with comment body * that we will show while comment is being posted. @@ -1193,7 +1182,7 @@ const normalizeNewlines = function(str) { * 2) Identify comment type; a) Main thread b) Discussion thread c) Discussion resolve * 3) Build temporary placeholder element (using `createPlaceholderNote`) * 4) Show placeholder note on UI - * 5) Perform network request to submit the note using `submitComment` + * 5) Perform network request to submit the note using `gl.utils.ajaxPost` * a) If request is successfully completed * 1. Remove placeholder element * 2. Show submitted Note element @@ -1238,7 +1227,7 @@ const normalizeNewlines = function(str) { /* eslint-disable promise/catch-or-return */ // Make request to submit comment on server - this.submitComment(formAction, formData) + gl.utils.ajaxPost(formAction, formData) .then((note) => { // Submission successful! remove placeholder $notesContainer.find(`#${uniqueId}`).remove(); @@ -1280,7 +1269,7 @@ const normalizeNewlines = function(str) { * * 1) Get Form metadata * 2) Update note element with new content - * 3) Perform network request to submit the updated note using `submitComment` + * 3) Perform network request to submit the updated note using `gl.utils.ajaxPost` * a) If request is successfully completed * 1. Show submitted Note element * b) If request failed @@ -1309,7 +1298,7 @@ const normalizeNewlines = function(str) { /* eslint-disable promise/catch-or-return */ // Make request to update comment on server - this.submitComment(formAction, formData) + gl.utils.ajaxPost(formAction, formData) .then((note) => { // Submission successful! render final note element this.updateNote(null, note, null); -- GitLab From 49f7111164e7742eed51ddd8a4234bdecb4074aa Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Fri, 28 Apr 2017 17:51:58 +0530 Subject: [PATCH 15/35] Add tests for `ajaxPost` and `guid` --- .../lib/utils/common_utils_spec.js | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js index a00efa10119ce4..e003b336a0bcf2 100644 --- a/spec/javascripts/lib/utils/common_utils_spec.js +++ b/spec/javascripts/lib/utils/common_utils_spec.js @@ -362,5 +362,33 @@ require('~/lib/utils/common_utils'); gl.utils.setCiStatusFavicon(BUILD_URL); }); }); + + describe('gl.utils.ajaxPost', () => { + it('should perform `$.ajax` call and do `POST` request', () => { + const requestURL = '/some/random/api'; + const data = { keyname: 'value' }; + const ajaxSpy = spyOn($, 'ajax').and.callFake(() => {}); + + gl.utils.ajaxPost(requestURL, data); + expect(ajaxSpy.calls.allArgs()[0][0].type).toEqual('POST'); + }); + }); + + describe('gl.utils.guid', () => { + it('should generate a random Globally Unique Identifier string', () => { + const guid = gl.utils.guid(); + const guidParts = guid.split('-'); + const partLengths = [8, 4, 4, 4, 12]; + const partLengthMatcher = (part, expectedLength) => { + expect(part.length).toBe(expectedLength); + }; + + expect(guid).toBeDefined(); + expect(guidParts.length).toBe(5); // There are 5 string chunks split with `-` + guidParts.forEach((part, index) => { + partLengthMatcher(part, partLengths[index]); + }); + }); + }); }); })(); -- GitLab From 22a6b46f8da85841d17cd7b61ebee2e575d29c5c Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Fri, 28 Apr 2017 17:52:52 +0530 Subject: [PATCH 16/35] Update animation class name --- spec/javascripts/notes_spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index cdc5c4510ffcd2..72f06d1a52e6b0 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -238,8 +238,8 @@ import '~/notes'; $resultantNote = Notes.animateAppendNote(noteHTML, $notesList); }); - it('should have `fade-in` class', () => { - expect($resultantNote.hasClass('fade-in')).toEqual(true); + it('should have `fade-in-full` class', () => { + expect($resultantNote.hasClass('fade-in-full')).toEqual(true); }); it('should append note to the notes list', () => { -- GitLab From 265790f00ac6bcfe7728c7a6f97d9f1b2bde986a Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Mon, 1 May 2017 14:50:32 +0530 Subject: [PATCH 17/35] Form submission is now done via button click --- spec/javascripts/notes_spec.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index 72f06d1a52e6b0..53b341230689b0 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -26,7 +26,7 @@ import '~/notes'; describe('task lists', function() { beforeEach(function() { - $('form').on('submit', function(e) { + $('.js-comment-button').on('click', function(e) { e.preventDefault(); }); this.notes = new Notes(); @@ -60,9 +60,12 @@ import '~/notes'; reset: function() {} }); - $('form').on('submit', function(e) { + $('.js-comment-button').on('click', (e) => { + const $form = $(this); e.preventDefault(); - $('.js-main-target-form').trigger('ajax:success'); + this.notes.addNote($form); + this.notes.reenableTargetFormSubmitButton(e); + this.notes.resetMainTargetForm(e); }); }); -- GitLab From 467d63170a84e0bdfa12de63a09e013632b00dea Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Mon, 1 May 2017 20:36:29 +0530 Subject: [PATCH 18/35] Add tests for all new methods added to Notes class --- spec/javascripts/notes_spec.js | 139 +++++++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index 53b341230689b0..2acf8f9575c60d 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -272,5 +272,144 @@ import '~/notes'; expect($note.replaceWith).toHaveBeenCalledWith($updatedNote); }); }); + + describe('getFormData', () => { + it('should return form metadata object from form reference', () => { + this.notes = new Notes(); + + const $form = $('form'); + const sampleComment = 'foobar'; + $form.find('textarea.js-note-text').val(sampleComment); + const { formData, formContent, formAction } = this.notes.getFormData($form); + + expect(formData.indexOf(sampleComment) > -1).toBe(true); + expect(formContent).toEqual(sampleComment); + expect(formAction).toEqual($form.attr('action')); + }); + }); + + describe('createPlaceholderNote', () => { + it('should return constructed placeholder element based on form contents', () => { + this.notes = new Notes(); + window.gon.current_username = 'root'; + window.gon.current_user_fullname = 'Administrator'; + + const sampleComment = 'foobar'; + const uniqueId = 'b1234-a4567'; + const $tempNote = this.notes.createPlaceholderNote(sampleComment, uniqueId); + const $tempNoteHeader = $tempNote.find('.note-header'); + + expect($tempNote.prop('nodeName')).toEqual('LI'); + expect($tempNote.attr('id')).toEqual(uniqueId); + $tempNote.find('.timeline-icon > a, .note-header-info > a').each(function() { + expect($(this).attr('href')).toEqual(`/${window.gon.current_username}`); + }); + expect($tempNoteHeader.find('.hidden-xs').text().trim()).toEqual(window.gon.current_user_fullname); + expect($tempNoteHeader.find('.note-headline-light').text().trim()).toEqual(`@${window.gon.current_username}`); + expect($tempNote.find('.note-body .note-text').text().trim()).toEqual(sampleComment); + }); + }); + + describe('postComment & updateComment', () => { + const sampleComment = 'foo'; + const updatedComment = 'bar'; + const note = { + id: 1234, + html: `
  • +
    ${sampleComment}
    +
    • `, + note: sampleComment, + valid: true + }; + let $form; + let $notesContainer; + + beforeEach(() => { + this.notes = new Notes(); + window.gon.current_username = 'root'; + window.gon.current_user_fullname = 'Administrator'; + $form = $('form'); + $notesContainer = $('ul.main-notes-list'); + $form.find('textarea.js-note-text').val(sampleComment); + $('.js-comment-button').click(); + }); + + it('should show placeholder note while new comment is being posted', () => { + expect($notesContainer.find('.note.being-posted').length > 0).toEqual(true); + }); + + it('should remove placeholder note when new comment is done posting', () => { + spyOn($, 'ajax').and.callFake((options) => { + options.success(note); + expect($notesContainer.find('.note.being-posted').length).toEqual(0); + }); + }); + + it('should show actual note element when new comment is done posting', () => { + spyOn($, 'ajax').and.callFake((options) => { + options.success(note); + expect($notesContainer.find(`#${note.id}`).length > 0).toEqual(true); + }); + }); + + it('should show flash error message when new comment failed to be posted', () => { + spyOn($, 'ajax').and.callFake((options) => { + options.error(); + expect($notesContainer.parent().find('.flash-container .flash-text').is(':visible')).toEqual(true); + }); + }); + + it('should show updated comment as _actively being posted_ while comment being updated', () => { + spyOn($, 'ajax').and.callFake((options) => { + options.success(note); + const $noteEl = $notesContainer.find(`#note_${note.id}`); + $noteEl.find('.js-note-edit').click(); + $noteEl.find('textarea.js-note-text').val(updatedComment); + $noteEl.find('.js-comment-save-button').click(); + expect($noteEl.hasClass('.being-posted')).toEqual(true); + expect($noteEl.find('.note-text').text()).toEqual(updatedComment); + }); + }); + + it('should show updated comment when comment update is done posting', () => { + spyOn($, 'ajax').and.callFake((options) => { + options.success(note); + const $noteEl = $notesContainer.find(`#note_${note.id}`); + $noteEl.find('.js-note-edit').click(); + $noteEl.find('textarea.js-note-text').val(updatedComment); + $noteEl.find('.js-comment-save-button').click(); + + spyOn($, 'ajax').and.callFake((updateOptions) => { + const updatedNote = Object.assign({}, note); + updatedNote.note = updatedComment; + updatedNote.html = `
  • +
    ${updatedComment}
    +
    • `; + updateOptions.success(updatedNote); + const $updatedNoteEl = $notesContainer.find(`#note_${updatedNote.id}`); + expect($updatedNoteEl.hasClass('.being-posted')).toEqual(false); // Remove being-posted visuals + expect($updatedNoteEl.find('note-text').text().trim()).toEqual(updatedComment); // Verify if comment text updated + }); + }); + }); + + it('should show flash error message when comment failed to be updated', () => { + spyOn($, 'ajax').and.callFake((options) => { + options.success(note); + const $noteEl = $notesContainer.find(`#note_${note.id}`); + $noteEl.find('.js-note-edit').click(); + $noteEl.find('textarea.js-note-text').val(updatedComment); + $noteEl.find('.js-comment-save-button').click(); + + spyOn($, 'ajax').and.callFake((updateOptions) => { + updateOptions.error(); + const $updatedNoteEl = $notesContainer.find(`#note_${note.id}`); + expect($updatedNoteEl.hasClass('.being-posted')).toEqual(false); // Remove being-posted visuals + expect($updatedNoteEl.find('note-text').text().trim()).toEqual(sampleComment); // See if comment reverted back to original + expect($notesContainer.parent().find('.flash-container .flash-text').is(':visible')).toEqual(true); // Flash error message shown + }); + }); + }); + }); }); }).call(window); -- GitLab From 0e39023aa10e81dd554d752cc5e536ce91ce57a6 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Wed, 3 May 2017 23:18:13 +0530 Subject: [PATCH 19/35] Fix broken rspec and spinach tests --- features/steps/project/merge_requests.rb | 2 +- features/steps/shared/note.rb | 2 ++ spec/features/merge_requests/user_posts_notes_spec.rb | 1 + spec/support/features/discussion_comments_shared_example.rb | 1 + .../features/issuable_slash_commands_shared_examples.rb | 1 + spec/support/time_tracking_shared_examples.rb | 5 +++++ 6 files changed, 11 insertions(+), 1 deletion(-) diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index ce7674a9314602..e8dee8c28c4517 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -98,7 +98,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps end step 'I click link "Close"' do - first(:css, '.close-mr-link').click + first(:css, '.js-note-target-close').click end step 'I submit new merge request "Wiki Feature"' do diff --git a/features/steps/shared/note.rb b/features/steps/shared/note.rb index 7885cc7ab77290..6895163a876007 100644 --- a/features/steps/shared/note.rb +++ b/features/steps/shared/note.rb @@ -23,6 +23,7 @@ module SharedNote page.within(".js-main-target-form") do fill_in "note[note]", with: "XML attached" click_button "Comment" + wait_for_ajax end end @@ -36,6 +37,7 @@ module SharedNote step 'I submit the comment' do page.within(".js-main-target-form") do click_button "Comment" + wait_for_ajax end end diff --git a/spec/features/merge_requests/user_posts_notes_spec.rb b/spec/features/merge_requests/user_posts_notes_spec.rb index c7cc4d6bc724ea..7fc0e2ce6eca49 100644 --- a/spec/features/merge_requests/user_posts_notes_spec.rb +++ b/spec/features/merge_requests/user_posts_notes_spec.rb @@ -98,6 +98,7 @@ find('.btn-save').click end + wait_for_ajax find('.note').hover find('.js-note-edit').click diff --git a/spec/support/features/discussion_comments_shared_example.rb b/spec/support/features/discussion_comments_shared_example.rb index bb4542b1683522..8cfbc5f761cdd3 100644 --- a/spec/support/features/discussion_comments_shared_example.rb +++ b/spec/support/features/discussion_comments_shared_example.rb @@ -27,6 +27,7 @@ find(close_selector).click + wait_for_ajax find(comments_selector, match: :first) find("#{comments_selector}.system-note") entries = all(comments_selector) diff --git a/spec/support/features/issuable_slash_commands_shared_examples.rb b/spec/support/features/issuable_slash_commands_shared_examples.rb index 1d26c7baace360..595bb92f5c0911 100644 --- a/spec/support/features/issuable_slash_commands_shared_examples.rb +++ b/spec/support/features/issuable_slash_commands_shared_examples.rb @@ -58,6 +58,7 @@ expect(page).not_to have_content '/label ~bug' expect(page).not_to have_content '/milestone %"ASAP"' + wait_for_ajax issuable.reload note = issuable.notes.user.first diff --git a/spec/support/time_tracking_shared_examples.rb b/spec/support/time_tracking_shared_examples.rb index 01bc80f957e246..e94e17da7e5880 100644 --- a/spec/support/time_tracking_shared_examples.rb +++ b/spec/support/time_tracking_shared_examples.rb @@ -8,6 +8,7 @@ it 'updates the sidebar component when estimate is added' do submit_time('/estimate 3w 1d 1h') + wait_for_ajax page.within '.time-tracking-estimate-only-pane' do expect(page).to have_content '3w 1d 1h' end @@ -16,6 +17,7 @@ it 'updates the sidebar component when spent is added' do submit_time('/spend 3w 1d 1h') + wait_for_ajax page.within '.time-tracking-spend-only-pane' do expect(page).to have_content '3w 1d 1h' end @@ -25,6 +27,7 @@ submit_time('/estimate 3w 1d 1h') submit_time('/spend 3w 1d 1h') + wait_for_ajax page.within '.time-tracking-comparison-pane' do expect(page).to have_content '3w 1d 1h' end @@ -34,6 +37,7 @@ submit_time('/estimate 3w 1d 1h') submit_time('/remove_estimate') + wait_for_ajax page.within '#issuable-time-tracker' do expect(page).to have_content 'No estimate or time spent' end @@ -43,6 +47,7 @@ submit_time('/spend 3w 1d 1h') submit_time('/remove_time_spent') + wait_for_ajax page.within '#issuable-time-tracker' do expect(page).to have_content 'No estimate or time spent' end -- GitLab From 328eeed029256c99de9ea4771c4aeb017b012ec9 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Wed, 3 May 2017 23:18:27 +0530 Subject: [PATCH 20/35] Add methods to find and strip slash commands, update comment posting logic --- app/assets/javascripts/notes.js | 37 +++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 2706ef998281ee..893bf175ac5806 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -26,6 +26,7 @@ const normalizeNewlines = function(str) { this.Notes = (function() { const MAX_VISIBLE_COMMIT_LIST_COUNT = 3; + const REGEX_SLASH_COMMANDS = /\/\w+/g; Notes.interval = null; @@ -1138,20 +1139,35 @@ const normalizeNewlines = function(str) { }; }; + /** + * Identify if comment has any slash commands + */ + Notes.prototype.hasSlashCommands = function(formContent) { + return REGEX_SLASH_COMMANDS.test(formContent); + }; + + /** + * Remove slash commands and leave comment with pure message + */ + Notes.prototype.stripSlashCommands = function(formContent) { + return formContent.replace(REGEX_SLASH_COMMANDS, '').trim(); + }; + /** * Create placeholder note DOM element populated with comment body * that we will show while comment is being posted. * Once comment is _actually_ posted on server, we will have final element * in response that we will show in place of this temporary element. */ - Notes.prototype.createPlaceholderNote = function(formContent, uniqueId) { + Notes.prototype.createPlaceholderNote = function(formContent, uniqueId, isDiscussionNote) { + const discussionCls = isDiscussionNote ? 'discussion' : ''; const $tempNote = $( `
  • -
    +
    @@ -1200,12 +1216,14 @@ const normalizeNewlines = function(str) { const $submitBtn = $(e.target); const $form = $submitBtn.parents('form'); const $closeBtn = $form.find('.js-note-target-close'); + const commentType = $submitBtn.parent().find('li.droplab-item-selected').attr('id'); 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 } = this.getFormData($form); const uniqueId = gl.utils.guid(); let $notesContainer; + let tempFormContent; // Get reference to notes container based on type of comment if (isDiscussionForm) { @@ -1221,8 +1239,17 @@ const normalizeNewlines = function(str) { $form.find('.js-comment-submit-button').disable(); } - // Show placeholder note and clear the form textarea - $notesContainer.append(this.createPlaceholderNote(formContent, uniqueId)); + tempFormContent = formContent; + if (this.hasSlashCommands(formContent)) { + tempFormContent = this.stripSlashCommands(formContent); + } + + if (tempFormContent) { + // Show placeholder note + $notesContainer.append(this.createPlaceholderNote(tempFormContent, uniqueId, commentType === 'discussion')); + } + + // Clear the form textarea $form.find('.js-note-text').val(''); /* eslint-disable promise/catch-or-return */ @@ -1254,6 +1281,8 @@ const normalizeNewlines = function(str) { this.reenableTargetFormSubmitButton(e); this.resetMainTargetForm(e); } + + $form.trigger('ajax:success', [note]); }).fail(() => { // Submission failed, remove placeholder note and show Flash error message $notesContainer.find(`#${uniqueId}`).remove(); -- GitLab From 4840ad256e669cb4d83ae74cd5e8ff8eaa358842 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Wed, 3 May 2017 23:18:51 +0530 Subject: [PATCH 21/35] Update tests for changes in Notes class --- spec/javascripts/notes_spec.js | 59 +++++++++++++++++++++++++++++++--- 1 file changed, 55 insertions(+), 4 deletions(-) diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index 2acf8f9575c60d..c7a44d74544c03 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -288,15 +288,50 @@ import '~/notes'; }); }); + describe('hasSlashCommands', () => { + beforeEach(() => { + this.notes = new Notes(); + }); + + it('should return true when comment has slash commands', () => { + const sampleComment = '/wip /milestone %1.0 /merge /unassign Merging this'; + const hasSlashCommands = this.notes.hasSlashCommands(sampleComment); + + expect(hasSlashCommands).toBeTruthy(); + }); + + it('should return false when comment does NOT have any slash commands', () => { + const sampleComment = 'Looking good, Awesome!'; + const hasSlashCommands = this.notes.hasSlashCommands(sampleComment); + + expect(hasSlashCommands).toBeFalsy(); + }); + }); + + describe('stripSlashCommands', () => { + const REGEX_SLASH_COMMANDS = /\/\w+/g; + + it('should strip slash commands from the comment', () => { + this.notes = new Notes(); + const sampleComment = '/wip /milestone %1.0 /merge /unassign Merging this'; + const stripedComment = this.notes.stripSlashCommands(sampleComment); + + expect(REGEX_SLASH_COMMANDS.test(stripedComment)).toBeFalsy(); + }); + }); + describe('createPlaceholderNote', () => { - it('should return constructed placeholder element based on form contents', () => { + const sampleComment = 'foobar'; + const uniqueId = 'b1234-a4567'; + + beforeEach(() => { this.notes = new Notes(); window.gon.current_username = 'root'; window.gon.current_user_fullname = 'Administrator'; + }); - const sampleComment = 'foobar'; - const uniqueId = 'b1234-a4567'; - const $tempNote = this.notes.createPlaceholderNote(sampleComment, uniqueId); + it('should return constructed placeholder element for regular note based on form contents', () => { + const $tempNote = this.notes.createPlaceholderNote(sampleComment, uniqueId, false); const $tempNoteHeader = $tempNote.find('.note-header'); expect($tempNote.prop('nodeName')).toEqual('LI'); @@ -304,10 +339,18 @@ import '~/notes'; $tempNote.find('.timeline-icon > a, .note-header-info > a').each(function() { expect($(this).attr('href')).toEqual(`/${window.gon.current_username}`); }); + expect($tempNote.find('.timeline-content').hasClass('discussion')).toBeFalsy(); expect($tempNoteHeader.find('.hidden-xs').text().trim()).toEqual(window.gon.current_user_fullname); expect($tempNoteHeader.find('.note-headline-light').text().trim()).toEqual(`@${window.gon.current_username}`); expect($tempNote.find('.note-body .note-text').text().trim()).toEqual(sampleComment); }); + + it('should return constructed placeholder element for discussion note based on form contents', () => { + const $tempNote = this.notes.createPlaceholderNote(sampleComment, uniqueId, true); + + expect($tempNote.prop('nodeName')).toEqual('LI'); + expect($tempNote.find('.timeline-content').hasClass('discussion')).toBeTruthy(); + }); }); describe('postComment & updateComment', () => { @@ -352,6 +395,14 @@ import '~/notes'; }); }); + it('should trigger ajax:success event on Form when new comment is done posting', () => { + spyOn($, 'ajax').and.callFake((options) => { + options.success(note); + spyOn($form, 'trigger'); + expect($form.trigger).toHaveBeenCalledWith('ajax:success', [note]); + }); + }); + it('should show flash error message when new comment failed to be posted', () => { spyOn($, 'ajax').and.callFake((options) => { options.error(); -- GitLab From 90a630b37196a5b5bbb3d84c55d2493106451659 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 4 May 2017 02:10:36 +0530 Subject: [PATCH 22/35] Add `wait_for_ajax` for comment post actions --- features/steps/project/merge_requests.rb | 9 ++++++++- features/steps/shared/note.rb | 6 ++++-- .../merge_requests/user_uses_slash_commands_spec.rb | 1 + .../features/discussion_comments_shared_example.rb | 1 + 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index e8dee8c28c4517..08d5d6972ffa97 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -98,7 +98,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps end step 'I click link "Close"' do - first(:css, '.js-note-target-close').click + first(:css, '.close-mr-link').click end step 'I submit new merge request "Wiki Feature"' do @@ -458,6 +458,8 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps click_button "Comment" end + wait_for_ajax + page.within ".files>div:nth-child(2) .note-body > .note-text" do expect(page).to have_content "Line is correct" end @@ -470,6 +472,8 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps fill_in "note_note", with: "Line is wrong on here" click_button "Comment" end + + wait_for_ajax end step 'I should still see a comment like "Line is correct" in the second file' do @@ -715,6 +719,9 @@ def leave_comment(message) fill_in "note_note", with: message click_button "Comment" end + + wait_for_ajax + page.within(".notes_holder", visible: true) do expect(page).to have_content message end diff --git a/features/steps/shared/note.rb b/features/steps/shared/note.rb index 6895163a876007..7d260025052cf6 100644 --- a/features/steps/shared/note.rb +++ b/features/steps/shared/note.rb @@ -23,8 +23,9 @@ module SharedNote page.within(".js-main-target-form") do fill_in "note[note]", with: "XML attached" click_button "Comment" - wait_for_ajax end + + wait_for_ajax end step 'I preview a comment text like "Bug fixed :smile:"' do @@ -37,8 +38,9 @@ module SharedNote step 'I submit the comment' do page.within(".js-main-target-form") do click_button "Comment" - wait_for_ajax end + + wait_for_ajax end step 'I write a comment like ":+1: Nice"' do diff --git a/spec/features/merge_requests/user_uses_slash_commands_spec.rb b/spec/features/merge_requests/user_uses_slash_commands_spec.rb index 3986439061ec87..08cd39fb8589c7 100644 --- a/spec/features/merge_requests/user_uses_slash_commands_spec.rb +++ b/spec/features/merge_requests/user_uses_slash_commands_spec.rb @@ -160,6 +160,7 @@ it 'changes target branch from a note' do write_note("message start \n/target_branch merge-test\n message end.") + wait_for_ajax expect(page).not_to have_content('/target_branch') expect(page).to have_content('message start') expect(page).to have_content('message end.') diff --git a/spec/support/features/discussion_comments_shared_example.rb b/spec/support/features/discussion_comments_shared_example.rb index 8cfbc5f761cdd3..95faea67bcf9c8 100644 --- a/spec/support/features/discussion_comments_shared_example.rb +++ b/spec/support/features/discussion_comments_shared_example.rb @@ -124,6 +124,7 @@ it "clicking 'Start discussion & close #{resource_name}' will post a discussion and close the #{resource_name}" do find(close_selector).click + wait_for_ajax find(comments_selector, match: :first) find("#{comments_selector}.system-note") entries = all(comments_selector) -- GitLab From f99ce08ec9f097ba0f5c5a4e22b7cfd82f2fdc81 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 4 May 2017 11:24:02 +0530 Subject: [PATCH 23/35] Fix form handling on comment post --- app/assets/javascripts/notes.js | 37 ++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 893bf175ac5806..fe14bc2225e44a 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -569,7 +569,7 @@ const normalizeNewlines = function(str) { Adds new note to list. */ - Notes.prototype.addDiscussionNote = function($form, note) { + Notes.prototype.addDiscussionNote = function($form, note, isNewDiffComment) { if ($form.attr('data-resolve-all') != null) { var projectPath = $form.data('project-path'); var discussionId = $form.data('discussion-id'); @@ -582,7 +582,9 @@ const normalizeNewlines = function(str) { this.renderNote(note, $form); // cleanup after successfully creating a diff/discussion note - this.removeDiscussionNoteForm($form); + if (isNewDiffComment) { + this.removeDiscussionNoteForm($form); + } }; /* @@ -1214,7 +1216,7 @@ const normalizeNewlines = function(str) { // Get Form metadata const $submitBtn = $(e.target); - const $form = $submitBtn.parents('form'); + let $form = $submitBtn.parents('form'); const $closeBtn = $form.find('.js-note-target-close'); const commentType = $submitBtn.parent().find('li.droplab-item-selected').attr('id'); const isMainForm = $form.hasClass('js-main-target-form'); @@ -1250,7 +1252,13 @@ const normalizeNewlines = function(str) { } // Clear the form textarea - $form.find('.js-note-text').val(''); + if ($notesContainer.length) { + if (isMainForm) { + this.resetMainTargetForm(e); + } else if (isDiscussionForm) { + this.removeDiscussionNoteForm($form); + } + } /* eslint-disable promise/catch-or-return */ // Make request to submit comment on server @@ -1272,20 +1280,33 @@ const normalizeNewlines = function(str) { .attr('data-project-path', $submitBtn.data('project-path')); } - // Show final note element on UI and append flash-container after that - this.addDiscussionNote($form, note); - $notesContainer.append(''); + // Show final note element on UI + this.addDiscussionNote($form, note, $notesContainer.length === 0); + + // append flash-container to the Notes list + if ($notesContainer.length) { + $notesContainer.append(''); + } } else if (isMainForm) { // Check if this was main thread comment // Show final note element on UI and perform form and action buttons cleanup this.addNote($form, note); this.reenableTargetFormSubmitButton(e); - this.resetMainTargetForm(e); } $form.trigger('ajax:success', [note]); }).fail(() => { // Submission failed, remove placeholder note and show Flash error message $notesContainer.find(`#${uniqueId}`).remove(); + + // Show form again on UI on failure + if (isDiscussionForm && $notesContainer.length) { + const replyButton = $notesContainer.parent().find('.js-discussion-reply-button'); + $.proxy(this.replyToDiscussionNote, replyButton[0], { target: replyButton[0] }).call(); + $form = $notesContainer.parent().find('form'); + } + + $form.find('.js-note-text').val(formContent); + this.reenableTargetFormSubmitButton(e); this.addNoteError($form); }); -- GitLab From 759a207836d2e528c11ed7cd1ed10d86d13d2467 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 4 May 2017 11:24:24 +0530 Subject: [PATCH 24/35] Update tests with form reset cases! --- spec/javascripts/notes_spec.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index c7a44d74544c03..f5652eae408f39 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -395,6 +395,13 @@ import '~/notes'; }); }); + it('should reset Form when new comment is done posting', () => { + spyOn($, 'ajax').and.callFake((options) => { + options.success(note); + expect($form.find('textarea.js-note-text')).toEqual(''); + }); + }); + it('should trigger ajax:success event on Form when new comment is done posting', () => { spyOn($, 'ajax').and.callFake((options) => { options.success(note); @@ -410,6 +417,13 @@ import '~/notes'; }); }); + it('should refill form textarea with original comment content when new comment failed to be posted', () => { + spyOn($, 'ajax').and.callFake((options) => { + options.error(); + expect($form.find('textarea.js-note-text')).toEqual(sampleComment); + }); + }); + it('should show updated comment as _actively being posted_ while comment being updated', () => { spyOn($, 'ajax').and.callFake((options) => { options.success(note); -- GitLab From 0d1ca38572a56776b017cbf933ddf3dd0b2c920b Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 4 May 2017 11:33:09 +0530 Subject: [PATCH 25/35] Add changelog entry --- changelogs/unreleased/27614-instant-comments.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/unreleased/27614-instant-comments.yml diff --git a/changelogs/unreleased/27614-instant-comments.yml b/changelogs/unreleased/27614-instant-comments.yml new file mode 100644 index 00000000000000..7b2592f46ede2e --- /dev/null +++ b/changelogs/unreleased/27614-instant-comments.yml @@ -0,0 +1,4 @@ +--- +title: Add support for instantly updating comments +merge_request: 10760 +author: -- GitLab From 9191bbd9596579b45b7ece5fdf1dd14d56584544 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 4 May 2017 15:58:14 +0530 Subject: [PATCH 26/35] Remove `guid`, replace with `_.uniqueId()` --- .../javascripts/lib/utils/common_utils.js | 8 -------- spec/javascripts/lib/utils/common_utils_spec.js | 17 ----------------- 2 files changed, 25 deletions(-) diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index c57beff68a0ddf..2f682fbd2fbf85 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -379,14 +379,6 @@ }); }; - /** - * Generates random unique GUID, eg; f4ccf724-4cc3-93c5-0912-e7b8d440a832 - */ - w.gl.utils.guid = () => { - const s4 = () => Math.floor((1 + Math.random()) * 0x10000).toString(16).substring(1); - return `${s4()}${s4()}-${s4()}-${s4()}-${s4()}-${s4()}${s4()}${s4()}`; - }; - w.gl.utils.setFavicon = (faviconPath) => { if (faviconEl && faviconPath) { faviconEl.setAttribute('href', faviconPath); diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js index e003b336a0bcf2..5eb147ed888063 100644 --- a/spec/javascripts/lib/utils/common_utils_spec.js +++ b/spec/javascripts/lib/utils/common_utils_spec.js @@ -373,22 +373,5 @@ require('~/lib/utils/common_utils'); expect(ajaxSpy.calls.allArgs()[0][0].type).toEqual('POST'); }); }); - - describe('gl.utils.guid', () => { - it('should generate a random Globally Unique Identifier string', () => { - const guid = gl.utils.guid(); - const guidParts = guid.split('-'); - const partLengths = [8, 4, 4, 4, 12]; - const partLengthMatcher = (part, expectedLength) => { - expect(part.length).toBe(expectedLength); - }; - - expect(guid).toBeDefined(); - expect(guidParts.length).toBe(5); // There are 5 string chunks split with `-` - guidParts.forEach((part, index) => { - partLengthMatcher(part, partLengths[index]); - }); - }); - }); }); })(); -- GitLab From 3ce5cd7bd6860ac2e883837d2679509661987a9a Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 4 May 2017 15:58:40 +0530 Subject: [PATCH 27/35] Use `_uniqueId()` instead of `guid` --- app/assets/javascripts/notes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index fe14bc2225e44a..94b8b982f185f5 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -1223,7 +1223,7 @@ const normalizeNewlines = function(str) { const isDiscussionForm = $form.hasClass('js-discussion-note-form'); const isDiscussionResolve = $submitBtn.hasClass('js-comment-resolve-button'); const { formData, formContent, formAction } = this.getFormData($form); - const uniqueId = gl.utils.guid(); + const uniqueId = _.uniqueId('tempNote_'); let $notesContainer; let tempFormContent; -- GitLab From c1f16653144344371a01b4d7dc80d807eecc7022 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 4 May 2017 20:26:49 +0530 Subject: [PATCH 28/35] Fixes as per the review feedback --- app/assets/javascripts/notes.js | 48 ++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 94b8b982f185f5..5514e11778bbc4 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -549,7 +549,7 @@ const normalizeNewlines = function(str) { return this.renderNote(note); }; - Notes.prototype.addNoteError = function($form) { + Notes.prototype.addNoteError = ($form) => { let formParentTimeline; if ($form.hasClass('js-main-target-form')) { formParentTimeline = $form.parents('.timeline'); @@ -559,9 +559,7 @@ const normalizeNewlines = function(str) { return new Flash('Your comment could not be submitted! Please check your network connection and try again.', 'alert', formParentTimeline); }; - Notes.prototype.updateNoteError = function($parentTimeline) { - return new Flash('Your comment could not be updated! Please check your network connection and try again.'); - }; + Notes.prototype.updateNoteError = $parentTimeline => new Flash('Your comment could not be updated! Please check your network connection and try again.'); /* Called in response to the new note form being submitted @@ -594,18 +592,18 @@ const normalizeNewlines = function(str) { */ Notes.prototype.updateNote = function(_xhr, noteEntity, _status) { - var $html, $note_li; + var $noteEntityEl, $note_li; // Convert returned HTML to a jQuery object so we can modify it further - $html = $(noteEntity.html); - $html.addClass('fade-in-full'); + $noteEntityEl = $(noteEntity.html); + $noteEntityEl.addClass('fade-in-full'); this.revertNoteEditForm(); - gl.utils.localTimeAgo($('.js-timeago', $html)); - $html.renderGFM(); - $html.find('.js-task-list-container').taskList('enable'); + gl.utils.localTimeAgo($('.js-timeago', $noteEntityEl)); + $noteEntityEl.renderGFM(); + $noteEntityEl.find('.js-task-list-container').taskList('enable'); // Find the note's `li` element by ID and replace it with the updated HTML $note_li = $('.note-row-' + noteEntity.id); - $note_li.replaceWith($html); + $note_li.replaceWith($noteEntityEl); if (typeof gl.diffNotesCompileComponents !== 'undefined') { gl.diffNotesCompileComponents(); @@ -1161,20 +1159,20 @@ const normalizeNewlines = function(str) { * Once comment is _actually_ posted on server, we will have final element * in response that we will show in place of this temporary element. */ - Notes.prototype.createPlaceholderNote = function(formContent, uniqueId, isDiscussionNote) { - const discussionCls = isDiscussionNote ? 'discussion' : ''; + Notes.prototype.createPlaceholderNote = function({ formContent, uniqueId, isDiscussionNote, currentUsername, currentUserFullname }) { + const discussionClass = isDiscussionNote ? 'discussion' : ''; const $tempNote = $( `
  • - +
    -
    +
    - - - @${gon.current_username} + + + @${currentUsername} @@ -1182,7 +1180,9 @@ const normalizeNewlines = function(str) {
    -
    ${formContent}
    +
    +

    ${formContent}

    +
    @@ -1218,7 +1218,7 @@ const normalizeNewlines = function(str) { const $submitBtn = $(e.target); let $form = $submitBtn.parents('form'); const $closeBtn = $form.find('.js-note-target-close'); - const commentType = $submitBtn.parent().find('li.droplab-item-selected').attr('id'); + const isDiscussionNote = $submitBtn.parent().find('li.droplab-item-selected').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'); @@ -1248,7 +1248,13 @@ const normalizeNewlines = function(str) { if (tempFormContent) { // Show placeholder note - $notesContainer.append(this.createPlaceholderNote(tempFormContent, uniqueId, commentType === 'discussion')); + $notesContainer.append(this.createPlaceholderNote({ + formContent: tempFormContent, + uniqueId, + isDiscussionNote, + currentUsername: gon.current_username, + currentUserFullname: gon.current_user_fullname, + })); } // Clear the form textarea -- GitLab From f3f95d11e9a24e43c0b488bc7e0ef38079a31e15 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 4 May 2017 20:28:34 +0530 Subject: [PATCH 29/35] Update tests --- spec/javascripts/notes_spec.js | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index f5652eae408f39..7bffa90ab145e4 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -323,30 +323,42 @@ import '~/notes'; describe('createPlaceholderNote', () => { const sampleComment = 'foobar'; const uniqueId = 'b1234-a4567'; + const currentUsername = 'root'; + const currentUserFullname = 'Administrator'; beforeEach(() => { this.notes = new Notes(); - window.gon.current_username = 'root'; - window.gon.current_user_fullname = 'Administrator'; }); it('should return constructed placeholder element for regular note based on form contents', () => { - const $tempNote = this.notes.createPlaceholderNote(sampleComment, uniqueId, false); + const $tempNote = this.notes.createPlaceholderNote({ + formContent: sampleComment, + uniqueId, + isDiscussionNote: false, + currentUsername, + currentUserFullname + }); const $tempNoteHeader = $tempNote.find('.note-header'); expect($tempNote.prop('nodeName')).toEqual('LI'); expect($tempNote.attr('id')).toEqual(uniqueId); $tempNote.find('.timeline-icon > a, .note-header-info > a').each(function() { - expect($(this).attr('href')).toEqual(`/${window.gon.current_username}`); + expect($(this).attr('href')).toEqual(`/${currentUsername}`); }); expect($tempNote.find('.timeline-content').hasClass('discussion')).toBeFalsy(); - expect($tempNoteHeader.find('.hidden-xs').text().trim()).toEqual(window.gon.current_user_fullname); - expect($tempNoteHeader.find('.note-headline-light').text().trim()).toEqual(`@${window.gon.current_username}`); + expect($tempNoteHeader.find('.hidden-xs').text().trim()).toEqual(currentUserFullname); + expect($tempNoteHeader.find('.note-headline-light').text().trim()).toEqual(`@${currentUsername}`); expect($tempNote.find('.note-body .note-text').text().trim()).toEqual(sampleComment); }); it('should return constructed placeholder element for discussion note based on form contents', () => { - const $tempNote = this.notes.createPlaceholderNote(sampleComment, uniqueId, true); + const $tempNote = this.notes.createPlaceholderNote({ + formContent: sampleComment, + uniqueId, + isDiscussionNote: true, + currentUsername, + currentUserFullname + }); expect($tempNote.prop('nodeName')).toEqual('LI'); expect($tempNote.find('.timeline-content').hasClass('discussion')).toBeTruthy(); -- GitLab From 671180edf58545e2182b9df2bbc1b31d01e29585 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 4 May 2017 23:29:19 +0530 Subject: [PATCH 30/35] Remove unnecessary `wait_for_ajax` --- spec/support/features/discussion_comments_shared_example.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/support/features/discussion_comments_shared_example.rb b/spec/support/features/discussion_comments_shared_example.rb index 95faea67bcf9c8..bb4542b1683522 100644 --- a/spec/support/features/discussion_comments_shared_example.rb +++ b/spec/support/features/discussion_comments_shared_example.rb @@ -27,7 +27,6 @@ find(close_selector).click - wait_for_ajax find(comments_selector, match: :first) find("#{comments_selector}.system-note") entries = all(comments_selector) @@ -124,7 +123,6 @@ it "clicking 'Start discussion & close #{resource_name}' will post a discussion and close the #{resource_name}" do find(close_selector).click - wait_for_ajax find(comments_selector, match: :first) find("#{comments_selector}.system-note") entries = all(comments_selector) -- GitLab From 9bf7e2853ba23cbf254cb76d42ff30949125175d Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 4 May 2017 23:29:38 +0530 Subject: [PATCH 31/35] Disable interaction to active note --- app/assets/stylesheets/pages/notes.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 453560c2986f78..cfea52c6e57425 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -58,6 +58,7 @@ ul.notes { border-bottom: 1px solid $white-normal; &.being-posted { + pointer-events: none; opacity: 0.5; .dummy-avatar { -- GitLab From 73de6d339b898ba302e7054fec130f078c5c68b0 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Thu, 4 May 2017 23:29:53 +0530 Subject: [PATCH 32/35] Update broken class name, add `ajax:*` listeners --- app/assets/javascripts/notes.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 5514e11778bbc4..93b2254b95c01c 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -113,6 +113,11 @@ const normalizeNewlines = function(str) { $(document).on("visibilitychange", this.visibilityChange); // when issue status changes, we need to refresh data $(document).on("issuable:change", this.refresh); + // ajax:events that happen on Form when actions like Reopen, Close are performed on Issues and MRs. + $(document).on("ajax:success", ".js-main-target-form", this.addNote); + $(document).on("ajax:success", ".js-discussion-note-form", this.addDiscussionNote); + $(document).on("ajax:success", ".js-main-target-form", this.resetMainTargetForm); + $(document).on("ajax:complete", ".js-main-target-form", this.reenableTargetFormSubmitButton); // when a key is clicked on the notes return $(document).on("keydown", ".js-note-text", this.keydownNoteText); }; @@ -132,6 +137,9 @@ const normalizeNewlines = function(str) { $(document).off("keydown", ".js-note-text"); $(document).off('click', '.js-comment-resolve-button'); $(document).off("click", '.system-note-commit-list-toggler'); + $(document).off("ajax:success", ".js-main-target-form"); + $(document).off("ajax:success", ".js-discussion-note-form"); + $(document).off("ajax:complete", ".js-main-target-form"); }; Notes.initCommentTypeToggle = function (form) { @@ -1339,7 +1347,7 @@ const normalizeNewlines = function(str) { const $submitBtn = $(e.target); const $form = $submitBtn.parents('form'); const $closeBtn = $form.find('.js-note-target-close'); - const $editingNote = $form.parents('.note.is-editting'); + const $editingNote = $form.parents('.note.is-editing'); const $noteBody = $editingNote.find('.js-task-list-container'); const $noteBodyText = $noteBody.find('.note-text'); const { formData, formContent, formAction } = this.getFormData($form); @@ -1349,7 +1357,7 @@ const normalizeNewlines = function(str) { // Show updated comment content temporarily $noteBodyText.html(formContent); - $editingNote.removeClass('is-editting').addClass('being-posted fade-in-half'); + $editingNote.removeClass('is-editing').addClass('being-posted fade-in-half'); $editingNote.find('.note-headline-meta a').html(''); /* eslint-disable promise/catch-or-return */ -- GitLab From 45bfdd7af9433283e3c5c137f70a37aae4e48863 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Fri, 5 May 2017 21:25:04 +0530 Subject: [PATCH 33/35] Fix slash commands handling on note creation --- app/assets/javascripts/notes.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 93b2254b95c01c..72709f68070d9e 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -272,12 +272,8 @@ const normalizeNewlines = function(str) { return this.initRefresh(); }; - Notes.prototype.handleCreateChanges = function(noteEntity) { + Notes.prototype.handleSlashCommands = function(noteEntity) { var votesBlock; - if (typeof noteEntity === 'undefined') { - return; - } - if (noteEntity.commands_changes) { if ('merge' in noteEntity.commands_changes) { $.get(mrRefreshWidgetUrl); @@ -553,7 +549,6 @@ const normalizeNewlines = function(str) { */ Notes.prototype.addNote = function($form, note) { - this.handleCreateChanges(note); return this.renderNote(note); }; @@ -1307,6 +1302,10 @@ const normalizeNewlines = function(str) { this.reenableTargetFormSubmitButton(e); } + if (note.commands_changes) { + this.handleSlashCommands(note); + } + $form.trigger('ajax:success', [note]); }).fail(() => { // Submission failed, remove placeholder note and show Flash error message -- GitLab From 8ce0197b9488ee1f3e439b7cbbc5818e1873dbac Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Sat, 6 May 2017 15:24:26 +0530 Subject: [PATCH 34/35] Remove redundant class on note update --- app/assets/javascripts/notes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 72709f68070d9e..55391ebc089a2a 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -1356,7 +1356,7 @@ const normalizeNewlines = function(str) { // Show updated comment content temporarily $noteBodyText.html(formContent); - $editingNote.removeClass('is-editing').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(''); /* eslint-disable promise/catch-or-return */ -- GitLab From 306c0303ae4fc4b4ca33ea4293d5eaa71d866ad1 Mon Sep 17 00:00:00 2001 From: kushalpandya Date: Sat, 6 May 2017 15:31:17 +0530 Subject: [PATCH 35/35] Fix invalid Ajax mocks --- spec/javascripts/notes_spec.js | 219 ++++++++++++++------------------- 1 file changed, 89 insertions(+), 130 deletions(-) diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index 7bffa90ab145e4..1026393e25e194 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -29,7 +29,7 @@ import '~/notes'; $('.js-comment-button').on('click', function(e) { e.preventDefault(); }); - this.notes = new Notes(); + this.notes = new Notes('', []); }); it('modifies the Markdown field', function() { @@ -51,7 +51,7 @@ import '~/notes'; var textarea = '.js-note-text'; beforeEach(function() { - this.notes = new Notes(); + this.notes = new Notes('', []); this.autoSizeSpy = spyOnEvent($(textarea), 'autosize:update'); spyOn(this.notes, 'renderNote').and.stub(); @@ -273,9 +273,92 @@ import '~/notes'; }); }); + describe('postComment & updateComment', () => { + const sampleComment = 'foo'; + const updatedComment = 'bar'; + const note = { + id: 1234, + html: `
  • +
    ${sampleComment}
    +
    • `, + note: sampleComment, + valid: true + }; + let $form; + let $notesContainer; + + beforeEach(() => { + this.notes = new Notes('', []); + window.gon.current_username = 'root'; + window.gon.current_user_fullname = 'Administrator'; + $form = $('form.js-main-target-form'); + $notesContainer = $('ul.main-notes-list'); + $form.find('textarea.js-note-text').val(sampleComment); + }); + + it('should show placeholder note while new comment is being posted', () => { + $('.js-comment-button').click(); + expect($notesContainer.find('.note.being-posted').length > 0).toEqual(true); + }); + + it('should remove placeholder note when new comment is done posting', () => { + const deferred = $.Deferred(); + spyOn($, 'ajax').and.returnValue(deferred.promise()); + $('.js-comment-button').click(); + + deferred.resolve(note); + expect($notesContainer.find('.note.being-posted').length).toEqual(0); + }); + + it('should show actual note element when new comment is done posting', () => { + const deferred = $.Deferred(); + spyOn($, 'ajax').and.returnValue(deferred.promise()); + $('.js-comment-button').click(); + + deferred.resolve(note); + expect($notesContainer.find(`#note_${note.id}`).length > 0).toEqual(true); + }); + + it('should reset Form when new comment is done posting', () => { + const deferred = $.Deferred(); + spyOn($, 'ajax').and.returnValue(deferred.promise()); + $('.js-comment-button').click(); + + deferred.resolve(note); + expect($form.find('textarea.js-note-text').val()).toEqual(''); + }); + + it('should show flash error message when new comment failed to be posted', () => { + const deferred = $.Deferred(); + spyOn($, 'ajax').and.returnValue(deferred.promise()); + $('.js-comment-button').click(); + + deferred.reject(); + expect($notesContainer.parent().find('.flash-container .flash-text').is(':visible')).toEqual(true); + }); + + it('should show flash error message when comment failed to be updated', () => { + const deferred = $.Deferred(); + spyOn($, 'ajax').and.returnValue(deferred.promise()); + $('.js-comment-button').click(); + + deferred.resolve(note); + const $noteEl = $notesContainer.find(`#note_${note.id}`); + $noteEl.find('.js-note-edit').click(); + $noteEl.find('textarea.js-note-text').val(updatedComment); + $noteEl.find('.js-comment-save-button').click(); + + deferred.reject(); + const $updatedNoteEl = $notesContainer.find(`#note_${note.id}`); + expect($updatedNoteEl.hasClass('.being-posted')).toEqual(false); // Remove being-posted visuals + expect($updatedNoteEl.find('.note-text').text().trim()).toEqual(sampleComment); // See if comment reverted back to original + expect($('.flash-container').is(':visible')).toEqual(true); // Flash error message shown + }); + }); + describe('getFormData', () => { it('should return form metadata object from form reference', () => { - this.notes = new Notes(); + this.notes = new Notes('', []); const $form = $('form'); const sampleComment = 'foobar'; @@ -290,7 +373,7 @@ import '~/notes'; describe('hasSlashCommands', () => { beforeEach(() => { - this.notes = new Notes(); + this.notes = new Notes('', []); }); it('should return true when comment has slash commands', () => { @@ -312,7 +395,7 @@ import '~/notes'; const REGEX_SLASH_COMMANDS = /\/\w+/g; it('should strip slash commands from the comment', () => { - this.notes = new Notes(); + this.notes = new Notes('', []); const sampleComment = '/wip /milestone %1.0 /merge /unassign Merging this'; const stripedComment = this.notes.stripSlashCommands(sampleComment); @@ -327,7 +410,7 @@ import '~/notes'; const currentUserFullname = 'Administrator'; beforeEach(() => { - this.notes = new Notes(); + this.notes = new Notes('', []); }); it('should return constructed placeholder element for regular note based on form contents', () => { @@ -364,129 +447,5 @@ import '~/notes'; expect($tempNote.find('.timeline-content').hasClass('discussion')).toBeTruthy(); }); }); - - describe('postComment & updateComment', () => { - const sampleComment = 'foo'; - const updatedComment = 'bar'; - const note = { - id: 1234, - html: `
  • -
    ${sampleComment}
    -
    • `, - note: sampleComment, - valid: true - }; - let $form; - let $notesContainer; - - beforeEach(() => { - this.notes = new Notes(); - window.gon.current_username = 'root'; - window.gon.current_user_fullname = 'Administrator'; - $form = $('form'); - $notesContainer = $('ul.main-notes-list'); - $form.find('textarea.js-note-text').val(sampleComment); - $('.js-comment-button').click(); - }); - - it('should show placeholder note while new comment is being posted', () => { - expect($notesContainer.find('.note.being-posted').length > 0).toEqual(true); - }); - - it('should remove placeholder note when new comment is done posting', () => { - spyOn($, 'ajax').and.callFake((options) => { - options.success(note); - expect($notesContainer.find('.note.being-posted').length).toEqual(0); - }); - }); - - it('should show actual note element when new comment is done posting', () => { - spyOn($, 'ajax').and.callFake((options) => { - options.success(note); - expect($notesContainer.find(`#${note.id}`).length > 0).toEqual(true); - }); - }); - - it('should reset Form when new comment is done posting', () => { - spyOn($, 'ajax').and.callFake((options) => { - options.success(note); - expect($form.find('textarea.js-note-text')).toEqual(''); - }); - }); - - it('should trigger ajax:success event on Form when new comment is done posting', () => { - spyOn($, 'ajax').and.callFake((options) => { - options.success(note); - spyOn($form, 'trigger'); - expect($form.trigger).toHaveBeenCalledWith('ajax:success', [note]); - }); - }); - - it('should show flash error message when new comment failed to be posted', () => { - spyOn($, 'ajax').and.callFake((options) => { - options.error(); - expect($notesContainer.parent().find('.flash-container .flash-text').is(':visible')).toEqual(true); - }); - }); - - it('should refill form textarea with original comment content when new comment failed to be posted', () => { - spyOn($, 'ajax').and.callFake((options) => { - options.error(); - expect($form.find('textarea.js-note-text')).toEqual(sampleComment); - }); - }); - - it('should show updated comment as _actively being posted_ while comment being updated', () => { - spyOn($, 'ajax').and.callFake((options) => { - options.success(note); - const $noteEl = $notesContainer.find(`#note_${note.id}`); - $noteEl.find('.js-note-edit').click(); - $noteEl.find('textarea.js-note-text').val(updatedComment); - $noteEl.find('.js-comment-save-button').click(); - expect($noteEl.hasClass('.being-posted')).toEqual(true); - expect($noteEl.find('.note-text').text()).toEqual(updatedComment); - }); - }); - - it('should show updated comment when comment update is done posting', () => { - spyOn($, 'ajax').and.callFake((options) => { - options.success(note); - const $noteEl = $notesContainer.find(`#note_${note.id}`); - $noteEl.find('.js-note-edit').click(); - $noteEl.find('textarea.js-note-text').val(updatedComment); - $noteEl.find('.js-comment-save-button').click(); - - spyOn($, 'ajax').and.callFake((updateOptions) => { - const updatedNote = Object.assign({}, note); - updatedNote.note = updatedComment; - updatedNote.html = `
  • -
    ${updatedComment}
    -
    • `; - updateOptions.success(updatedNote); - const $updatedNoteEl = $notesContainer.find(`#note_${updatedNote.id}`); - expect($updatedNoteEl.hasClass('.being-posted')).toEqual(false); // Remove being-posted visuals - expect($updatedNoteEl.find('note-text').text().trim()).toEqual(updatedComment); // Verify if comment text updated - }); - }); - }); - - it('should show flash error message when comment failed to be updated', () => { - spyOn($, 'ajax').and.callFake((options) => { - options.success(note); - const $noteEl = $notesContainer.find(`#note_${note.id}`); - $noteEl.find('.js-note-edit').click(); - $noteEl.find('textarea.js-note-text').val(updatedComment); - $noteEl.find('.js-comment-save-button').click(); - - spyOn($, 'ajax').and.callFake((updateOptions) => { - updateOptions.error(); - const $updatedNoteEl = $notesContainer.find(`#note_${note.id}`); - expect($updatedNoteEl.hasClass('.being-posted')).toEqual(false); // Remove being-posted visuals - expect($updatedNoteEl.find('note-text').text().trim()).toEqual(sampleComment); // See if comment reverted back to original - expect($notesContainer.parent().find('.flash-container .flash-text').is(':visible')).toEqual(true); // Flash error message shown - }); - }); - }); - }); }); }).call(window); -- GitLab