diff --git a/app/assets/javascripts/gl_form.js b/app/assets/javascripts/gl_form.js
index 7dc2d13e5d8e8e83d0d6943c2d66c45ad03f9fba..04814fa0843cee9cad8d5549cf9b8cc246572709 100644
--- a/app/assets/javascripts/gl_form.js
+++ b/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();
diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js
index 0a0e73e0cccf7b0692c786d921d3de5400baecb3..31a71379af3886616cbdf0fd88193e14fb088a48 100644
--- a/app/assets/javascripts/lib/utils/common_utils.js
+++ b/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);
diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js
index a8b9a352870b50d5bfc9c128e72c9b49417fba89..8de5a6191b6cd1ac364e71b2cdbee6900a85f211 100644
--- a/app/assets/javascripts/notes.js
+++ b/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) {
diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss
index 9f1c6a8a26478caba5e18126aa2d25832351e481..f984b469609731a480e3cfc5aea03d4edb557bda 100644
--- a/app/assets/stylesheets/pages/note_form.scss
+++ b/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;
+ }
+}
diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss
index 38c548908f87623b3f33d3df68b20cb14c232603..ad4c31ca29e92b188674ea45c9db5230650493e4 100644
--- a/app/assets/stylesheets/pages/notes.scss
+++ b/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;
diff --git a/app/views/projects/notes/_edit_form.html.haml b/app/views/projects/notes/_edit_form.html.haml
index 8620f4922821725f50521b6ca467a010a669c36b..d36b4e6c8ab03d3cadf9d797f1d1899dbe42101e 100644
--- a/app/views/projects/notes/_edit_form.html.haml
+++ b/app/views/projects/notes/_edit_form.html.haml
@@ -1,11 +1,15 @@
.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
diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml
index eb869ea85bfb42c551628eaca9078428b01be612..36c388c3318ae2505b39a9082531e600f6cc403c 100644
--- a/app/views/projects/notes/_note.html.haml
+++ b/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
diff --git a/app/views/projects/notes/_notes_with_form.html.haml b/app/views/projects/notes/_notes_with_form.html.haml
index 00b62a595ff213fe190071003c1868f6cb5fdf06..f01bcfac898929a90ea3d11e1a87d3bdcb36396e 100644
--- a/app/views/projects/notes/_notes_with_form.html.haml
+++ b/app/views/projects/notes/_notes_with_form.html.haml
@@ -1,5 +1,8 @@
%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
diff --git a/changelogs/unreleased/single-edit-comment-widget-2.yml b/changelogs/unreleased/single-edit-comment-widget-2.yml
new file mode 100644
index 0000000000000000000000000000000000000000..e8b8beb15dee7b09487dcba2596f9eeba66c6e27
--- /dev/null
+++ b/changelogs/unreleased/single-edit-comment-widget-2.yml
@@ -0,0 +1,5 @@
+---
+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:
diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb
index 9fffbb43e87e493f6912de64b00dfa5ef57d2d70..b785b2f7704a580c9e4387f0a4f5bd5685ff71bb 100644
--- a/spec/features/notes_on_merge_requests_spec.rb
+++ b/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
diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js
index d3bfa7730fa29029f8532a30e08a346677a4cfb0..bb13af7ac0c1f02ed6d2efdcdd2a5dce462904e6 100644
--- a/spec/javascripts/notes_spec.js
+++ b/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() {
diff --git a/spec/javascripts/zen_mode_spec.js b/spec/javascripts/zen_mode_spec.js
index d80ce5a7f7e3a546cb6213027923d13db5fd5424..5b4d007c8f7363b7f555d83520f59cceb544ed9c 100644
--- a/spec/javascripts/zen_mode_spec.js
+++ b/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
}));
};