537 react to comments

[squigglybob]

resolves #537

[squigglybob]

Hi @corsacca @ahillbilly I think I'm done with this one if you could check it out.

Let me know if any of the styling needs adjusting.

Also @corsacca I wasn't sure what the best permissions to check for on the API of this, so have just used the self::can_update(post, id) function for now let me know if it needs anything else

[micahmills]

Hey @squigglybob this is great! It works exactly as I would expect and even works in a right to left language!

One question I have is what is the advantage of using the images of emojis instead of the HTML entities directly? I would think we could save the HTTP requests for the images and the user would get the platform specific emoji designs if we used the HTML entities, but I might be missing something that requires the images.

I did notice one edge case styling thing. If you have a lot of emojis selected and the screen size is small enough the comment controls flow to multiple lines making the emoji pill buttons expend to multiple lines.

[squigglybob]

I'll have a look at the small screen size issue, and see if I can get the edit/delete controls to go to the next line on smaller screens.

I'll also investigate just using the emoji utf8 charachters instead of paths, I guess one benifit is that any emoji image can be used, and it's not limited to what you can find in the utf8 emoji list (although it is pretty extensive :)

[squigglybob]

Looks better on an iPhone 5 size now (320px!!!)

[corsacca]

Thanks for this great work @squigglybob and the review @micahmills
I'm getting this error when adding a comment.
The submit comment button stays disabled, keeping me from adding another comment.

[micahmills]

Thanks for this great work @squigglybob and the review @micahmills
I'm getting this error when adding a comment.
The submit comment button stays disabled, keeping me from adding another comment.

I am seeing the same thing with an console error of
TypeError: Cannot convert undefined or null to object at Function.entries (<anonymous>) at eval (lodash.templateSources[0]:39) at r (lodash.min.js?ver=4.17.19:9) at Function.hf (lodash.min.js?ver=4.17.19:83) at eval (lodash.templateSources[0]:18) at comments.js?ver=1618482543:371 at Array.forEach (<anonymous>) at display_activity_comment (comments.js?ver=1618482543:345) at Object.<anonymous> (comments.js?ver=1618482543:30) at e (jquery.min.js?ver=3.5.1:2)

[squigglybob]

Ok, I've added in a default object for the comment reactions so hopefully that fixes the error you both found in the previous two comments. Let me know if that does it...

It's strange that I didn't get the same error on my machine Ubuntu 20.04, Chrome 88.0.4324.150

[micahmills]

This seems to have fixed the issue, and the addition of the UTF-8 emojis is great.

This looks good to go. Great work!

[corsacca]

Hey. Thanks bearing with me on one more request.
Can you make sure all the strings are being escaped?
One example is
<img class="emoji" alt="${reaction.name}" src="${reaction.path}">
reaction.name can be a translated value, so it could contain js.
As much as possible we try to run all variables through window.lodash.escape() to deal with any xxs attacks, even ones that likely aren't (like reaction.path).

( Using <%- in the commentTemplate i think does escaping )

[squigglybob]

@corsacca I've had a look through the comments.js file and think that the only one that needed escaping was the one mentioned in your change request.

The only other ones are in the lodash template and being escaped by the <%- tags

[corsacca]

Thank you @squigglybob !
I'm looking forward to using this one :)