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

Upvote/downvote toggle

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

Problem

I'm sure there is a better way to implement this feature. I am making an app that allows a user "upvote" or "downvote" a post. Right now, the code works and disables the upvote button if pressed and vice versa with the downvote button (the idea is that a user can only upvote or downvote once).

post.js

import React, { Component } from 'react';
import { Card, CardText, CardHeader } from 'material-ui/Card';
import FlatButton from 'material-ui/FlatButton';

class Post extends Component {

  constructor(props) {
    super(props);

    this.state = {
      disabledUpvote: false,
      disabledDownvote: false
    }

  }

  handleUpvoteClicked() {
    const downvoteDisabled = this.state.disabledDownvote;

    if (downvoteDisabled == false) {
      this.setState({ 
        disabledUpvote: true,
        disabledDownvote: false
      });
    } else {
      this.setState({
        disabledUpvote: true,
        disabledDownvote: false
      });
    }
  } 

  handleDownvoteClicked() {
    const upvoteDisabled = this.state.disabledUpvote;

    if (upvoteDisabled == false) {
      this.setState({ 
        disabledDownvote: true,
        disabledUpvote: false
      });
    } else {
      this.setState({
        disabledDownvote: true,
        disabledUpvote: false
      });
    }
  } 

  render() {
    const { content, createdAt, votes } = this.props.post;

    return (
      
        
          
            
              {moment(createdAt).fromNow()}
            
            {content}
            
              
              
            
          
        
      
    );
  }
}

export default Post;


I am specifically looking for help with the handleUpvoteClicked() and handleDownvoteClicked() functions as they both appear to be somewhat redundant.

Solution

One thing I noticed is that you're passing this.handleUpvoteClicked.bind(this) as a prop to your FlatButton components. The problem is that the call to Function.prototype.bind creates a new function instance for both click handlers every time your Post renders, meaning your FlatButton components see a different value (function reference) being passed in as a prop, and they'll also re-render. Probably not a big deal for your simple setup, but it's something to keep in mind for later, because it can drastically affect performance for bigger apps.

The solution is to bind your methods in the constructor. That way .bind(this) is only called once during construction (good because that method is a little slow), and your components will see a consistent function reference being passed in as a prop and will know they don't have to re-render.

I also tweaked your conditionals inside the click handlers a bit, so that they only run if the button isn't disabled.

Here's what I came up with:



import React, { Component } from 'react';
import { Card, CardText, CardHeader } from 'material-ui/Card';
import FlatButton from 'material-ui/FlatButton';

class Post extends Component {

constructor(props) {
super(props);

this.state = {
disabledUpvote: false,
disabledDownvote: false
}

this.handleUpvoteClicked = this.handleUpvoteClicked.bind(this);
this.handleDownvoteClicked = this.handleDownvoteClicked.bind(this);
}

handleUpvoteClicked() {
if (!this.state.disabledUpvote) {
this.setState({
disabledUpvote: true,
disabledDownvote: false
});
}
}

handleDownvoteClicked() {
if (!this.state.disabledDownvote) {
this.setState({
disabledUpvote: false,
disabledDownvote: true
});
}
}

render() {
const { content, createdAt, votes } = this.props.post;

return (




{moment(createdAt).fromNow()}

{content}







);
}
}

export default Post;

Context

StackExchange Code Review Q#133669, answer score: 5

Revisions (0)

No revisions yet.