Likes Fix (button mashing and initial negative likes) #4852

sashadev-sky · 2019-02-18T20:37:02Z

Fixes #3524, #4589

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
code is in uniquely-named feature branch and has no merge conflicts 📁
PR is descriptively titled 📑
screenshots/GIFs are attached 📎 in case of UI updation
ask @publiclab/reviewers for help, in a comment below

================

Please find notes and GIFs here:

https://github.com/sashadev-sky/PL-notes/blob/master/likes.md

===============

plotsbot · 2019-02-18T20:48:52Z

	1 Warning
⚠️	It looks like you merged from master in this pull request. Please rebase to get rid of the merge commits – you may want to rewind the master branch and rebase instead of merging in from master, which can cause problems when accepting new code!

	1 Message
📖	@sashadev-sky Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution! And don’t be discouraged if you see errors – we’re here to help.

Generated by 🚫 Danger

sashadev-sky · 2019-02-19T00:32:55Z

@jywarren From working with this code I have felt like the notification popups UI isn't compatible with the 'like' and 'unlike' functionalities. I was thinking maybe to just remove them all together as they might be replaced when a notifications API is fully integrated with the site. Please see my comments and GIFs in the link in my PR description and let me know your thoughts! Thank you

avsingh999 · 2019-02-19T13:25:01Z

@sashadev-sky can you please add Gif
thanks : )

sashadev-sky · 2019-02-19T13:37:28Z

@avsingh999 the Gif is linked in the PR request description. It wouldn't let me drag it here because the file was too large

avsingh999 · 2019-02-19T15:13:27Z

@avsingh999 the Gif is linked in the PR request description. It wouldn't let me drag it here because the file was too large

Sorry I didn't see 😄

namangupta01 · 2019-02-20T03:11:39Z

Hi @sashadev-sky, the right approach for doing this according to me is update the likes when we get the response with the number of likes. And disable the button when the request is taking place of the previous like. What do you think?

sashadev-sky · 2019-02-20T07:57:48Z

@namangupta01 I think disabling a button does not prevent event listeners from firing. So that is why I had to make sure to turn them off in between promises

namangupta01 · 2019-02-22T14:03:28Z

By disabling the button, I mean to stop progressing the event when request is going on using preventDefault or by using if else.

Gemfile

namangupta01 · 2019-02-22T14:08:38Z

app/views/like/_like.html.erb

-        }
+
+       // note useful for comments on notes but maybe useful for something else. 
+        // if(window.location.href.includes("#like")) {


It would be better to remove unused code.

I don’t think code should be deleted before you know the reason it was put there in the first place. As noted in its comment I am still investigating that and this PR is not labeled for review. If you have input on the function of it that would be useful. Thank you

You could look through the blame view for this file to see when it was added and when removed, and see if you can track it that way!

It actually doesn't do anything helpful I looked into it. But instead of removing it I will comment it out. I think I will open a new issue after this for implementing likes on comments (currently not working) and fix it up there. Is that okay with everyone? So the uncommented code will just be there for a bit.

spec/javascripts/plots2_spec.js

app/assets/javascripts/like.js

sashadev-sky · 2019-02-22T17:15:11Z

@namangupta01 this pull request is marked as “in progress” I will make the gem updates when I’m ready for merge review 👍🏻

namangupta01 · 2019-02-22T17:16:14Z

No problem, let me know if you need some help! 👍

sashadev-sky · 2019-02-22T19:47:09Z

@namangupta01 I don't quite understand the change you are suggesting. I don't think preventdefault should have any effect on the button in this scenario as it is not a form submission

also if you could help me write front end tests that would be great!! It's my first time writing them I could use some help

jywarren · 2019-02-27T19:13:08Z

Hi @sashadev-sky - by front end tests do you mean front-end javascript tests? We do some JavaScript tests here: https://github.com/publiclab/plots2/tree/master/spec/javascripts but they're not great because they are only run against static fixtures, which need to be kept up to date. There's a little about this here:

https://github.com/publiclab/plots2/blob/master/doc/TESTING.md#client-side-tests

I'm also interested in exploring system tests, in #3683, and i just opened a PR here: #4888 (though not sure if it will run).

That will actually run a full-stack version of the site in Chrome, and allows click interactions, so it really tests the whole system. I think this could be a good way forward for testing likes and we may want to move away from the teaspoon/mocha tests since they can't test on dynamic data, and require the static fixtures to be maintained. What do you think?

app/views/notes/_notes.html.erb

package.json

spec/javascripts/like_spec.js

jywarren

Small changes - likers.count - but all good otherwise! Good work here! This has long been a bit of an untidy part of our JS code. Thanks!

pull master

jywarren · 2019-03-12T20:15:37Z

Spinner idea: <i class="fa fa-spinner fa-spin"></i> - could indicate to user to stop clicking

you could also add disabled class too. jquery toggleClass()

jywarren · 2019-03-12T20:21:57Z

Folks have asked about what debounce means and how this strategy works. @sashadev-sky here's a link to a great explanation: https://css-tricks.com/debouncing-throttling-explained-examples/ -- it can really help throttle client/server interactions and make things smoother. I recently implemented it in this PR with @aashna27 's help: #4904

Example of debounce: https://github.com/publiclab/plots2/pull/4904/files#diff-128f89a0762ba84f1ad9619753645287R15

jywarren · 2019-03-12T20:23:26Z

https://css-tricks.com/the-difference-between-throttling-and-debouncing/

jywarren · 2019-03-12T20:26:54Z

$('.myButton').click(debounce(function onClick(){
   // ... do things
}, 350);

350 is the # of milliseconds. Play around with 100-350 range

jywarren · 2019-03-12T20:34:45Z

https://css-tricks.com/debouncing-throttling-explained-examples/

jywarren · 2019-03-12T20:36:00Z

immediate -- https://github.com/component/debounce#debouncefn-wait--immediate--false-

pull upstream

sashadev-sky · 2019-03-16T04:05:46Z

@jywarren likes are debounced! Please review. Not much update to show in terms of UI so the gif in the description is basically the same still as what I saw when running it on the unstable branch. I also saw from the waterfall in Chrome dev tools that the debounce is taking pauses for the requests and not every click will lead to a request when mashing the button.

jywarren · 2019-03-16T16:17:11Z

Awesome and well done here!!!

* version updates * squash commits * leave note for bridge into next likes functionaliy fix * squash commits * leave note for bridge into next likes functionaliy fix * disable noty notification for likes and comment likes * fix comment template if statement * updates * check no debounce * debounce likes * debounce

version updates

5d98cc4

sashadev-sky added the in-progress label Feb 18, 2019

sashadev-sky changed the title ~~Likes Fix~~ Likes Fix (button mashing and initial negative likes) Feb 18, 2019

sashadev-sky requested a review from avsingh999 February 18, 2019 20:47

sashadev-sky requested a review from jywarren February 19, 2019 00:00

sashadev-sky mentioned this pull request Feb 20, 2019

Weekly Community Check-In #10 - "Trespassing Boundaries" #4844

Closed

namangupta01 requested changes Feb 22, 2019

View reviewed changes

jywarren reviewed Feb 27, 2019

View reviewed changes

app/views/notes/_notes.html.erb Show resolved Hide resolved

jywarren reviewed Feb 27, 2019

View reviewed changes

package.json Outdated Show resolved Hide resolved

jywarren reviewed Feb 27, 2019

View reviewed changes

spec/javascripts/like_spec.js Show resolved Hide resolved

jywarren requested changes Feb 27, 2019

View reviewed changes

sashadev-sky force-pushed the likes-fix branch 5 times, most recently from f7b12d6 to c672c96 Compare March 6, 2019 10:21

rebase updates

960e092

sashadev-sky force-pushed the likes-fix branch from ab9014a to ccb5f6f Compare March 6, 2019 11:14

sashadev-sky added needs review and removed in-progress labels Mar 8, 2019

sashadev-sky added 3 commits March 12, 2019 14:28

Merge branch 'master' of https://github.com/publiclab/plots2

80c4e57

pull master

squash commits

1cb1c92

leave note for bridge into next likes functionaliy fix

3565617

sashadev-sky force-pushed the likes-fix branch from 3fc92ef to 3565617 Compare March 12, 2019 18:57

sashadev-sky added 10 commits March 15, 2019 17:30

Merge branch 'master' of https://github.com/publiclab/plots2

f411a0c

pull upstream

squash commits

55c60c1

leave note for bridge into next likes functionaliy fix

2b126f3

disable noty notification for likes and comment likes

25314cb

remove notynotification for likes

fb9ed60

fix comment template if statement

a3e99f2

updates

5b52a08

check no debounce

2bcf467

debounce likes

fb15783

debounce

1d8e25c

jywarren approved these changes Mar 16, 2019

View reviewed changes

jywarren merged commit 3cc86dd into publiclab:master Mar 16, 2019

This was referenced Mar 16, 2019

likes should not be negative at '/events' #4589

Closed

Comment Likes Does Not Work #5113

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Likes Fix (button mashing and initial negative likes) #4852

Likes Fix (button mashing and initial negative likes) #4852

sashadev-sky commented Feb 18, 2019 •

edited

Loading

plotsbot commented Feb 18, 2019 •

edited

Loading

sashadev-sky commented Feb 19, 2019

avsingh999 commented Feb 19, 2019

sashadev-sky commented Feb 19, 2019

avsingh999 commented Feb 19, 2019

namangupta01 commented Feb 20, 2019

sashadev-sky commented Feb 20, 2019

namangupta01 commented Feb 22, 2019

namangupta01 Feb 22, 2019

sashadev-sky Feb 22, 2019

jywarren Feb 27, 2019

sashadev-sky Mar 6, 2019

sashadev-sky commented Feb 22, 2019 •

edited

Loading

namangupta01 commented Feb 22, 2019

sashadev-sky commented Feb 22, 2019 •

edited

Loading

jywarren commented Feb 27, 2019

jywarren left a comment

jywarren commented Mar 12, 2019 •

edited

Loading

jywarren commented Mar 12, 2019

jywarren commented Mar 12, 2019

jywarren commented Mar 12, 2019 •

edited by sashadev-sky

Loading

jywarren commented Mar 12, 2019

jywarren commented Mar 12, 2019

sashadev-sky commented Mar 16, 2019

jywarren commented Mar 16, 2019

Likes Fix (button mashing and initial negative likes) #4852

Likes Fix (button mashing and initial negative likes) #4852

Conversation

sashadev-sky commented Feb 18, 2019 • edited Loading

plotsbot commented Feb 18, 2019 • edited Loading

sashadev-sky commented Feb 19, 2019

avsingh999 commented Feb 19, 2019

sashadev-sky commented Feb 19, 2019

avsingh999 commented Feb 19, 2019

namangupta01 commented Feb 20, 2019

sashadev-sky commented Feb 20, 2019

namangupta01 commented Feb 22, 2019

namangupta01 Feb 22, 2019

Choose a reason for hiding this comment

sashadev-sky Feb 22, 2019

Choose a reason for hiding this comment

jywarren Feb 27, 2019

Choose a reason for hiding this comment

sashadev-sky Mar 6, 2019

Choose a reason for hiding this comment

sashadev-sky commented Feb 22, 2019 • edited Loading

namangupta01 commented Feb 22, 2019

sashadev-sky commented Feb 22, 2019 • edited Loading

jywarren commented Feb 27, 2019

jywarren left a comment

Choose a reason for hiding this comment

jywarren commented Mar 12, 2019 • edited Loading

jywarren commented Mar 12, 2019

jywarren commented Mar 12, 2019

jywarren commented Mar 12, 2019 • edited by sashadev-sky Loading

jywarren commented Mar 12, 2019

jywarren commented Mar 12, 2019

sashadev-sky commented Mar 16, 2019

jywarren commented Mar 16, 2019

sashadev-sky commented Feb 18, 2019 •

edited

Loading

plotsbot commented Feb 18, 2019 •

edited

Loading

sashadev-sky commented Feb 22, 2019 •

edited

Loading

sashadev-sky commented Feb 22, 2019 •

edited

Loading

jywarren commented Mar 12, 2019 •

edited

Loading

jywarren commented Mar 12, 2019 •

edited by sashadev-sky

Loading