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

Follow button like on Twitter

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

Problem

I want to create an like button like on Twitter. I used Twitter bootstrap and this code:

$(document).ready(function(){
  $('.btn-follow').on('mouseover', function(e){
    if ($(this).hasClass('following')){
      $(this).removeClass('btn-primary').addClass('btn-danger').text('UnFollow');
    }
  })
  $('.btn-follow').on('mouseleave',function(e){
    if ($(this).hasClass('following')){
      $(this).text('Following').removeClass('btn-danger').addClass('btn-primary');
    }
  })
  $('.btn-follow').on('click',function(e){
    $(this).toggleClass('following follow')
    if ($(this).hasClass('follow')){
      alert('unfollow')
      $(this).text('Follow').removeClass('btn-danger')
    }else{
      alert('follow')
      $(this).text('Following')
    }
  })
});


And in HTML I have this:

Following
    
    
  Follow


But I think it's very dirty code and has some bugs. How can I summarize this code?

This code has a bug. When I click on click of the follow button, the text is changed to "Following", but the color of the button is not changed while the mouse leave from the button area. How I can fix it?

Solution

I reworked the code a bit. Check out the comments for explanation.

$(document).ready(function() {
  //Don't copy and paste the same string.
  //It is also helpful when the CSS or text changes. There is just one part
  //of the code to look at.
  var followBtn = $('.btn-follow'),
      followCls = 'following',
      dangerCls = 'btn-danger',
      primaryCls = 'btn-primary',
      unfollowText = 'UnFollow',
      followText = 'Follow'

  //No need to keep making a jQuery "query" to get a the button every time.
  //I am not sure about the internals of jQuery, but I assume it is reading the 
  //DOM on every $('.btn-follow') call.

  //Don't pass argument to method if it is not being used.
  followBtn.on('mouseover', function(){
    //same thing inside the handler no need for $(this).
    if (followBtn.hasClass(followCls)){ 
      followBtn.removeClass(primaryCls).addClass(dangerCls).text(unfollowText);
    }
  });

  followBtn.on('mouseleave',function(){
    if (followBtn.hasClass(followCls)){
      //Keep the same order as before, just be consistent.
      followBtn.removeClass(dangerCls).addClass(primaryCls).text(followText);
    }
  });
  //The rest
});


If you post the CSS I can help you with that. I suspect that a few of the removeClass calls can be removed in favor of more specific selectors.

Code Snippets

$(document).ready(function() {
  //Don't copy and paste the same string.
  //It is also helpful when the CSS or text changes. There is just one part
  //of the code to look at.
  var followBtn = $('.btn-follow'),
      followCls = 'following',
      dangerCls = 'btn-danger',
      primaryCls = 'btn-primary',
      unfollowText = 'UnFollow',
      followText = 'Follow'

  //No need to keep making a jQuery "query" to get a the button every time.
  //I am not sure about the internals of jQuery, but I assume it is reading the 
  //DOM on every $('.btn-follow') call.


  //Don't pass argument to method if it is not being used.
  followBtn.on('mouseover', function(){
    //same thing inside the handler no need for $(this).
    if (followBtn.hasClass(followCls)){ 
      followBtn.removeClass(primaryCls).addClass(dangerCls).text(unfollowText);
    }
  });

  followBtn.on('mouseleave',function(){
    if (followBtn.hasClass(followCls)){
      //Keep the same order as before, just be consistent.
      followBtn.removeClass(dangerCls).addClass(primaryCls).text(followText);
    }
  });
  //The rest
});

Context

StackExchange Code Review Q#29486, answer score: 3

Revisions (0)

No revisions yet.