From 3935b1d614125f6ea811bf511fdd8641977fdaa1 Mon Sep 17 00:00:00 2001
From: Thomas Nordquist <xxnerowingerxx@gmail.com>
Date: Fri, 14 Jun 2019 10:41:52 +0200
Subject: [PATCH] Fix flaky diagram

---
 .../components/Sidebar/CodeDiff/Gutters.tsx   | 56 ++++++++++++----
 app/src/components/Sidebar/CodeDiff/index.tsx | 65 +++----------------
 tslint.json                                   |  1 +
 3 files changed, 54 insertions(+), 68 deletions(-)

diff --git a/app/src/components/Sidebar/CodeDiff/Gutters.tsx b/app/src/components/Sidebar/CodeDiff/Gutters.tsx
index 0190615..5112e0f 100644
--- a/app/src/components/Sidebar/CodeDiff/Gutters.tsx
+++ b/app/src/components/Sidebar/CodeDiff/Gutters.tsx
@@ -1,11 +1,19 @@
 import * as diff from 'diff'
+import * as q from '../../../../../backend/src/Model'
 import * as React from 'react'
 import Add from '@material-ui/icons/Add'
 import Remove from '@material-ui/icons/Remove'
 import ShowChart from '@material-ui/icons/ShowChart'
+import TopicPlot from '../TopicPlot'
+import {
+  Fade,
+  Paper,
+  Popper,
+  Theme,
+  Tooltip
+  } from '@material-ui/core'
 import { JsonPropertyLocation } from '../../../../../backend/src/JsonAstParser'
 import { lineChangeStyle, trimNewlineRight } from './util'
-import { Theme, Tooltip } from '@material-ui/core'
 import { withStyles } from '@material-ui/styles'
 
 interface Props {
@@ -13,9 +21,7 @@ interface Props {
   literalPositions: Array<JsonPropertyLocation>
   classes: any
   className: string
-  showDiagram: (dotPath: string, target: React.Ref<HTMLElement>) => void
-  hideDiagram: () => void
-  hasEnoughDataToDisplayDiagrams: boolean
+  messageHistory: q.MessageHistory
 }
 
 const style = (theme: Theme) => {
@@ -29,11 +35,14 @@ const style = (theme: Theme) => {
 
   return {
     icon,
+    iconDisabled: {
+      ...icon,
+      color: theme.palette.text.disabled,
+    },
     iconButton: {
       ...icon,
       width: '16px',
       height: '16px',
-      marginTop: '1px',
       padding: '2px',
       '&:hover': {
         color: theme.palette.primary.contrastText,
@@ -49,34 +58,55 @@ const style = (theme: Theme) => {
   }
 }
 
-function ChartIcon(props: { classes: any, literal: JsonPropertyLocation, showDiagram: (dotPath: string, target: React.Ref<HTMLElement>) => void, hideDiagram: () => void }) {
+function ChartIcon(props: { messageHistory: q.MessageHistory, classes: any, literal: JsonPropertyLocation }) {
   const chartIconRef = React.useRef(null)
+  const [open, setOpen] = React.useState(false)
 
   const mouseOver = React.useCallback((event: React.MouseEvent<Element>) => {
-    props.showDiagram(props.literal.path, chartIconRef)
+    setOpen(true)
   }, [props.literal.path])
 
   const mouseOut = React.useCallback(() => {
-    props.hideDiagram()
+    setOpen(false)
   }, [])
 
-  return (
+  return (<span>
     <ShowChart ref={chartIconRef} className={props.classes.icon} onMouseEnter={mouseOver} onMouseLeave={mouseOut} />
+    <Popper
+      open={open}
+      anchorEl={chartIconRef.current}
+      placement="left-end"
+    >
+      <Fade in={open} timeout={300}>
+        <Paper style={{ width: '300px' }}>
+          {open ? <TopicPlot history={props.messageHistory} dotPath={props.literal.path} /> : <span/>}
+        </Paper>
+      </Fade>
+    </Popper>
+  </span>
   )
 }
 
 function tokensForLine(change: diff.Change, line: number, props: Props) {
   const { classes, literalPositions } = props
-
+  const hasEnoughDataToDisplayDiagrams = props.messageHistory.count() > 1
   const literal = literalPositions[line]
-  const diagram = literal ? <Tooltip title="Not enough data"><ChartIcon classes={{ icon: props.classes.iconButton }} literal={literal} showDiagram={props.showDiagram} hideDiagram={props.hideDiagram}/></Tooltip> : null
+
+  let chartIcon = null
+  if (literal) {
+    if (hasEnoughDataToDisplayDiagrams) {
+      chartIcon = <ChartIcon messageHistory={props.messageHistory} classes={{ icon: props.classes.iconButton }} literal={literal} />
+    } else {
+      chartIcon = <Tooltip title="Not enough data"><ShowChart className={props.classes.iconDisabled} style={{ color: '#aaa' }} /></Tooltip>
+    }
+  }
 
   if (change.added) {
-    return [diagram, <Add key="add" className={classes.icon} />]
+    return [chartIcon, <Add key="add" className={classes.icon} />]
   } else if (change.removed) {
     return [<Remove key="remove" className={classes.icon} />]
   } else {
-    return [diagram, <div key="placeholder" style={{ width: '12px', display: 'inline-block' }} dangerouslySetInnerHTML={{ __html: '&nbsp;'}} />]
+    return [chartIcon, <div key="placeholder" style={{ width: '12px', display: 'inline-block' }} dangerouslySetInnerHTML={{ __html: '&nbsp;' }} />]
   }
 }
 
diff --git a/app/src/components/Sidebar/CodeDiff/index.tsx b/app/src/components/Sidebar/CodeDiff/index.tsx
index 8ffdd64..fd44c0a 100644
--- a/app/src/components/Sidebar/CodeDiff/index.tsx
+++ b/app/src/components/Sidebar/CodeDiff/index.tsx
@@ -4,19 +4,12 @@ import * as q from '../../../../../backend/src/Model'
 import * as React from 'react'
 import DiffCount from './DiffCount'
 import Gutters from './Gutters'
-import TopicPlot from '../TopicPlot'
-import {
-  Fade,
-  Paper,
-  Popper,
-  withStyles
-  } from '@material-ui/core'
 import { isPlottable, lineChangeStyle, trimNewlineRight } from './util'
 import { JsonPropertyLocation, literalsMappedByLines } from '../../../../../backend/src/JsonAstParser'
 import { selectTextWithCtrlA } from '../../../utils/handleTextSelectWithCtrlA'
 import { style } from './style'
+import { withStyles } from '@material-ui/core'
 import 'prismjs/components/prism-json'
-const throttle = require('lodash.throttle')
 
 interface Props {
   messageHistory: q.MessageHistory
@@ -27,36 +20,23 @@ interface Props {
   classes: any
 }
 
-interface State {
-  diagram?: DiagramOptions
-}
-
-interface DiagramOptions {
-  dotPath: string
-  anchorEl: React.Ref<HTMLElement>
-}
+interface State {}
 
 class CodeDiff extends React.Component<Props, State> {
   private handleCtrlA = selectTextWithCtrlA({ targetSelector: 'pre ~ pre' })
 
-  private updateDiagram = throttle((diagram?: DiagramOptions) => {
-    this.setState({ diagram })
-  }, 200)
-
   constructor(props: Props) {
     super(props)
     this.state = {}
   }
 
-  private showDiagram = (dotPath: string, target: React.Ref<Element>) => {
-    this.updateDiagram({
-      dotPath,
-      anchorEl: target,
-    })
-  }
-
-  private hideDiagram = () => {
-    this.updateDiagram(undefined)
+  private isValidJson(str: string) {
+    try {
+      JSON.parse(str)
+      return true
+    } catch (error) {
+      return false
+    }
   }
 
   private plottableLiteralsIndexedWithLineNumbers() {
@@ -94,47 +74,22 @@ class CodeDiff extends React.Component<Props, State> {
   public render() {
     const changes = diff.diffLines(this.props.previous, this.props.current)
     const literalPositions = this.plottableLiteralsIndexedWithLineNumbers()
-
     const code = this.renderStyledCodeLines(changes)
 
-    const { diagram } = this.state
-    const hasEnoughDataToDisplayDiagrams = this.props.messageHistory.count() > 1
     return (
       <div>
         <div tabIndex={0} onKeyDown={this.handleCtrlA} className={this.props.classes.codeWrapper}>
           <Gutters
-            showDiagram={this.showDiagram}
-            hideDiagram={this.hideDiagram}
             className={this.props.classes.gutters}
-            hasEnoughDataToDisplayDiagrams={hasEnoughDataToDisplayDiagrams}
             changes={changes}
+            messageHistory={this.props.messageHistory}
             literalPositions={literalPositions} />
           <pre className={this.props.classes.codeBlock}>{code}</pre>
         </div>
-        <Popper
-          open={Boolean(this.state.diagram) && hasEnoughDataToDisplayDiagrams}
-          anchorEl={diagram && (diagram.anchorEl as any).current}
-          placement="left-end"
-        >
-        <Fade in={Boolean(this.state.diagram)} timeout={300}>
-          <Paper style={{ width: '300px' }}>
-            {diagram ? <TopicPlot history={this.props.messageHistory} dotPath={diagram.dotPath} /> : <span/>}
-          </Paper>
-        </Fade>
-        </Popper>
         <DiffCount changes={changes} nameOfCompareMessage={this.props.nameOfCompareMessage} />
       </div>
     )
   }
-
-  private isValidJson(str: string) {
-    try {
-      JSON.parse(str)
-      return true
-    } catch (error) {
-      return false
-    }
-  }
 }
 
 export default withStyles(style)(CodeDiff)
diff --git a/tslint.json b/tslint.json
index a7a2403..555bfee 100644
--- a/tslint.json
+++ b/tslint.json
@@ -12,6 +12,7 @@
     "no-submodule-imports": false,
     "array-type": [true, "generic"],
     "prefer-array-literal": false,
+    "function-name": false,
     "variable-name": [true, "ban-keywords", "check-format", "allow-pascal-case"],
     "trailing-comma": [
       true,