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.
1 Answer 1
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;