Fix Bug 1611510 - Implement Source String Comments

[abowler2]

Initial work towards implementing source string comments. Replaced the textarea for the comments with a Slate editor and added the ability to @ mention users.

---

Work still needed:

• Add ability to linkify links
• Add actual users from database for mentions
• Fix Flow errors
• Fix crash when submitting on enter
• Find a way for editor to grow when a \n is added
• Find out why height limit is not being honored
• Find a way to check if only \n added to avoid empty comments
• See if there is a way to remove !important from CSS change
• Fix suggestions staying fixed when div scrolls

---

TODO:

• Convert existing comments to new format (this may not be needed)
• Fix or file notification/focus bug
• Add the "Request context" / "Report issue" link in the metadata section
• Add the ability for PMs to "Pin" their responses
• See if there is a way to select a mention with mouse click

[mathjazz]

Thanks for the patch and keeping up this tedious work!

I'll ignore nits and smaller issues in my review for now so that we can first mitigate the risk of needing to drop this library. However, since we hit the next two issues in the previous attempt as well and we might close the old PR soon, I'll list them here again:

1. Break long words into new lines: abeb790#diff-917d1532b1d18eaa3aa01ddd93c9e17c
2. Fix the shrinking submit button issue: 16f966c#diff-97a5a5d80f7699b198c5465ea7237e53

We should also copy both TODO items over from #1609.

[abowler2]

Rebased with master

[abowler2]

After spending some time working out how to detect links while comments are added I ran into issues finding the right solution for detecting links that did not include a protocol.

In the end I decided to use html-react-parser to parse the html into react elements which allowed me to again use Linkify when rendering the comments.

I was also able to handle the added \n when checking for empty comments as well as remove any trailing \n to prevent blank lines being added to the end of a comment.

[abowler2]

Just pushed up my most recent work which adds the ability to get user data from the DB for the suggestions, keeps the suggestions from overflowing the edge of the window, and removes the work for adding the Request context / Report issue button as it will be worked on as a separate patch.

The final known piece remaining is to fix the positioning of the suggestions when scroll is active.

Also, there are now conflicts with master around the terminology changes. However, I'm not sure how to handle the conflict in AddComment.js as I changed the way focus is handled due to autofocus not working. Please advise on how I should handle that.

[abowler2]

I was not able to figure out what Flow was asking for here.

[adngdb]

I'm pretty sure the number of people who fully understand Flow is lower than the number of fingers on the hand of an arm-less person.

[abowler2]

Mentions are now working 😃

I left a note above regarding issues with Flow, and I didn't tackle the conflicts because IIRC you were going to do a rebase for those since they involve the Terminology changes (let me know if you want me to go ahead and tackle that though). Otherwise, the tests pass for me locally so this should be ready for a review.

[mathjazz]

Thanks for the update, April!

The most important thing in rebasing this patch is to move the condition we currently use on the autoFocus param to ReactEditor.focus(editor);. Other than that, it seems pretty straightforward (just a little more manual work than usually, because it's a lot of commits).

Let me know if you still want me to do that myself.

[mathjazz]

Here are the proposed CSS changes for the mentions popup:

Diff:

diff --git a/frontend/src/core/comments/components/AddComment.css b/frontend/src/core/comments/components/AddComment.css
index 2f6fbebc4..e068f330d 100644
--- a/frontend/src/core/comments/components/AddComment.css
+++ b/frontend/src/core/comments/components/AddComment.css
@@ -32,18 +32,31 @@
     padding: 10px 12px;
     background-color: #272a2f;
     border: 1px solid #333941;
-    border-radius: 4px;
-    box-shadow: 0 1px 5px rgba(0,0,0,.2);
 }
 
 .mention {
-    padding: 0 4px;
+    padding: 4px 4px 0;
     cursor: pointer;
 }
 
 .mention:hover, 
 .active-mention {
-    background: #aaa;
+    background: #3F4752;
+}
+
+.mention .user-avatar {
+    display: inline-block;
+    vertical-align: middle;
+}
+
+.mention .user-avatar img {
+    border-radius: 2px;
+    border: 1px solid #5E6475;
+    margin-right: 8px;
+}
+
+.mention .name {
+    font-weight: 300;
 }
 
 .mention-element {
diff --git a/frontend/src/core/comments/components/AddComment.js b/frontend/src/core/comments/components/AddComment.js
index 5cb8696ec..ec58347d6 100644
--- a/frontend/src/core/comments/components/AddComment.js
+++ b/frontend/src/core/comments/components/AddComment.js
@@ -335,7 +335,10 @@ export default function AddComments(props: Props) {
                                     className={ i === index ? 'mention active-mention' : 'mention' }
                                     onMouseDown={event => handleMouseDown(event)}
                                 >
-                                    {char}
+                                    <span className="user-avatar">
+                                        <img src="//www.gravatar.com/avatar/0345e1d1fab27d251f6bae37c7088cfc?s=88&d=https%3A%2F%2Fui-avatars.com%2Fapi%2FFrancesco%2520Lodolo%2F88%2F333941%2FFFFFFF" alt="User Avatar" width="22" height="22" />
+                                    </span>
+                                    <span className="name">{ char }</span>
                                 </div>
                             ))}
                         </div>

Of course, the user avatar must not be hardcoded. :)

I think you're already aware of it, but in case you are note: if no comments are present in the Team comments section, the mentions list gets cut off at the top of the page. We should probably show it at the bottom instead.

[abowler2]

I've added the style changes and fixed up a few things. In doing that I reduced the number of suggestions displayed in order to avoid many of the overflow issues. FWIW, they fit the best when there are only 3, but that felt a bit limiting so I set it to 5. However, with that setting it can still get cut off at the top of the team comments if the window is smaller (mostly this occurred for me when I had the dev tools open at the bottom)

I did try to set a max-height and then allow all the suggestions to populate with the ability to scroll through the names, but I was not able to get the div to scroll with the use of the arrow keys. Let me know if this is something you would like me to dig into and I can try to see if there is a solution for that.

Of course, if you find any further functional issues just let me know 😊

[mathjazz]

That works very well!

Excellent job and thanks for sticking to that massive effort from the design and research phase all the way to implementing the final details. As far as I can tell this has turned out to be by far the hardest task you've been assigned to within the Pontoon project, and you've once again proven to be an amazing coder. Well done!

I left a bunch of comments, but there isn't anything major.

[mathjazz]

Thanks for the update!

Only two nits remain.

[mathjazz]

Thanks for the update, @abowler2! r+

I'll leave the PR unmerged until @adngdb has a look at it on Monday.

Like I've already said, you've done an excellent job with this patch. Well done!

[adngdb]

I've been doing a high-level review, focusing on hooks usage and a bit on architecture and complexity. I did not run the code locally, as I assume that has been covered by Matjaz extensively. I've spotted a few issues ranging from nits to performance-impacting problems. Since I'm also quite new to hooks, it is possible that some of my comments are wrong, in which case please let me know why so I can learn! ☺️

[adngdb]

I'm pretty sure the number of people who fully understand Flow is lower than the number of fingers on the hand of an arm-less person.

[adngdb]

Missing semicolon here. 😉

[abowler2]

I've addressed @adngdb comments and made the needed changes.

[adngdb]

[mathjazz]

Anyone else having problems building this patch locally?

Step 32/35 : RUN yarn build
 ---> Running in 7960c46406e7
yarn run v1.22.4
$ react-scripts build
Creating an optimized production build...
Browserslist: caniuse-lite is outdated. Please run next command `yarn upgrade`
Failed to compile.

./node_modules/slate/dist/index.es.js
Attempted import error: 'createDraft' is not exported from 'immer'.


error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
ERROR: Service 'webapp' failed to build: The command '/bin/sh -c yarn build' returned a non-zero code: 1
make: *** [build] Error 1
Leopold:pontoon mathjazz$

It seems like upgrading slate, slate-react and html-react-parser to latest versions solves the problem:

• slate: 0.58.4
• slate-react: 0.58.4
• html-react-parser: 0.13.0

[adngdb]

It happens to me too ✋