Incrementing up/down vote counts after voting #56

Closed
wants to merge 7 commits into
from

4 participants

@stevenleeg

Adds some code that increments the up/down vote counts below the title upon a successful vote.

@rwaldron

This adds two additional calls for the same $("#" + news_id) context element to both logic paths. The context selection and .upvote / .downvote filters should be cached as references

@rwaldron rwaldron and 1 other commented on an outdated diff Oct 23, 2011
public/js/app.js
@@ -120,6 +120,7 @@ $(document).ready(function() {
success: function(reply) {
var r = jQuery.parseJSON(reply);
if (r.status == "ok") {
+ $('#'+news_id+' .upvotes').text(parseInt($('#'+news_id+' .upvotes').text()) + 1)
@rwaldron
rwaldron added a line comment Oct 23, 2011

These parseInt calls are missing the radix argument.

@antirez
Owner
antirez added a line comment Oct 25, 2011

Doesn't this only counts if your integers have additional zeroes on the left?

@rwaldron
rwaldron added a line comment Oct 25, 2011

It's just responsible programming to include the argument

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rwaldron rwaldron commented on an outdated diff Oct 23, 2011
public/js/app.js
@@ -145,6 +146,7 @@ $(document).ready(function() {
success: function(reply) {
var r = jQuery.parseJSON(reply);
if (r.status == "ok") {
+ $('#'+news_id+' .downvotes').text(parseInt($('#'+news_id+' .downvotes').text()) + 1)
@rwaldron
rwaldron added a line comment Oct 23, 2011

Same as above... these parseInt calls are missing the radix argument.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stevenleeg

I believe I fixed the problems. Let me know if anything else seems off.

@rwaldron rwaldron commented on an outdated diff Oct 23, 2011
public/js/app.js
@@ -145,6 +147,8 @@ $(document).ready(function() {
success: function(reply) {
var r = jQuery.parseJSON(reply);
if (r.status == "ok") {
+ var vote_count = $('#'+news_id+' .downvotes')
@rwaldron
rwaldron added a line comment Oct 23, 2011

The rest of the code is using double quotes, but you've used single quotes?

Also, you're missing semi-colons here and below.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Steve Gattuso added some commits Oct 23, 2011
@JonnieCache

+1 on this. Always thought this was needed on HN.

@antirez
Owner

probably this can't be merged now that I merged the changes to make the layout using only HTML5 standard tags?

@stevenleeg
@antirez
Owner

not sure, after merging the latest pull requests not even normal voting works ;) Fixing this problem and trying if there are conflicts. Well there are not conflicts but since the markup changed not sure if this is going to wok.

@stevenleeg
@antirez
Owner

Thanks maybe I'll be able to test it as well today. Btw voting fixed with last commit.

@fcambus fcambus added a commit that closed this pull request Jun 23, 2013
@fcambus fcambus Incrementing up/down vote counts after voting news (Close #56)
Pull request #56 could not be merged directly, as the markup and the
JavaScript code changed. Thanks @stevenleeg.
b65b27b
@fcambus fcambus closed this in b65b27b Jun 23, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment