From 520a60d1bef5087a7a1315fb6718be9556b4bcd0 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Sun, 13 Aug 2017 18:46:35 +0200 Subject: [PATCH 01/14] Use popovers with formatted comments instead of tooltips --- app/views/request_for_comments/show.html.erb | 27 +++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/app/views/request_for_comments/show.html.erb b/app/views/request_for_comments/show.html.erb index 6a21ca24..ce6f8d8e 100644 --- a/app/views/request_for_comments/show.html.erb +++ b/app/views/request_for_comments/show.html.erb @@ -153,18 +153,37 @@ also, all settings from the rails model needed for the editor configuration in t }); jqrequest.done(function(response){ - $.each(response, function(index, comment) { - comment.className = "code-ocean_comment"; + var comments = response.slice().sort(function (a, b) { + return a.row > b.row; + }); + while (comments.length > 0) { + var cluster = []; + var clusterRow = comments[0].row; + while (comments.length > 0 && comments[0].row === clusterRow) { + cluster.push(comments.shift()); + } + var popupContent = ''; + cluster.forEach(function(comment) { + popupContent += '

' + comment.username + ': ' + comment.text.replace(/\n/g, '
') + '

'; + }); + var icon = $('*[data-file-id="' + fileid + '"] > .ace_gutter > .ace_gutter-layer > .ace_gutter-cell:contains("' + (clusterRow + 1) + '")'); + icon.popover({ + content: popupContent, + html: true, + trigger: 'hover', + container: 'body' + }); + } + $.each(response, function(index, comment) { + comment.className = 'code-ocean_comment'; // if we have tabs or carriage returns in the comment, just add the name and leave it as it is. otherwise: format! if(comment.text.includes("\n") || comment.text.includes("\t")){ comment.text = comment.username + ": " + comment.text; } else { comment.text = comment.username + ": " + stringDivider(comment.text, 80, "\n"); } - }); - session.setAnnotations(response); }) } From e1c45f025fd294519569fea5b9b33a33d671790a Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Sun, 13 Aug 2017 18:46:55 +0200 Subject: [PATCH 02/14] Hide Ace's tooltips for annotations --- app/assets/stylesheets/request-for-comments.css.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/assets/stylesheets/request-for-comments.css.scss b/app/assets/stylesheets/request-for-comments.css.scss index cbaf9d36..87eeba44 100644 --- a/app/assets/stylesheets/request-for-comments.css.scss +++ b/app/assets/stylesheets/request-for-comments.css.scss @@ -17,3 +17,7 @@ width: 100%; height: 200px; } + +.ace_tooltip { + display: none !important; +} From 0f3db8a644fae7ee39f952dcb10726f5ffd5e454 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 16 Aug 2017 17:29:08 +0200 Subject: [PATCH 03/14] Remove old comments before refresh to fix event leak --- app/views/request_for_comments/show.html.erb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/views/request_for_comments/show.html.erb b/app/views/request_for_comments/show.html.erb index ce6f8d8e..3e889ac6 100644 --- a/app/views/request_for_comments/show.html.erb +++ b/app/views/request_for_comments/show.html.erb @@ -140,6 +140,11 @@ also, all settings from the rails model needed for the editor configuration in t currentEditor.on("guttermousedown", handleSidebarClick); }); + function cleanupPopovers() { + // remove all possible popovers + $('.editor > .ace_gutter > .ace_gutter-layer > .ace_gutter-cell').popover('destroy'); + } + function setAnnotations(editor, fileid) { var session = editor.getSession(); @@ -166,7 +171,7 @@ also, all settings from the rails model needed for the editor configuration in t cluster.forEach(function(comment) { popupContent += '

' + comment.username + ': ' + comment.text.replace(/\n/g, '
') + '

'; }); - var icon = $('*[data-file-id="' + fileid + '"] > .ace_gutter > .ace_gutter-layer > .ace_gutter-cell:contains("' + (clusterRow + 1) + '")'); + var icon = $('*[data-file-id="' + fileid + '"] > .ace_gutter > .ace_gutter-layer > div:nth-child(' + (clusterRow + 1) + ')'); icon.popover({ content: popupContent, html: true, @@ -201,6 +206,7 @@ also, all settings from the rails model needed for the editor configuration in t } function deleteComment(file_id, row, editor) { + cleanupPopovers(); var jqxhr = $.ajax({ type: 'DELETE', url: "/comments", @@ -215,6 +221,7 @@ also, all settings from the rails model needed for the editor configuration in t } function createComment(file_id, row, editor, commenttext){ + cleanupPopovers(); var jqxhr = $.ajax({ data: { comment: { From 1ffda4f89685419dd4750802214d633a1aa61f53 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 16 Aug 2017 17:29:26 +0200 Subject: [PATCH 04/14] Fix cluster sorting --- app/views/request_for_comments/show.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/request_for_comments/show.html.erb b/app/views/request_for_comments/show.html.erb index 3e889ac6..4907949b 100644 --- a/app/views/request_for_comments/show.html.erb +++ b/app/views/request_for_comments/show.html.erb @@ -159,7 +159,7 @@ also, all settings from the rails model needed for the editor configuration in t jqrequest.done(function(response){ var comments = response.slice().sort(function (a, b) { - return a.row > b.row; + return a.row - b.row; }); while (comments.length > 0) { var cluster = []; From b5c997e8a959eb9623ffff109eadfdbd752cd46a Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Wed, 16 Aug 2017 18:20:56 +0200 Subject: [PATCH 05/14] Preprocess comment text to protect from XSS attacks --- app/views/request_for_comments/show.html.erb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app/views/request_for_comments/show.html.erb b/app/views/request_for_comments/show.html.erb index 4907949b..96470d05 100644 --- a/app/views/request_for_comments/show.html.erb +++ b/app/views/request_for_comments/show.html.erb @@ -64,6 +64,8 @@


+ +