From be9b640130e583c929970ca3719e72b2929e0299 Mon Sep 17 00:00:00 2001
From: Filip Barl <filip.barl@gmail.com>
Date: Mon, 12 Dec 2016 11:36:43 +0100
Subject: [PATCH] Addressed @davkal's and @jpellizzari's comments

---
 .../scripts/components/grid-mode-selector.js  |  8 ++---
 .../app/scripts/components/matched-results.js | 29 ++++++++----------
 .../node-details/node-details-labels.js       | 16 +++++-----
 client/app/scripts/components/nodes.js        | 30 +++++++++----------
 client/app/scripts/components/plugins.js      |  7 ++---
 client/package.json                           |  1 -
 6 files changed, 41 insertions(+), 50 deletions(-)

diff --git a/client/app/scripts/components/grid-mode-selector.js b/client/app/scripts/components/grid-mode-selector.js
index c81a76ae1..e8d843674 100644
--- a/client/app/scripts/components/grid-mode-selector.js
+++ b/client/app/scripts/components/grid-mode-selector.js
@@ -5,7 +5,7 @@ import classNames from 'classnames';
 import { toggleGridMode } from '../actions/app-actions';
 
 
-function renderItem(icons, label, isSelected, onClick) {
+const Item = (icons, label, isSelected, onClick) => {
   const className = classNames('grid-mode-selector-action', {
     'grid-mode-selector-action-selected': isSelected
   });
@@ -17,7 +17,7 @@ function renderItem(icons, label, isSelected, onClick) {
       <span>{label}</span>
     </div>
   );
-}
+};
 
 class GridModeSelector extends React.Component {
 
@@ -42,8 +42,8 @@ class GridModeSelector extends React.Component {
     return (
       <div className="grid-mode-selector">
         <div className="grid-mode-selector-wrapper">
-          {renderItem('fa fa-share-alt', 'Graph', !gridMode, this.disableGridMode)}
-          {renderItem('fa fa-table', 'Table', gridMode, this.enableGridMode)}
+          {Item('fa fa-share-alt', 'Graph', !gridMode, this.disableGridMode)}
+          {Item('fa fa-table', 'Table', gridMode, this.enableGridMode)}
         </div>
       </div>
     );
diff --git a/client/app/scripts/components/matched-results.js b/client/app/scripts/components/matched-results.js
index 9fd0d53c3..ef1a170f3 100644
--- a/client/app/scripts/components/matched-results.js
+++ b/client/app/scripts/components/matched-results.js
@@ -7,24 +7,21 @@ const SHOW_ROW_COUNT = 2;
 const MAX_MATCH_LENGTH = 24;
 
 
-function renderMatch(match) {
-  return (
-    <div className="matched-results-match" key={match.label}>
-      <div className="matched-results-match-wrapper">
-        <span className="matched-results-match-label">
-          {match.label}:
-        </span>
-        <MatchedText
-          text={match.text} match={match}
-          maxLength={MAX_MATCH_LENGTH}
-          truncate={match.truncate} />
-      </div>
+const Match = match => (
+  <div className="matched-results-match" key={match.label}>
+    <div className="matched-results-match-wrapper">
+      <span className="matched-results-match-label">
+        {match.label}:
+      </span>
+      <MatchedText
+        text={match.text} match={match}
+        maxLength={MAX_MATCH_LENGTH}
+        truncate={match.truncate} />
     </div>
-  );
-}
+  </div>
+);
 
 class MatchedResults extends React.Component {
-
   render() {
     const { matches, style } = this.props;
 
@@ -44,7 +41,7 @@ class MatchedResults extends React.Component {
 
     return (
       <div className="matched-results" style={style}>
-        {matches.keySeq().take(SHOW_ROW_COUNT).map(fieldId => renderMatch(matches.get(fieldId)))}
+        {matches.keySeq().take(SHOW_ROW_COUNT).map(fieldId => Match(matches.get(fieldId)))}
         {moreFieldMatches && <div className="matched-results-more" title={moreFieldMatchesTitle}>
           {`${moreFieldMatches.size} more matches`}
         </div>}
diff --git a/client/app/scripts/components/node-details/node-details-labels.js b/client/app/scripts/components/node-details/node-details-labels.js
index a8ae1aac7..6823f1330 100644
--- a/client/app/scripts/components/node-details/node-details-labels.js
+++ b/client/app/scripts/components/node-details/node-details-labels.js
@@ -7,14 +7,12 @@ import NodeDetailsControlButton from './node-details-control-button';
 import ShowMore from '../show-more';
 
 
-function renderControls(controls) {
-  return (
-    <div className="node-details-labels-controls">
-      {sortBy(controls, 'rank').map(control => <NodeDetailsControlButton
-        nodeId={control.nodeId} control={control} key={control.id} />)}
-    </div>
-  );
-}
+const Controls = controls => (
+  <div className="node-details-labels-controls">
+    {sortBy(controls, 'rank').map(control => <NodeDetailsControlButton
+      nodeId={control.nodeId} control={control} key={control.id} />)}
+  </div>
+);
 
 export default class NodeDetailsLabels extends React.Component {
 
@@ -49,7 +47,7 @@ export default class NodeDetailsLabels extends React.Component {
 
     return (
       <div className="node-details-labels">
-        {controls && renderControls(controls)}
+        {controls && Controls(controls)}
         {rows.map(field => (
           <div className="node-details-labels-field" key={field.id}>
             <div
diff --git a/client/app/scripts/components/nodes.js b/client/app/scripts/components/nodes.js
index 34ac83e08..380d168cc 100644
--- a/client/app/scripts/components/nodes.js
+++ b/client/app/scripts/components/nodes.js
@@ -13,21 +13,19 @@ const navbarHeight = 194;
 const marginTop = 0;
 
 
-function renderEmptyTopologyError(show) {
-  return (
-    <NodesError faIconClass="fa-circle-thin" hidden={!show}>
-      <div className="heading">Nothing to show. This can have any of these reasons:</div>
-      <ul>
-        <li>We haven&apos;t received any reports from probes recently.
-         Are the probes properly configured?</li>
-        <li>There are nodes, but they&apos;re currently hidden. Check the view options
-         in the bottom-left if they allow for showing hidden nodes.</li>
-        <li>Containers view only: you&apos;re not running Docker,
-         or you don&apos;t have any containers.</li>
-      </ul>
-    </NodesError>
-  );
-}
+const EmptyTopologyError = show => (
+  <NodesError faIconClass="fa-circle-thin" hidden={!show}>
+    <div className="heading">Nothing to show. This can have any of these reasons:</div>
+    <ul>
+      <li>We haven&apos;t received any reports from probes recently.
+       Are the probes properly configured?</li>
+      <li>There are nodes, but they&apos;re currently hidden. Check the view options
+       in the bottom-left if they allow for showing hidden nodes.</li>
+      <li>Containers view only: you&apos;re not running Docker,
+       or you don&apos;t have any containers.</li>
+    </ul>
+  </NodesError>
+);
 
 class Nodes extends React.Component {
   constructor(props, context) {
@@ -60,7 +58,7 @@ class Nodes extends React.Component {
             itemType={getNodeType(currentTopology, topologies)}
             show={topologiesLoaded && !nodesLoaded} />
         </DelayedShow>
-        {renderEmptyTopologyError(topologiesLoaded && nodesLoaded && topologyEmpty)}
+        {EmptyTopologyError(topologiesLoaded && nodesLoaded && topologyEmpty)}
 
         {gridMode ?
           <NodesGrid {...this.state} nodeSize="24" margins={CANVAS_MARGINS} /> :
diff --git a/client/app/scripts/components/plugins.js b/client/app/scripts/components/plugins.js
index 93f52567f..61cdea746 100644
--- a/client/app/scripts/components/plugins.js
+++ b/client/app/scripts/components/plugins.js
@@ -4,7 +4,7 @@ import classNames from 'classnames';
 import ReactTooltip from 'react-tooltip';
 
 
-function renderPlugin({id, label, description, status}) {
+const Plugin = ({id, label, description, status}) => {
   const error = status !== 'ok';
   const className = classNames({ error });
   const title = `Plugin description: ${description}<br />Status: ${status}`;
@@ -19,7 +19,7 @@ function renderPlugin({id, label, description, status}) {
       <ReactTooltip class="tooltip" effect="solid" offset={{right: 7}} />
     </span>
   );
-}
+};
 
 class Plugins extends React.Component {
   render() {
@@ -29,8 +29,7 @@ class Plugins extends React.Component {
         <span className="plugins-label">
           Plugins:
         </span>
-        {hasPlugins && this.props.plugins.toIndexedSeq()
-          .map(plugin => renderPlugin(plugin.toJS()))}
+        {hasPlugins && this.props.plugins.toIndexedSeq().map(plugin => Plugin(plugin.toJS()))}
         {!hasPlugins && <span className="plugins-empty">n/a</span>}
       </div>
     );
diff --git a/client/package.json b/client/package.json
index c943e9c24..dbe360ea4 100644
--- a/client/package.json
+++ b/client/package.json
@@ -57,7 +57,6 @@
     "eslint-config-airbnb": "13.0.0",
     "eslint-loader": "1.6.1",
     "eslint-plugin-import": "2.2.0",
-    "eslint-plugin-jasmine": "2.2.0",
     "eslint-plugin-jsx-a11y": "2.2.3",
     "eslint-plugin-react": "6.8.0",
     "extract-text-webpack-plugin": "1.0.1",