Add reactions to images #344
Conversation
|
Thanks for opening this pull request! |
app/assets/javascripts/reactions.js
Outdated
| @@ -0,0 +1,40 @@ | |||
| function changeReactionCount(value, warpableId) { | |||
| $('#reaction-count-' + warpableId).html(value); | |||
There was a problem hiding this comment.
Can we use classes here, in case we start wanting to have the star buttons shown in multiple places? Like, for example, on the toolbars that popup on the images, for example. Thanks!
There was a problem hiding this comment.
I'm unable to add the functions in a class here..The class was not getting loaded.
It said 'Reaction is undefined'
But I don't see why we need a class for this?
The functions are publicly accessible so they can be accessed from anywhere
kaustubh-nair
force-pushed
the
main
branch
2 times, most recently
from
5aa771b
to
580e74c
Compare
|
@jywarren I've made the changes requested |
kaustubh-nair
force-pushed
the
main
branch
2 times, most recently
from
25aa786
to
5aa771b
Compare
|
Hi, can you check out the errors in Travis and see if you can get them fixed? If you get stuck, please ask for help and we'll do our best! Thank you! |
|
Code Climate has analyzed commit d5c9565 and detected 128 issues on this pull request. Here's the issue category breakdown:
Note: there are 13 critical issues. View more on Code Climate. |
Fixes #126
Here is a screenshot for the star button!
@jywarren How do I test this? I am not sure how to load images into tests. Also there isn't a warpable controller. Should I make an integration test to test this?
I haven't added multiple reactions for images. It only has a star button for now, since I was not sure how to place all the reactions in the view. And I didn't know to show the reactions. I tried to make the code as dynamic as possible to include multiple reactions for the future so maybe we can make a follow up issue after this to deal with that.
Thanks!