Implement PR review suggestions

ui/package.json

@@ -25,10 +25,8 @@
     "tslint:fix": "tslint --fix -c ./tslint.json '{src,test}/**/*.ts?(x)'",
     "prettier":
       "prettier --single-quote --trailing-comma es5 --bracket-spacing false --semi false --write \"{src,spec}/**/*.js\"",
-    "lint:fix":
-      "yarn run prettier && yarn run eslint:fix && yarn run tslint:fix",
-    "eslint-check":
-      "eslint --print-config .eslintrc | eslint-config-prettier-check"
+    "lint:fix": "yarn run prettier && yarn run eslint:fix && yarn run tslint:fix",
+    "eslint-check": "eslint --print-config .eslintrc | eslint-config-prettier-check"
   },
   "author": "",
   "eslintConfig": {

ui/src/dashboards/components/GraphOptionsCustomizableField.tsx

@@ -1,4 +1,4 @@
-import React, {PureComponent} from 'react'
+import React, {PureComponent, ChangeEvent} from 'react'
 import {findDOMNode} from 'react-dom'
 import {
   DragSourceSpec,
@@ -7,34 +7,35 @@ import {
   DragSource,
   DropTarget,
   DragSourceConnector,
+  ConnectDragSource,
+  ConnectDropTarget,
+  ConnectDragPreview,
 } from 'react-dnd'
 
-const FieldType = 'field'
+const fieldType = 'field'
 
-interface Field {
+interface RenamableField {
   internalName: string
   displayName: string
   visible: boolean
-  order?: number
-  index?: number
 }
 
-interface FieldSourceProps {
+interface GraphOptionsCustomizableFieldProps {
   internalName: string
   displayName: string
   visible: boolean
   index: number
   id: string
   key: string
+  onFieldUpdate: (field: RenamableField) => void
   isDragging?: boolean
-  onFieldUpdate?: (field: Field) => void
-  connectDragSource?: any
-  connectDropTarget?: any
-  connectDragPreview?: any
+  connectDragSource?: ConnectDragSource
+  connectDropTarget?: ConnectDropTarget
+  connectDragPreview?: ConnectDragPreview
   moveField: (dragIndex: number, hoverIndex: number) => void
 }
 
-const fieldSource: DragSourceSpec<FieldSourceProps> = {
+const fieldSource: DragSourceSpec<GraphOptionsCustomizableFieldProps> = {
   beginDrag(props) {
     return {
       id: props.id,
@@ -97,11 +98,11 @@ function MyDragSource(dragv1, dragv2, dragfunc1) {
   return target => DragSource(dragv1, dragv2, dragfunc1)(target) as any
 }
 
-@MyDropTarget(FieldType, fieldTarget, (connect: DropTargetConnector) => ({
+@MyDropTarget(fieldType, fieldTarget, (connect: DropTargetConnector) => ({
   connectDropTarget: connect.dropTarget(),
 }))
 @MyDragSource(
-  FieldType,
+  fieldType,
   fieldSource,
   (connect: DragSourceConnector, monitor: DragSourceMonitor) => ({
     connectDragSource: connect.dragSource(),
@@ -110,7 +111,7 @@ function MyDragSource(dragv1, dragv2, dragfunc1) {
   })
 )
 export default class GraphOptionsCustomizableField extends PureComponent<
-  FieldSourceProps
+  GraphOptionsCustomizableFieldProps
 > {
   constructor(props) {
     super(props)
@@ -164,7 +165,7 @@ export default class GraphOptionsCustomizableField extends PureComponent<
             spellCheck={false}
             id="internalName"
             value={displayName}
-            onBlur={this.handleFieldRename}
+            data-test="custom-time-format"
             onChange={this.handleFieldRename}
             placeholder={`Rename ${internalName}`}
             disabled={!visible}
@@ -174,7 +175,7 @@ export default class GraphOptionsCustomizableField extends PureComponent<
     )
   }
 
-  private handleFieldRename(e) {
+  private handleFieldRename(e: ChangeEvent<HTMLInputElement>) {
     const {onFieldUpdate, internalName, visible} = this.props
     onFieldUpdate({internalName, displayName: e.target.value, visible})
   }

ui/src/dashboards/components/GraphOptionsCustomizeFields.tsx

@@ -1,23 +1,21 @@
 import React, {SFC} from 'react'
-import GraphOptionsCustomizableField from 'src/dashboards/components/GraphOptionsCustomizableField'
-
-// import FancyScrollbar from 'src/shared/components/FancyScrollbar'
 import {DragDropContext} from 'react-dnd'
 import HTML5Backend from 'react-dnd-html5-backend'
 
-interface Field {
+import GraphOptionsCustomizableField from 'src/dashboards/components/GraphOptionsCustomizableField'
+
+interface RenamableField {
   internalName: string
   displayName: string
   visible: boolean
-  order?: number
 }
 
-interface Props {
-  fields: Field[]
-  onFieldUpdate: (field: Field) => void
-  moveField?: (dragIndex: number, hoverIndex: number) => void
+interface GraphOptionsCustomizeFieldsProps {
+  fields: RenamableField[]
+  onFieldUpdate: (field: RenamableField) => void
+  moveField: (dragIndex: number, hoverIndex: number) => void
 }
-const GraphOptionsCustomizeFields: SFC<Props> = ({
+const GraphOptionsCustomizeFields: SFC<GraphOptionsCustomizeFieldsProps> = ({
   fields,
   onFieldUpdate,
   moveField,

ui/src/dashboards/components/GraphOptionsTimeFormat.tsx

@@ -84,6 +84,7 @@ class GraphOptionsTimeFormat extends PureComponent<Props, State> {
             spellCheck={false}
             placeholder="Enter custom format..."
             value={format}
+            data-test="custom-time-format"
             className="form-control input-sm custom-time-format"
             onChange={this.handleChangeFormat}
           />

ui/src/dashboards/components/TableOptions.tsx

@@ -27,7 +27,6 @@ interface RenamableField {
   internalName: string
   displayName: string
   visible: boolean
-  order?: number
 }
 
 interface Options {
@@ -69,30 +68,6 @@ export class TableOptions extends PureComponent<Props, {}> {
     return tableOptionsDifferent
   }
 
-  public moveField(dragIndex, hoverIndex) {
-    const {handleUpdateTableOptions, tableOptions} = this.props
-    const {fieldNames} = tableOptions
-    const fields = fieldNames.length > 1 ? fieldNames : this.computedFieldNames
-
-    const dragField = fields[dragIndex]
-    const removedFields = _.concat(
-      _.slice(fields, 0, dragIndex),
-      _.slice(fields, dragIndex + 1)
-    )
-    const addedFields = _.concat(
-      _.slice(removedFields, 0, hoverIndex),
-      [dragField],
-      _.slice(removedFields, hoverIndex)
-    )
-    const orderedFields = addedFields.map((f, i) => {
-      return {...f, order: i}
-    })
-    handleUpdateTableOptions({
-      ...tableOptions,
-      fieldNames: orderedFields,
-    })
-  }
-
   public render() {
     const {
       tableOptions: {timeFormat, fieldNames, verticalTimeAxis, fixFirstColumn},
@@ -156,6 +131,27 @@ export class TableOptions extends PureComponent<Props, {}> {
     )
   }
 
+  private moveField(dragIndex, hoverIndex) {
+    const {handleUpdateTableOptions, tableOptions} = this.props
+    const {fieldNames} = tableOptions
+    const fields = fieldNames.length > 1 ? fieldNames : this.computedFieldNames
+
+    const dragField = fields[dragIndex]
+    const removedFields = _.concat(
+      _.slice(fields, 0, dragIndex),
+      _.slice(fields, dragIndex + 1)
+    )
+    const addedFields = _.concat(
+      _.slice(removedFields, 0, hoverIndex),
+      [dragField],
+      _.slice(removedFields, hoverIndex)
+    )
+    handleUpdateTableOptions({
+      ...tableOptions,
+      fieldNames: addedFields,
+    })
+  }
+
   private get computedFieldNames() {
     const {queryConfigs} = this.props
     const queryFields = _.flatten(

ui/src/shared/components/TableGraph.js

@@ -236,10 +236,8 @@ class TableGraph extends Component {
     const timeFieldIndex = fieldNames.findIndex(
       field => field.internalName === TIME_FIELD_DEFAULT.internalName
     )
-    const timeField = fieldNames.find(
-      field => field.internalName === TIME_FIELD_DEFAULT.internalName
-    )
-    const visibleTime = _.get(timeField, 'visible', true)
+
+    const visibleTime = _.get(fieldNames, [timeFieldIndex, 'visible'], true)
 
     const isFixedRow = rowIndex === 0 && columnIndex > 0
     const isFixedColumn = fixFirstColumn && rowIndex > 0 && columnIndex === 0

ui/src/utils/timeSeriesTransformers.js

@@ -204,7 +204,7 @@ export const orderTableColumns = (data, fieldNames) => {
       return dataLabel === fieldName.internalName
     })
   })
-  const filteredFieldSortOrder = _.filter(fieldsSortOrder, f => f !== -1)
+  const filteredFieldSortOrder = filter(fieldsSortOrder, f => f !== -1)
   const orderedData = map(data, row => {
     return row.map((v, j, arr) => arr[filteredFieldSortOrder[j]])
   })

ui/test/dashboards/components/GraphOptionsTimeFormat.test.tsx

@@ -25,10 +25,11 @@ describe('Dashboards.Components.GraphOptionsTimeFormat', () => {
     describe('when it is not a custom format', () => {
       it('renders only a dropdown', () => {
         const wrapper = setup()
+        const input = wrapper.find({'data-test': 'custom-time-format'})
 
         expect(wrapper.find(Dropdown).exists()).toBe(true)
         expect(wrapper.find(QuestionMarkTooltip).exists()).toBe(false)
-        expect(wrapper.find('input.custom-time-format').exists()).toBe(false)
+        expect(input.exists()).toBe(false)
       })
     })
 
@@ -40,13 +41,14 @@ describe('Dashboards.Components.GraphOptionsTimeFormat', () => {
 
         const label = wrapper.find('label')
         const link = label.find('a')
+        const input = wrapper.find({'data-test': 'custom-time-format'})
 
         expect(wrapper.find(Dropdown).exists()).toBe(true)
         expect(label.exists()).toBe(true)
         expect(link.exists()).toBe(true)
         expect(link.prop('href')).toBe(TIME_FORMAT_TOOLTIP_LINK)
         expect(link.find(QuestionMarkTooltip).exists()).toBe(true)
-        expect(wrapper.find('input.custom-time-format').exists()).toBe(true)
+        expect(input.exists()).toBe(true)
       })
     })
 
@@ -55,7 +57,7 @@ describe('Dashboards.Components.GraphOptionsTimeFormat', () => {
         const timeFormat = 'mmmmmmm'
         const wrapper = setup({timeFormat})
         const dropdown = wrapper.find(Dropdown)
-        const input = wrapper.find('input.custom-time-format')
+        const input = wrapper.find({'data-test': 'custom-time-format'})
         const label = wrapper.find('label')
         const link = label.find('a')