patternjavascriptMinor
Upvote/downvote toggle
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
I am specifically looking for help with the
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
The solution is to bind your methods in the constructor. That way
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:
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.