-
Notifications
You must be signed in to change notification settings - Fork 479
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
React: Remove uses of React.createClass in applab/* #17570
Conversation
db29926
to
c6c589b
Compare
this.makeDraggable(); | ||
}, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like these need to be bound since they access this
, is that correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lifecycle methods don't need to be bound - React calls them in the component's context already.
return this.props.isShareView || !this.props.hasDesignMode; | ||
}, | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs to be bound?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I could tell, this helper is always invoked directly from this
(i.e. this.shouldHideToggle()
) and never passed around as a callback (e.g. onClick={this.shouldHideToggle}
) so there isn't a need to bind it.
More work extracted from #16861 getting us ready to update to React 15.6+.
I'm breaking the ES6-ification of our React components up into several PRs because they're fairly significant changes and it'll be easier to introduce regressions, so I'd like them to be reviewed more carefully. This first batch is components within src/applab.