6
\$\begingroup\$

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 (
 <div className="post-container">
 <Card>
 <CardText>
 <p className="timestamp">
 {moment(createdAt).fromNow()}
 </p>
 {content}
 <div style={{ textAlign: "right" }}>
 <FlatButton 
 label="Upvote" 
 primary={true} 
 disabled={this.state.disabledUpvote}
 onClick={this.handleUpvoteClicked.bind(this)} />
 <FlatButton 
 label="Downvote" 
 secondary={true}
 disabled={this.state.disabledDownvote}
 onClick={this.handleDownvoteClicked.bind(this)} />
 </div>
 </CardText>
 </Card>
 </div>
 );
 }
}
export default Post;

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

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jul 1, 2016 at 23:31
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

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 (
 <div className="post-container">
 <Card>
 <CardText>
 <p className="timestamp">
 {moment(createdAt).fromNow()}
 </p>
 {content}
 <div style={{ textAlign: "right" }}>
 <FlatButton 
 label="Upvote" 
 primary={true} 
 disabled={this.state.disabledUpvote}
 onClick={this.handleUpvoteClicked} />
 <FlatButton 
 label="Downvote" 
 secondary={true}
 disabled={this.state.disabledDownvote}
 onClick={this.handleDownvoteClicked} />
 </div>
 </CardText>
 </Card>
 </div>
 );
 }
}
export default Post;

answered Jul 2, 2016 at 2:17
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.