Merge branch 'single-edit-comment-widget-2' into 'master'

Refactor discussion edit widget to have only one at a time.

Closes #23227

See merge request !8356

app/assets/javascripts/gl_form.js

@@ -35,8 +35,8 @@
         autosize(this.textarea);
         // form and textarea event listeners
         this.addEventListeners();
-        gl.text.init(this.form);
       }
+      gl.text.init(this.form);
       // hide discard button
       this.form.find('.js-note-discard').hide();
       return this.form.show();

app/assets/javascripts/lib/utils/common_utils.js

@@ -106,8 +106,9 @@
       );
     };
 
-    gl.utils.getPagePath = function() {
-      return $('body').data('page').split(':')[0];
+    gl.utils.getPagePath = function(index) {
+      index = index || 0;
+      return $('body').data('page').split(':')[index];
     };
 
     gl.utils.parseUrl = function (url) {
@@ -127,6 +128,17 @@
       return e.metaKey || e.ctrlKey || e.altKey || e.shiftKey;
     };
 
+    gl.utils.scrollToElement = function($el) {
+      var top = $el.offset().top;
+      gl.navBarHeight = gl.navBarHeight || $('.navbar-gitlab').height();
+      gl.navLinksHeight = gl.navLinksHeight || $('.nav-links').height();
+      gl.mrTabsHeight = gl.mrTabsHeight || $('.merge-request-tabs').height();
+
+      return $('body, html').animate({
+        scrollTop: top - (gl.navBarHeight + gl.navLinksHeight + gl.mrTabsHeight)
+      }, 200);
+    };
+
   })(window);
 
 }).call(this);

app/assets/javascripts/notes.js

@@ -52,6 +52,12 @@
       this.setupMainTargetNoteForm();
       this.initTaskList();
       this.collapseLongCommitList();
+
+      // We are in the Merge Requests page so we need another edit form for Changes tab
+      if (gl.utils.getPagePath(1) === 'merge_requests') {
+        $('.note-edit-form').clone()
+          .addClass('mr-note-edit-form').insertAfter('.note-edit-form');
+      }
     }
 
     Notes.prototype.addBinding = function() {
@@ -63,7 +69,7 @@
       // 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);
+      $(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);
@@ -466,6 +472,7 @@
       var $html, $note_li;
       // Convert returned HTML to a jQuery object so we can modify it further
       $html = $(note.html);
+      this.revertNoteEditForm();
       gl.utils.localTimeAgo($('.js-timeago', $html));
       $html.renderGFM();
       $html.find('.js-task-list-container').taskList('enable');
@@ -480,48 +487,56 @@
     };
 
 
+    Notes.prototype.checkContentToAllowEditing = function($el) {
+      var initialContent = $el.find('.original-note-content').text().trim();
+      var currentContent = $el.find('.note-textarea').val();
+      var isAllowed = true;
+
+      if (currentContent === initialContent) {
+        this.removeNoteEditForm($el);
+      }
+      else {
+        var $buttons = $el.find('.note-form-actions');
+        var isWidgetVisible = gl.utils.isInViewport($el.get(0));
+
+        if (!isWidgetVisible) {
+          gl.utils.scrollToElement($el);
+        }
+
+        $el.find('.js-edit-warning').show();
+        isAllowed = false;
+      }
+
+      return isAllowed;
+    }
+
+
     /*
     Called in response to clicking the edit note link
 
     Replaces the note text with the note edit form
     Adds a data attribute to the form with the original content of the note for cancellations
-     */
-
+    */
     Notes.prototype.showEditForm = function(e, scrollTo, myLastNote) {
-      var $noteText, done, form, note;
       e.preventDefault();
-      note = $(this).closest(".note");
-      note.addClass("is-editting");
-      form = note.find(".note-edit-form");
-      form.addClass('current-note-edit-form');
-      // Show the attachment delete link
-      note.find(".js-note-attachment-delete").show();
-      done = function($noteText) {
-        var noteTextVal;
-        // Neat little trick to put the cursor at the end
-        noteTextVal = $noteText.val();
-        // Store the original note text in a data attribute to retrieve if a user cancels edit.
-        form.find('form.edit-note').data('original-note', noteTextVal);
-        return $noteText.val('').val(noteTextVal);
-      };
-      new GLForm(form);
-      if ((scrollTo != null) && (myLastNote != null)) {
-        // scroll to the bottom
-        // so the open of the last element doesn't make a jump
-        $('html, body').scrollTop($(document).height());
-        return $('html, body').animate({
-          scrollTop: myLastNote.offset().top - 150
-        }, 500, function() {
-          var $noteText;
-          $noteText = form.find(".js-note-text");
-          $noteText.focus();
-          return done($noteText);
-        });
-      } else {
-        $noteText = form.find('.js-note-text');
-        $noteText.focus();
-        return done($noteText);
+
+      var $target = $(e.target);
+      var $editForm = $(this.getEditFormSelector($target));
+      var $note = $target.closest('.note');
+      var $currentlyEditing = $('.note.is-editting:visible');
+
+      if ($currentlyEditing.length) {
+        var isEditAllowed = this.checkContentToAllowEditing($currentlyEditing);
+
+        if (!isEditAllowed) {
+          return;
+        }
       }
+
+      $note.find('.js-note-attachment-delete').show();
+      $editForm.addClass('current-note-edit-form');
+      $note.addClass('is-editting');
+      this.putEditFormInPlace($target);
     };
 
 
@@ -532,19 +547,41 @@
      */
 
     Notes.prototype.cancelEdit = function(e) {
-      var note;
       e.preventDefault();
-      note = $(e.target).closest('.note');
+      var $target = $(e.target);
+      var note = $target.closest('.note');
+      note.find('.js-edit-warning').hide();
+      this.revertNoteEditForm($target);
       return this.removeNoteEditForm(note);
     };
 
+    Notes.prototype.revertNoteEditForm = function($target) {
+      $target = $target || $('.note.is-editting:visible');
+      var selector = this.getEditFormSelector($target);
+      var $editForm = $(selector);
+
+      $editForm.insertBefore('.notes-form');
+      $editForm.find('.js-comment-button').enable();
+      $editForm.find('.js-edit-warning').hide();
+    };
+
+    Notes.prototype.getEditFormSelector = function($el) {
+      var selector = '.note-edit-form:not(.mr-note-edit-form)';
+
+      if ($el.parents('#diffs').length) {
+        selector = '.note-edit-form.mr-note-edit-form';
+      }
+
+      return selector;
+    };
+
     Notes.prototype.removeNoteEditForm = function(note) {
-      var form;
-      form = note.find(".current-note-edit-form");
-      note.removeClass("is-editting");
-      form.removeClass("current-note-edit-form");
+      var form = note.find('.current-note-edit-form');
+      note.removeClass('is-editting');
+      form.removeClass('current-note-edit-form');
+      form.find('.js-edit-warning').hide();
       // Replace markdown textarea text with original note text.
-      return form.find(".js-note-text").val(form.find('form.edit-note').data('original-note'));
+      return form.find('.js-note-text').val(form.find('form.edit-note').data('original-note'));
     };
 
 
@@ -837,15 +874,44 @@
 
     Notes.prototype.initTaskList = function() {
       this.enableTaskList();
-      return $(document).on('tasklist:changed', '.note .js-task-list-container', this.updateTaskList);
+      return $(document).on('tasklist:changed', '.note .js-task-list-container', this.updateTaskList.bind(this));
     };
 
     Notes.prototype.enableTaskList = function() {
       return $('.note .js-task-list-container').taskList('enable');
     };
 
-    Notes.prototype.updateTaskList = function() {
-      return $('form', this).submit();
+    Notes.prototype.putEditFormInPlace = function($el) {
+      var $editForm = $(this.getEditFormSelector($el));
+      var $note = $el.closest('.note');
+
+      $editForm.insertAfter($note.find('.note-text'));
+
+      var $originalContentEl = $note.find('.original-note-content');
+      var originalContent = $originalContentEl.text().trim();
+      var postUrl = $originalContentEl.data('post-url');
+      var targetId = $originalContentEl.data('target-id');
+      var targetType = $originalContentEl.data('target-type');
+
+      new GLForm($editForm.find('form'));
+
+      $editForm.find('form').attr('action', postUrl);
+      $editForm.find('.js-form-target-id').val(targetId);
+      $editForm.find('.js-form-target-type').val(targetType);
+      $editForm.find('.js-note-text').focus().val(originalContent);
+      $editForm.find('.js-md-write-button').trigger('click');
+      $editForm.find('.referenced-users').hide();
+    }
+
+    Notes.prototype.updateTaskList = function(e) {
+      var $target = $(e.target);
+      var $list = $target.closest('.js-task-list-container');
+      var $editForm = $(this.getEditFormSelector($target));
+      var $note = $list.closest('.note');
+
+      this.putEditFormInPlace($list);
+      $editForm.find('#note_note').val($note.find('.original-task-list').val());
+      $('form', $list).submit();
     };
 
     Notes.prototype.updateNotesCount = function(updateCount) {

app/assets/stylesheets/pages/note_form.scss

@@ -27,6 +27,7 @@
 .new-note,
 .note-edit-form {
   .note-form-actions {
+    position: relative;
     margin-top: $gl-padding;
   }
 
@@ -265,3 +266,18 @@
     }
   }
 }
+
+.note-edit-warning.settings-message {
+  display: none;
+  padding: 5px 10px;
+  position: absolute;
+  left: 127px;
+  top: 2px;
+
+  @media (max-width: $screen-xs-max) {
+    position: relative;
+    top: 0;
+    left: 0;
+    margin-bottom: 10px;
+  }
+}

app/assets/stylesheets/pages/notes.scss

@@ -588,13 +588,11 @@ ul.notes {
 
 // Merge request notes in diffs
 .diff-file {
-
   // Diff is side by side
   .notes_content.parallel .note-header .note-headline-light {
     display: block;
     position: relative;
   }
-
   // Diff is inline
   .notes_content .note-header .note-headline-light {
     display: inline-block;

app/views/projects/notes/_edit_form.html.haml

 .note-edit-form
-  = form_for note, url: namespace_project_note_path(@project.namespace, @project, note), method: :put, remote: true, authenticity_token: true, html: { class: 'edit-note common-note-form js-quick-submit' } do |f|
-    = note_target_fields(note)
-    = render layout: 'projects/md_preview', locals: { preview_class: 'md-preview' } do
-      = render 'projects/zen', f: f, attr: :note, classes: 'note-textarea js-note-text js-task-list-field', placeholder: "Write a comment or drag your files here..."
+  = form_tag '#', method: :put, remote: true, class: 'edit-note common-note-form js-quick-submit' do
+    = hidden_field_tag :authenticity_token, form_authenticity_token
+    = hidden_field_tag :target_id, '', class: 'js-form-target-id'
+    = hidden_field_tag :target_type, '', class: 'js-form-target-type'
+    = render layout: 'projects/md_preview', locals: { preview_class: 'md-preview', referenced_users: true } do
+      = render 'projects/zen', attr: 'note[note]', classes: 'note-textarea js-note-text js-task-list-field', placeholder: "Write a comment or drag your files here..."
       = render 'projects/notes/hints'
 
     .note-form-actions.clearfix
-      = f.submit 'Save Comment', class: 'btn btn-nr btn-save js-comment-button'
+      .settings-message.note-edit-warning.js-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-cancel.note-edit-cancel{ type: 'button' }
         Cancel

app/views/projects/notes/_note.html.haml

@@ -67,7 +67,9 @@
             = note.redacted_note_html
         = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago', include_author: true)
         - if note_editable
-          = render 'projects/notes/edit_form', note: note
+          .original-note-content.hidden{ data: { post_url: namespace_project_note_path(@project.namespace, @project, note), target_id: note.noteable.id, target_type: note.noteable.class.name.underscore } }
+            #{note.note}
+          %textarea.hidden.js-task-list-field.original-task-list #{note.note}
         .note-awards
           = render 'award_emoji/awards_block', awardable: note, inline: false
         - if note.system

app/views/projects/notes/_notes_with_form.html.haml

 %ul#notes-list.notes.main-notes-list.timeline
   = render "projects/notes/notes"
+
+= render 'projects/notes/edit_form'
+
 %ul.notes.notes-form.timeline
   %li.timeline-entry
     .flash-container.timeline-content

changelogs/unreleased/single-edit-comment-widget-2.yml

+---
+title: Refactored note edit form to improve frontend performance on MR and Issues
+  pages, especially pages with has a lot of discussions in it
+merge_request: 8356
+author:

spec/features/notes_on_merge_requests_spec.rb

@@ -70,16 +70,15 @@ describe 'Comments', feature: true do
     end
 
     describe 'when editing a note', js: true do
-      it 'contains the hidden edit form' do
-        page.within("#note_#{note.id}") do
-          is_expected.to have_css('.note-edit-form', visible: false)
-        end
+      it 'there should be a hidden edit form' do
+        is_expected.to have_css('.note-edit-form:not(.mr-note-edit-form)', visible: false, count: 1)
+        is_expected.to have_css('.note-edit-form.mr-note-edit-form', visible: false, count: 1)
       end
 
       describe 'editing the note' do
         before do
           find('.note').hover
-          find(".js-note-edit").click
+          find('.js-note-edit').click
         end
 
         it 'shows the note edit form and hide the note body' do
@@ -90,14 +89,29 @@ describe 'Comments', feature: true do
           end
         end
 
-        # TODO: fix after 7.7 release
-        # it "should reset the edit note form textarea with the original content of the note if cancelled" do
-        #   within(".current-note-edit-form") do
-        #     fill_in "note[note]", with: "Some new content"
-        #     find(".btn-cancel").click
-        #     expect(find(".js-note-text", visible: false).text).to eq note.note
-        #   end
-        # end
+        it 'should reset the edit note form textarea with the original content of the note if cancelled' do
+          within('.current-note-edit-form') do
+            fill_in 'note[note]', with: 'Some new content'
+            find('.btn-cancel').click
+            expect(find('.js-note-text', visible: false).text).to eq ''
+          end
+        end
+
+        it 'allows using markdown buttons after saving a note and then trying to edit it again' do
+          page.within('.current-note-edit-form') do
+            fill_in 'note[note]', with: 'This is the new content'
+            find('.btn-save').click
+          end
+
+          find('.note').hover
+          find('.js-note-edit').click
+
+          page.within('.current-note-edit-form') do
+            expect(find('#note_note').value).to eq('This is the new content')
+            find('.js-md:first-child').click
+            expect(find('#note_note').value).to eq('This is the new content****')
+          end
+        end
 
         it 'appends the edited at time to the note' do
           page.within('.current-note-edit-form') do

spec/javascripts/notes_spec.js

@@ -19,6 +19,7 @@
       fixture.load(commentsTemplate);
       gl.utils.disableButtonIfEmptyField = _.noop;
       window.project_uploads_path = 'http://test.host/uploads';
+      $('body').data('page', 'projects:issues:show');
     });
 
     describe('task lists', function() {

spec/javascripts/zen_mode_spec.js

@@ -32,9 +32,9 @@
         return expect(Mousetrap.pause).toHaveBeenCalled();
       });
       return it('removes textarea styling', function() {
-        $('textarea').attr('style', 'height: 400px');
+        $('.notes-form textarea').attr('style', 'height: 400px');
         enterZen();
-        return expect('textarea').not.toHaveAttr('style');
+        return expect($('.notes-form textarea')).not.toHaveAttr('style');
       });
     });
     describe('in use', function() {
@@ -43,7 +43,7 @@
       });
       return it('exits on Escape', function() {
         escapeKeydown();
-        return expect($('.zen-backdrop')).not.toHaveClass('fullscreen');
+        return expect($('.notes-form .zen-backdrop')).not.toHaveClass('fullscreen');
       });
     });
     return describe('on exit', function() {
@@ -64,15 +64,15 @@
   });
 
   enterZen = function() {
-    return $('.js-zen-enter').click();
+    return $('.notes-form .js-zen-enter').click();
   };
 
-  exitZen = function() { // Ohmmmmmmm
-    return $('.js-zen-leave').click();
+  exitZen = function() {
+    return $('.notes-form .js-zen-leave').click();
   };
 
   escapeKeydown = function() {
-    return $('textarea').trigger($.Event('keydown', {
+    return $('.notes-form textarea').trigger($.Event('keydown', {
       keyCode: 27
     }));
   };