From 60a73bab1c83c9f8065aa1c642a7ca2c80cd6d2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 20 Nov 2017 22:14:25 +0100 Subject: [PATCH 1/4] Submit comments with Enter and use Shift+Enter for new lines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- apps/comments/js/commentstabview.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/comments/js/commentstabview.js b/apps/comments/js/commentstabview.js index 7398a709421..0d752877b04 100644 --- a/apps/comments/js/commentstabview.js +++ b/apps/comments/js/commentstabview.js @@ -517,9 +517,10 @@ $field.toggleClass('error', limitExceeded); $submitButton.prop('disabled', limitExceeded); - //submits form on ctrl+Enter or cmd+Enter - if (ev.keyCode === 13 && (ev.ctrlKey || ev.metaKey)) { + // Submits form with Enter, but Shift+Enter is a new line + if (ev.keyCode === 13 && !ev.shiftKey) { $submitButton.click(); + ev.preventDefault(); } }, From d03fcbca77368e749cc30c899c38ace1d414a94d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 30 Nov 2017 19:25:50 +0100 Subject: [PATCH 2/4] Set text only in the message div of the new comment form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When finding ".message" elements on "view.$el" the message area for the new comment form and all the comments were matched. Now the selector was restricted to match only the message area for the new comment form. Signed-off-by: Daniel Calviño Sánchez --- apps/comments/tests/js/commentstabviewSpec.js | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/apps/comments/tests/js/commentstabviewSpec.js b/apps/comments/tests/js/commentstabviewSpec.js index 8b99ad081cd..d2170a37f7d 100644 --- a/apps/comments/tests/js/commentstabviewSpec.js +++ b/apps/comments/tests/js/commentstabviewSpec.js @@ -219,6 +219,7 @@ describe('OCA.Comments.CommentsTabView tests', function() { describe('posting comments', function() { var createStub; var currentUserStub; + var $newCommentForm; beforeEach(function() { view.collection.set(testComments); @@ -229,6 +230,8 @@ describe('OCA.Comments.CommentsTabView tests', function() { displayName: 'Test User' }); + $newCommentForm = view.$el.find('.newCommentForm'); + // Required for the absolute selector used to find the new comment // after a successful creation in _onSubmitSuccess. $('#testArea').append(view.$el); @@ -239,8 +242,8 @@ describe('OCA.Comments.CommentsTabView tests', function() { }); it('creates a new comment when clicking post button', function() { - view.$el.find('.message').text('New message'); - view.$el.find('form').submit(); + $newCommentForm.find('.message').text('New message'); + $newCommentForm.submit(); expect(createStub.calledOnce).toEqual(true); expect(createStub.lastCall.args[0]).toEqual({ @@ -253,8 +256,8 @@ describe('OCA.Comments.CommentsTabView tests', function() { }); }); it('creates a new comment with mentions when clicking post button', function() { - view.$el.find('.message').text('New message @anotheruser'); - view.$el.find('form').submit(); + $newCommentForm.find('.message').text('New message @anotheruser'); + $newCommentForm.submit(); var createStubExpectedData = { actorId: 'testuser', @@ -297,8 +300,8 @@ describe('OCA.Comments.CommentsTabView tests', function() { expect($message.find('.avatar[data-user=anotheruser] ~ .contactsmenu-popover').length).toEqual(1); }); it('does not create a comment if the field is empty', function() { - view.$el.find('.message').val(' '); - view.$el.find('form').submit(); + $newCommentForm.find('.message').val(' '); + $newCommentForm.submit(); expect(createStub.notCalled).toEqual(true); }); @@ -307,8 +310,8 @@ describe('OCA.Comments.CommentsTabView tests', function() { for (var i = 0; i < view._commentMaxLength * 2; i++) { bigMessage += 'a'; } - view.$el.find('.message').val(bigMessage); - view.$el.find('form').submit(); + $newCommentForm.find('.message').val(bigMessage); + $newCommentForm.submit(); expect(createStub.notCalled).toEqual(true); }); @@ -319,8 +322,8 @@ describe('OCA.Comments.CommentsTabView tests', function() { beforeEach(function() { tooltipStub = sinon.stub($.fn, 'tooltip'); - $message = view.$el.find('.message'); - $submitButton = view.$el.find('.submit'); + $message = $newCommentForm.find('.message'); + $submitButton = $newCommentForm.find('.submit'); }); afterEach(function() { tooltipStub.restore(); From c43f64d56b517e061ceb48e30ebaa48cc7866236 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 30 Nov 2017 19:30:27 +0100 Subject: [PATCH 3/4] Add unit tests for posting comments with enter key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- apps/comments/tests/js/commentstabviewSpec.js | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/apps/comments/tests/js/commentstabviewSpec.js b/apps/comments/tests/js/commentstabviewSpec.js index d2170a37f7d..d4456728f02 100644 --- a/apps/comments/tests/js/commentstabviewSpec.js +++ b/apps/comments/tests/js/commentstabviewSpec.js @@ -255,6 +255,34 @@ describe('OCA.Comments.CommentsTabView tests', function() { creationDateTime: new Date(Date.UTC(2016, 1, 3, 10, 5, 9)).toUTCString() }); }); + it('creates a new comment when typing enter', function() { + $newCommentForm.find('.message').text('New message'); + var keydownEvent = new $.Event('keydown', {keyCode: 13}); + $newCommentForm.find('.message').trigger(keydownEvent); + + expect(createStub.calledOnce).toEqual(true); + expect(createStub.lastCall.args[0]).toEqual({ + actorId: 'testuser', + actorDisplayName: 'Test User', + actorType: 'users', + verb: 'comment', + message: 'New message', + creationDateTime: new Date(Date.UTC(2016, 1, 3, 10, 5, 9)).toUTCString() + }); + expect(keydownEvent.isDefaultPrevented()).toEqual(true); + }); + it('creates a new line when typing shift+enter', function() { + $newCommentForm.find('.message').text('New message'); + var keydownEvent = new $.Event('keydown', {keyCode: 13, shiftKey: true}); + $newCommentForm.find('.message').trigger(keydownEvent); + + expect(createStub.calledOnce).toEqual(false); + // PhantomJS does not seem to handle typing in a contenteditable, so + // instead of looking for a new line the best that can be done is + // checking that the default behaviour would have been executed. + expect($newCommentForm.find('.message').text()).toContain('New message'); + expect(keydownEvent.isDefaultPrevented()).toEqual(false); + }); it('creates a new comment with mentions when clicking post button', function() { $newCommentForm.find('.message').text('New message @anotheruser'); $newCommentForm.submit(); From 9b1f3b969e8c273d86901432421189295383d81f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 7 Dec 2017 04:59:29 +0100 Subject: [PATCH 4/4] Fix Enter sending comment instead of adding autocomplete item to message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the autocomplete popover is shown the At.js plugin listens on the message input field for key down events, and when Enter is pressed it adds the selected item to the message. However, as "_onTypeComment" also handles key down events for the message input field, when Enter was pressed the comment was submitted and At.js had no chance to add the item before that happened. Now when Enter is pressed and the autocomplete popover is shown the comment is not submitted, and thus At.js adds the selected item to the message as expected. Signed-off-by: Daniel Calviño Sánchez --- apps/comments/js/commentstabview.js | 7 ++- apps/comments/tests/js/commentstabviewSpec.js | 45 +++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/apps/comments/js/commentstabview.js b/apps/comments/js/commentstabview.js index 0d752877b04..2ab6349d98a 100644 --- a/apps/comments/js/commentstabview.js +++ b/apps/comments/js/commentstabview.js @@ -517,8 +517,11 @@ $field.toggleClass('error', limitExceeded); $submitButton.prop('disabled', limitExceeded); - // Submits form with Enter, but Shift+Enter is a new line - if (ev.keyCode === 13 && !ev.shiftKey) { + // Submits form with Enter, but Shift+Enter is a new line. If the + // autocomplete popover is being shown Enter does not submit the + // form either; it will be handled by At.js which will add the + // currently selected item to the message. + if (ev.keyCode === 13 && !ev.shiftKey && !$field.atwho('isSelecting')) { $submitButton.click(); ev.preventDefault(); } diff --git a/apps/comments/tests/js/commentstabviewSpec.js b/apps/comments/tests/js/commentstabviewSpec.js index d4456728f02..813b2a72eae 100644 --- a/apps/comments/tests/js/commentstabviewSpec.js +++ b/apps/comments/tests/js/commentstabviewSpec.js @@ -271,6 +271,51 @@ describe('OCA.Comments.CommentsTabView tests', function() { }); expect(keydownEvent.isDefaultPrevented()).toEqual(true); }); + it('creates a new mention when typing enter in the autocomplete popover', function() { + var autoCompleteStub = sinon.stub(view, '_onAutoComplete'); + autoCompleteStub.callsArgWith(1, [{"id":"userId", "label":"User Name", "source":"users"}]); + + // Force the autocomplete to be initialized + view._initAutoComplete($newCommentForm.find('.message')); + + // PhantomJS does not seem to handle typing in a contenteditable, so + // some tricks are needed to show the autocomplete popover. + // + // Instead of sending key events to type "@u" the characters are + // programatically set in the input field. + $newCommentForm.find('.message').text('Mention to @u'); + + // When focusing on the input field the caret is not guaranteed to + // be at the end; instead of calling "focus()" on the input field + // the caret is explicitly set at the end of the input field, that + // is, after "@u". + var range = document.createRange(); + range.selectNodeContents($newCommentForm.find('.message')[0]); + range.collapse(false); + var selection = window.getSelection(); + selection.removeAllRanges(); + selection.addRange(range); + + // As PhantomJS does not handle typing in a contenteditable the key + // typed here is in practice ignored by At.js, but despite that it + // will cause the popover to be shown. + $newCommentForm.find('.message').trigger(new $.Event('keydown', {keyCode: 's'})); + $newCommentForm.find('.message').trigger(new $.Event('keyup', {keyCode: 's'})); + + expect(autoCompleteStub.calledOnce).toEqual(true); + + var keydownEvent = new $.Event('keydown', {keyCode: 13}); + $newCommentForm.find('.message').trigger(keydownEvent); + + expect(createStub.calledOnce).toEqual(false); + expect($newCommentForm.find('.message').html()).toContain('Mention to User Name'); + expect($newCommentForm.find('.message').text()).not.toContain('@'); + // In this case the default behaviour is prevented by the + // "onKeydown" event handler of At.js. + expect(keydownEvent.isDefaultPrevented()).toEqual(true); + }); it('creates a new line when typing shift+enter', function() { $newCommentForm.find('.message').text('New message'); var keydownEvent = new $.Event('keydown', {keyCode: 13, shiftKey: true});