chore(lint): Turn on jsx-no-bind for system-addon and clean up existing code (#2928)

.eslintrc.js

@@ -34,6 +34,7 @@ module.exports = {
     "react/jsx-curly-spacing": [2, "never"],
     "react/jsx-equals-spacing": [2, "never"],
     "react/jsx-key": 2,
+    "react/jsx-no-bind": 2,
     "react/jsx-no-comment-textnodes": 2,
     "react/jsx-no-duplicate-props": 2,
     "react/jsx-no-target-blank": 2,

content-src/.eslintrc.js

@@ -0,0 +1,5 @@
+module.exports = {
+  "rules": {
+    "react/jsx-no-bind": 0
+  }
+};

content-test/.eslintrc.js

@@ -0,0 +1,5 @@
+module.exports = {
+  "rules": {
+    "react/jsx-no-bind": 0
+  }
+};

system-addon/content-src/components/Card/Card.jsx

@@ -17,9 +17,18 @@ class Card extends React.Component {
   constructor(props) {
     super(props);
     this.state = {showContextMenu: false, activeCard: null};
+    this.onMenuButtonClick = this.onMenuButtonClick.bind(this);
+    this.onMenuUpdate = this.onMenuUpdate.bind(this);
   }
-  toggleContextMenu(event, index) {
-    this.setState({showContextMenu: true, activeCard: index});
+  onMenuButtonClick(event) {
+    event.preventDefault();
+    this.setState({
+      activeCard: this.props.index,
+      showContextMenu: true
+    });
+  }
+  onMenuUpdate(showContextMenu) {
+    this.setState({showContextMenu});
   }
   render() {
     const {index, link, dispatch, contextMenuOptions} = this.props;
@@ -45,19 +54,16 @@ class Card extends React.Component {
         </div>
       </a>
       <button className="context-menu-button"
-        onClick={e => {
-          e.preventDefault();
-          this.toggleContextMenu(e, index);
-        }}>
+        onClick={this.onMenuButtonClick}>
         <span className="sr-only">{`Open context menu for ${link.title}`}</span>
       </button>
       <LinkMenu
-      dispatch={dispatch}
-      visible={isContextMenuOpen}
-      onUpdate={val => this.setState({showContextMenu: val})}
-      index={index}
-      site={link}
-      options={link.context_menu_options || contextMenuOptions} />
+        dispatch={dispatch}
+        index={index}
+        onUpdate={this.onMenuUpdate}
+        options={link.context_menu_options || contextMenuOptions}
+        site={link}
+        visible={isContextMenuOpen} />
    </li>);
   }
 }

system-addon/content-src/components/ContextMenu/ContextMenu.jsx

@@ -21,46 +21,60 @@ class ContextMenu extends React.Component {
       window.removeEventListener("click", this.hideContext);
     }
   }
-  componentDidUnmount() {
+  componentWillUnmount() {
     window.removeEventListener("click", this.hideContext);
   }
-  onKeyDown(event, option) {
+  render() {
+    return (<span hidden={!this.props.visible} className="context-menu">
+      <ul role="menu" className="context-menu-list">
+        {this.props.options.map((option, i) => (option.type === "separator" ?
+          (<li key={i} className="separator" />) :
+          (<ContextMenuItem key={i} option={option} hideContext={this.hideContext} />)
+        ))}
+      </ul>
+    </span>);
+  }
+}
+
+class ContextMenuItem extends React.Component {
+  constructor(props) {
+    super(props);
+    this.onClick = this.onClick.bind(this);
+    this.onKeyDown = this.onKeyDown.bind(this);
+  }
+  onClick() {
+    this.props.hideContext();
+    this.props.option.onClick();
+  }
+  onKeyDown(event) {
+    const {option} = this.props;
     switch (event.key) {
       case "Tab":
         // tab goes down in context menu, shift + tab goes up in context menu
         // if we're on the last item, one more tab will close the context menu
         // similarly, if we're on the first item, one more shift + tab will close it
         if ((event.shiftKey && option.first) || (!event.shiftKey && option.last)) {
-          this.hideContext();
+          this.props.hideContext();
         }
         break;
       case "Enter":
-        this.hideContext();
+        this.props.hideContext();
         option.onClick();
         break;
     }
   }
   render() {
-    return (<span hidden={!this.props.visible} className="context-menu">
-      <ul role="menu" className="context-menu-list">
-      {this.props.options.map((option, i) => {
-        if (option.type === "separator") {
-          return (<li key={i} className="separator" />);
-        }
-        return (<li role="menuitem" className="context-menu-item" key={i}>
-          <a tabIndex="0"
-            onKeyDown={e => this.onKeyDown(e, option)}
-            onClick={() => {
-              this.hideContext();
-              option.onClick();
-            }}>
+    const {option} = this.props;
+    return (
+      <li role="menuitem" className="context-menu-item">
+        <a onClick={this.onClick} onKeyDown={this.onKeyDown} tabIndex="0">
           {option.icon && <span className={`icon icon-spacer icon-${option.icon}`} />}
           {option.label}
-          </a></li>);
-      })}
-      </ul>
-    </span>);
+        </a>
+      </li>);
   }
 }
 
 module.exports = ContextMenu;
+module.exports.ContextMenu = ContextMenu;
+module.exports.ContextMenuItem = ContextMenuItem;

system-addon/content-src/components/TopSites/TopSites.jsx

@@ -12,17 +12,30 @@ class TopSite extends React.Component {
   constructor(props) {
     super(props);
     this.state = {showContextMenu: false, activeTile: null};
+    this.onLinkClick = this.onLinkClick.bind(this);
+    this.onMenuButtonClick = this.onMenuButtonClick.bind(this);
+    this.onMenuUpdate = this.onMenuUpdate.bind(this);
   }
   toggleContextMenu(event, index) {
-    this.setState({showContextMenu: true, activeTile: index});
+    this.setState({
+      activeTile: index,
+      showContextMenu: true
+    });
   }
-  trackClick() {
+  onLinkClick() {
     this.props.dispatch(ac.UserEvent({
       event: "CLICK",
       source: TOP_SITES_SOURCE,
       action_position: this.props.index
     }));
   }
+  onMenuButtonClick(event) {
+    event.preventDefault();
+    this.toggleContextMenu(event, this.props.index);
+  }
+  onMenuUpdate(showContextMenu) {
+    this.setState({showContextMenu});
+  }
   render() {
     const {link, index, dispatch} = this.props;
     const isContextMenuOpen = this.state.showContextMenu && this.state.activeTile === index;
@@ -31,7 +44,7 @@ class TopSite extends React.Component {
     const topSiteOuterClassName = `top-site-outer${isContextMenuOpen ? " active" : ""}`;
     const style = {backgroundImage: (link.screenshot ? `url(${link.screenshot})` : "none")};
     return (<li className={topSiteOuterClassName} key={link.guid || link.url}>
-        <a onClick={() => this.trackClick()} href={link.url}>
+        <a href={link.url} onClick={this.onLinkClick}>
           <div className="tile" aria-hidden={true}>
               <span className="letter-fallback">{title[0]}</span>
               <div className={screenshotClassName} style={style} />
@@ -41,21 +54,17 @@ class TopSite extends React.Component {
             <span>{title}</span>
           </div>
         </a>
-        <button className="context-menu-button"
-          onClick={e => {
-            e.preventDefault();
-            this.toggleContextMenu(e, index);
-          }}>
+        <button className="context-menu-button" onClick={this.onMenuButtonClick}>
           <span className="sr-only">{`Open context menu for ${title}`}</span>
         </button>
         <LinkMenu
           dispatch={dispatch}
-          visible={isContextMenuOpen}
-          onUpdate={val => this.setState({showContextMenu: val})}
-          site={link}
           index={index}
+          onUpdate={this.onMenuUpdate}
+          options={TOP_SITES_CONTEXT_MENU_OPTIONS}
+          site={link}
           source={TOP_SITES_SOURCE}
-          options={TOP_SITES_CONTEXT_MENU_OPTIONS} />
+          visible={isContextMenuOpen} />
     </li>);
   }
 }

system-addon/test/.eslintrc.js

@@ -8,5 +8,8 @@ module.exports = {
     "assert": true,
     "sinon": true,
     "chai": true
+  },
+  "rules": {
+    "react/jsx-no-bind": 0
   }
 };

system-addon/test/unit/content-src/components/ContextMenu.test.jsx

@@ -1,6 +1,6 @@
 const React = require("react");
 const {shallow, mount} = require("enzyme");
-const ContextMenu = require("content-src/components/ContextMenu/ContextMenu");
+const {ContextMenu, ContextMenuItem} = require("content-src/components/ContextMenu/ContextMenu");
 const DEFAULT_PROPS = {
   onUpdate: () => {},
   visible: false,
@@ -30,16 +30,16 @@ describe("<ContextMenu>", () => {
   it("should add a link for all types that are not separators", () => {
     const options = [{label: "item1"}, {type: "separator"}];
     const wrapper = shallow(<ContextMenu {...DEFAULT_PROPS} options={options} />);
-    assert.lengthOf(wrapper.find(".context-menu-item"), 1);
+    assert.lengthOf(wrapper.find(ContextMenuItem), 1);
   });
   it("should add an icon to items that need icons", () => {
     const options = [{label: "item1", icon: "icon1"}, {type: "separator"}];
-    const wrapper = shallow(<ContextMenu {...DEFAULT_PROPS} options={options} />);
+    const wrapper = mount(<ContextMenu {...DEFAULT_PROPS} options={options} />);
     assert.lengthOf(wrapper.find(".icon-icon1"), 1);
   });
   it("should be tabbable", () => {
     const options = [{label: "item1", icon: "icon1"}, {type: "separator"}];
-    const wrapper = shallow(<ContextMenu {...DEFAULT_PROPS} options={options} />);
+    const wrapper = mount(<ContextMenu {...DEFAULT_PROPS} options={options} />);
     assert.equal(wrapper.find(".context-menu-item").props().role, "menuitem");
   });
   it("should call onUpdate with false when an option is clicked", () => {