Feature/emoji on comment area #6422
Conversation
dzuqe
requested review from
danlipert,
octavioamu and
thelostone-mc
as code owners
Codecov Report
@@ Coverage Diff @@
## master #6422 +/- ##
==========================================
+ Coverage 27.22% 27.24% +0.02%
==========================================
Files 289 289
Lines 26378 26378
Branches 3909 3909
==========================================
+ Hits 7182 7188 +6
+ Misses 18929 18923 -6
Partials 267 267
Continue to review full report at Codecov.
|
| @@ -79,6 +79,10 @@ a .sup { | |||
| .action.open { | |||
| background-color: #eeeeee; | |||
| } | |||
| .emoji-container, | |||
| .comment-area{ | |||
There was a problem hiding this comment.
Do we need set the .comment-area to top:0 ?
There was a problem hiding this comment.
No we didn't, I have since removed the rule and a few others that weren't necessary within inline css.
app/assets/v2/js/activity.js
Outdated
| $(document).on('click', '.emoji_button', function(e) { | ||
| e.preventDefault(); | ||
| var commentArea = $(this).parents('.comment-area')[0]; | ||
| var emojiContainer = $(this).parents('.emoji-container')[0]; |
…ner styling; remove inline css on js; replace var with const on js;
|
I've made the requested changes. |
…ilar variable name
|
I have changed the duplicate variable name to a more precise name |
| @@ -5,26 +5,33 @@ | |||
| background-color: #ecf0f4; | |||
There was a problem hiding this comment.
In general, please do not include arbitrary style changes in PRs as it makes it difficult to do a code review. Please limit your changes to the feature or fix thats described in the PR description - if you want to do a style fix PR, you can submit it separately.
There was a problem hiding this comment.
I've removed the code style changes and left only changes I've made for the feature. I will follow this rule of thumb from now onwards.
…i-container styling; remove inline css on js; replace var with const on js;" This reverts commit 5551dbc. undo code style changes
|
I've made the requested changes. The emojis do have colors, I just had a terrible install of a Fedora Virtual Machine. Was pretty slow too, haha. I've since installed another flavor of Linux straight on the hdd. See video for color emojis https://streamable.com/cxrlp4 |
|
@hydr063n very good work, appreciate the fixes. classic linux lacking emoji support ;) |
Description
I created a feature that enables users to post emojis on their comment to an activity
Refers/Fixes
#6035
Testing
Before:
https://imgur.com/a/x4EptXu
After:
https://streamable.com/ujhflw