Comment on attachment 8922771 [details] Bug 1412269 - DevTools Framework to ES6 classes https://reviewboard.mozilla.org/r/193926/#review200016 Please see my inline comments. Otherwise works for me R+ Thanks Mike! Honza ::: devtools/client/framework/components/toolbox-controller.js (Diff revision 1) > - * This component serves as a state controller for the toolbox React component. It's a > - * thin layer for translating events and state of the outside world into the React update > + constructor(props, context) { > + super(props, context); > - * cycle. This solution was used to keep the amount of code changes to a minimimum while > - * adapting the existing codebase to start using React. > - */ > -module.exports = createClass({ Please put back the comment for this component. ::: devtools/client/framework/components/toolbox-controller.js (Diff revision 1) > -module.exports = createClass({ > - displayName: "ToolboxController", > > - getInitialState() { > + this.state = { > - // See the ToolboxToolbar propTypes for documentation on each of these items in state, > - // and for the defintions of the props that are expected to be passed in. typo: defintions -> definitions ::: devtools/client/framework/components/toolbox-controller.js:43 (Diff revision 1) > + this.setDockButtonsEnabled = this.setDockButtonsEnabled.bind(this); > + this.setHostTypes = this.setHostTypes.bind(this); > + this.setCanCloseToolbox = this.setCanCloseToolbox.bind(this); > + this.setPanelDefinitions = this.setPanelDefinitions.bind(this); > + this.setToolboxButtons = this.setToolboxButtons.bind(this); > + this.setCanMinimize = this.setCanMinimize.bind(this); Do we have to bind all these methods? ::: devtools/client/framework/components/toolbox-toolbar.js (Diff revision 1) > - * This is the overall component for the toolbox toolbar. It is designed to not know how > - * the state is being managed, and attempts to be as pure as possible. The > + static get propTypes() { > + return { > - * ToolboxController component controls the changing state, and passes in everything as > - * props. > - */ > -module.exports = createClass({ Comment for the component got removed, please put it back.

Comment on attachment 8922771 [details] Bug 1412269 - DevTools Framework to ES6 classes https://reviewboard.mozilla.org/r/193926/#review200016 > Do we have to bind all these methods? This is what createClass does under the hood. When it comes to custom methods (not React built-in methods), if we want `this` inside a class to refer to the class then it has to be bound.

Pushed by mratcliffe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/23cedc247ac9 DevTools Framework to ES6 classes r=Honza