HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Application for allowing a user to "follow" another user

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
allowingapplicationuserfollowforanother

Problem

I have taken a stab at creating my first jQuery script from scratch. I use this to post an update to a Django application that allows a user to "follow" another user. I am sure that there are better ways to do this and I appreciate any help that anyone can give.


jQuery(function() {
    jQuery('.follow').click(function(event) {
        event.preventDefault();
        var url = jQuery(this).attr("href");
        var followtype = jQuery(this).text();
        var user = jQuery(".follow").attr("href").match('/relationships/[a-z]+/([a-z]+)/following/')[1];
        var postdata = {
            'csrfmiddlewaretoken': '{{ csrf_token }}'
        }
        jQuery.post(url, postdata, function(json){
            if(json.result == '1') {
                if(followtype == 'Follow') {
                    jQuery('.follow').text('Unfollow');
                    jQuery('.follow').attr("href", '/relationships/remove/'+user+'/following/');
                } else {
                    jQuery('.follow').text('Follow');
                    jQuery('.follow').attr("href", '/relationships/add/'+user+'/following/');
                }
            }
        })
    });
});

Solution

This is how I'd do it. There may be flaws because you haven't provided your HTML.

I assume that within the click handler, you only want to interact with the current element being clicked. The reason I say that is because you had $('.follow') a number of times in there, which would affect all .follow elements.

//     v--- makes the shortcut available inside the .ready() callback
jQuery(function($) {
    $('.follow').click(function(event) {
        event.preventDefault();

            // assuming it's a link, no need for jQuery here
        var url = this.href;

            // Looking at the code in your callback, I assume you only have text content
        var followtype = this.innerHTML;

            // I assume you actually want just the current "follow", not all
        var user = url.match('/relationships/[a-z]+/([a-z]+)/following/');

            // Make certain there was a match before accessing the Array.
            // If there was no match, just return the function
        if( user )
            user = user[1];
        else
            return;

        var postdata = {
            'csrfmiddlewaretoken': '{{ csrf_token }}'
        };
            // Inside the $.post callback, are we still only working with the current
            //   "follow" clicked? If so, cache it here, and use it in the callback
        var this_follow = this;

        $.post(url, postdata, function(json){
            if(json.result == '1') {
                if(followtype == 'Follow') {
                     this_follow.innerHTML = 'Unfollow';
                     this_follow.pathname = '/relationships/remove/'+user+'/following/';
                } else {
                    this_follow.innerHTML = 'Follow';
                    this_follow.pathname = '/relationships/add/'+user+'/following/';
                }
            }
        })
    });
});


As you can see, I tend to eliminate jQuery when convenient.

I should note that using .innerHTML in the way I have is cheating a bit. It's an HTML parser really, but it's convenient for changing small chunks of text quickly.

To do it properly, you'd select the child text node of the element, and change its nodeValue.

this_follow.firstChild.nodeValue = 'Unfollow';

Code Snippets

//     v--- makes the shortcut available inside the .ready() callback
jQuery(function($) {
    $('.follow').click(function(event) {
        event.preventDefault();

            // assuming it's a link, no need for jQuery here
        var url = this.href;

            // Looking at the code in your callback, I assume you only have text content
        var followtype = this.innerHTML;

            // I assume you actually want just the current "follow", not all
        var user = url.match('/relationships/[a-z]+/([a-z]+)/following/');

            // Make certain there was a match before accessing the Array.
            // If there was no match, just return the function
        if( user )
            user = user[1];
        else
            return;

        var postdata = {
            'csrfmiddlewaretoken': '{{ csrf_token }}'
        };
            // Inside the $.post callback, are we still only working with the current
            //   "follow" clicked? If so, cache it here, and use it in the callback
        var this_follow = this;

        $.post(url, postdata, function(json){
            if(json.result == '1') {
                if(followtype == 'Follow') {
                     this_follow.innerHTML = 'Unfollow';
                     this_follow.pathname = '/relationships/remove/'+user+'/following/';
                } else {
                    this_follow.innerHTML = 'Follow';
                    this_follow.pathname = '/relationships/add/'+user+'/following/';
                }
            }
        })
    });
});
this_follow.firstChild.nodeValue = 'Unfollow';

Context

StackExchange Code Review Q#8621, answer score: 2

Revisions (0)

No revisions yet.