Prevent unneccessary re-renders

2019-06-06 23:15:12 +02:00
parent 15a0c8a5c5
commit e728a721aa
5 changed files with 81 additions and 102 deletions
--- a/app/src/components/Sidebar/CodeDiff/Gutters.tsx
+++ b/app/src/components/Sidebar/CodeDiff/Gutters.tsx
@@ -5,7 +5,7 @@ import Remove from '@material-ui/icons/Remove'
 import ShowChart from '@material-ui/icons/ShowChart'
 import { JsonPropertyLocation } from '../../../../../backend/src/JsonAstParser'
 import { lineChangeStyle, trimNewlineRight } from './util'
-import { Theme } from '@material-ui/core'
+import { Theme, Tooltip } from '@material-ui/core'
 import { withStyles } from '@material-ui/styles'

 interface Props {
@@ -13,42 +13,55 @@ interface Props {
  literalPositions: Array<JsonPropertyLocation>
  classes: any
  className: string
-  showDiagram: (dotPath: string, target: EventTarget) => void
+  showDiagram: (dotPath: string, target: React.Ref<HTMLElement>) => void
  hideDiagram: () => void
+  hasEnoughDataToDisplayDiagrams: boolean
 }

 const style = (theme: Theme) => {
+  const icon = {
+    verticalAlign: 'top',
+    width: '12px',
+    height: '12px',
+    marginTop: '2px',
+    borderRadius: '50%',
+  }
+
  return {
+    icon,
+    iconButton: {
+      ...icon,
+      width: '16px',
+      height: '16px',
+      marginTop: '1px',
+      padding: '2px',
+      '&:hover': {
+        color: theme.palette.primary.contrastText,
+        backgroundColor: theme.palette.primary.main,
+      },
+    },
    gutterLine: {
      textAlign: 'right' as 'right',
      paddingRight: theme.spacing(0.5),
      height: '16px',
      width: '100%',
    },
-    icon: {
-      width: '12px',
-      height: '12px',
-      marginTop: '2px',
-      borderRadius: '50%',
-      '&:hover': {
-        color: theme.palette.primary.contrastText,
-        backgroundColor: theme.palette.primary.main,
-      },
-    },
  }
 }

-function ChartIcon(props: { classes: any, literal: JsonPropertyLocation, showDiagram: (dotPath: string, target: EventTarget) => void, hideDiagram: () => void }) {
-  const mouseOver = (event: React.MouseEvent<Element>) => {
-    props.showDiagram(props.literal.path, event.target)
-  }
+function ChartIcon(props: { classes: any, literal: JsonPropertyLocation, showDiagram: (dotPath: string, target: React.Ref<HTMLElement>) => void, hideDiagram: () => void }) {
+  const chartIconRef = React.useRef(null)

-  const mouseOut = (event: React.MouseEvent) => {
+  const mouseOver = React.useCallback((event: React.MouseEvent<Element>) => {
+    props.showDiagram(props.literal.path, chartIconRef)
+  }, [props.literal.path])
+
+  const mouseOut = React.useCallback(() => {
    props.hideDiagram()
-  }
+  }, [])

  return (
-    <ShowChart className={props.classes.icon} onMouseEnter={mouseOver} onMouseLeave={mouseOut} />
+    <ShowChart ref={chartIconRef} className={props.classes.icon} onMouseEnter={mouseOver} onMouseLeave={mouseOut} />
  )
 }

@@ -56,7 +69,7 @@ function tokensForLine(change: diff.Change, line: number, props: Props) {
  const { classes, literalPositions } = props

  const literal = literalPositions[line]
-  const diagram = literal ? <ChartIcon classes={{ icon: props.classes.icon }} literal={literal} showDiagram={props.showDiagram} hideDiagram={props.hideDiagram}/> : null
+  const diagram = literal ? <Tooltip title="Not enough data"><ChartIcon classes={{ icon: props.classes.iconButton }} literal={literal} showDiagram={props.showDiagram} hideDiagram={props.hideDiagram}/></Tooltip> : null

  if (change.added) {
    return [diagram, <Add key="add" className={classes.icon} />]
--- a/app/src/components/Sidebar/CodeDiff/index.tsx
+++ b/app/src/components/Sidebar/CodeDiff/index.tsx
@@ -5,11 +5,16 @@ import * as React from 'react'
 import DiffCount from './DiffCount'
 import Gutters from './Gutters'
 import TopicPlot from '../TopicPlot'
-import { CodeBlockColors, CodeBlockColorsBraceMonokai } from '../CodeBlockColors'
+import {
+  Fade,
+  Paper,
+  Popper,
+  withStyles
+  } from '@material-ui/core'
 import { isPlottable, lineChangeStyle, trimNewlineRight } from './util'
 import { JsonPropertyLocation, literalsMappedByLines } from '../../../../../backend/src/JsonAstParser'
-import { Theme, withStyles, Popper, Paper, Fade, Zoom } from '@material-ui/core'
 import { selectTextWithCtrlA } from '../../../utils/handleTextSelectWithCtrlA'
+import { style } from './style'
 import 'prismjs/components/prism-json'
 const throttle = require('lodash.throttle')

@@ -27,8 +32,8 @@ interface State {
 }

 interface DiagramOptions {
-  dotPath?: string
-  anchorEl?: EventTarget
+  dotPath: string
+  anchorEl: React.Ref<HTMLElement>
 }

 class CodeDiff extends React.Component<Props, State> {
@@ -43,33 +48,35 @@ class CodeDiff extends React.Component<Props, State> {
    this.state = {}
  }

-  private showDiagram(dotPath: string, target: EventTarget) {
+  private showDiagram = (dotPath: string, target: React.Ref<Element>) => {
    this.updateDiagram({
      dotPath,
      anchorEl: target,
    })
  }

-  private hideDiagram() {
+  private hideDiagram = () => {
    this.updateDiagram(undefined)
  }

-  public render() {
-    const changes = diff.diffLines(this.props.previous, this.props.current)
-    const styledLines = Prism.highlight(this.props.current, Prism.languages.json, 'json').split('\n')
-    const literalPositions = (
-      (literalsMappedByLines(this.props.current) || [])
-          .map((l: JsonPropertyLocation) => isPlottable(l.value) ? l : undefined)
-    ) as Array<JsonPropertyLocation>
+  private plottableLiteralsIndexedWithLineNumbers() {
+    const allLiterals = this.isValidJson(this.props.current) ? (literalsMappedByLines(this.props.current) || []) : []

+    return allLiterals
+      .map((l: JsonPropertyLocation) => isPlottable(l.value) ? l : undefined) as Array<JsonPropertyLocation>
+  }
+
+  private renderLines(changes: Array<Diff.Change>) {
+    const styledLines = Prism.highlight(this.props.current, Prism.languages.json, 'json').split('\n')
    let lineNumber = 0
-    const code = changes.map((change, key) => {
+
+    return changes.map((change, key) => {
      const hasStyledCode = change.removed !== true
      const changedLines = change.count || 0
      if (hasStyledCode && this.props.language === 'json') {
        const currentLines = styledLines.slice(lineNumber, lineNumber + changedLines)
        const lines = currentLines.map((html: string, idx: number) => {
-          return <div key={`${key}-${idx}`} style={lineChangeStyle(change)} className={`${this.props.classes.line}`}><span dangerouslySetInnerHTML={{ __html: html }} /></div>
+          return <div key={`${key}-${idx}`} style={lineChangeStyle(change)} className={this.props.classes.line}><span dangerouslySetInnerHTML={{ __html: html }} /></div>
        })
        lineNumber += changedLines

@@ -82,23 +89,31 @@ class CodeDiff extends React.Component<Props, State> {
          return <div key={`${key}-${idx}`} style={lineChangeStyle(change)} className={this.props.classes.line}><span>{line}</span></div>
        })
    }).reduce((a, b) => a.concat(b), [])
+  }
+
+  public render() {
+    const changes = diff.diffLines(this.props.previous, this.props.current)
+    const literalPositions = this.plottableLiteralsIndexedWithLineNumbers()
+
+    const code = this.renderLines(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={(dotPath, target) => this.showDiagram(dotPath, target)}
-            hideDiagram={() => this.hideDiagram()}
+            showDiagram={this.showDiagram}
+            hideDiagram={this.hideDiagram}
            className={this.props.classes.gutters}
+            hasEnoughDataToDisplayDiagrams={hasEnoughDataToDisplayDiagrams}
            changes={changes}
            literalPositions={literalPositions} />
          <pre className={this.props.classes.codeBlock}>{code}</pre>
        </div>
        <Popper
-          open={Boolean(this.state.diagram)}
-          anchorEl={diagram && diagram.anchorEl as any}
+          open={Boolean(this.state.diagram) && hasEnoughDataToDisplayDiagrams}
+          anchorEl={diagram && (diagram.anchorEl as any).current}
          placement="left-end"
        >
        <Fade in={Boolean(this.state.diagram)} timeout={300}>
@@ -111,67 +126,14 @@ class CodeDiff extends React.Component<Props, State> {
      </div>
    )
  }
-}

-const style = (theme: Theme) => {
-  const codeBlockColors = theme.palette.type === 'light' ? CodeBlockColors : CodeBlockColorsBraceMonokai
-
-  const codeBaseStyle = {
-    font: "12px/normal 'Monaco', 'Menlo', 'Ubuntu Mono', 'Consolas', 'source-code-pro', monospace",
-    display: 'inline-grid' as 'inline-grid',
-    margin: '0',
-    padding: '1px 0 0 0',
+  private isValidJson(str: string) {
+    try {
+      JSON.parse(str)
+      return true
+    } catch (error) {
+      return false
    }
-
-  return {
-    line: {
-      lineHeight: 'normal' as 'normal',
-      paddingLeft: '4px',
-      width: '100%',
-      height: '16px',
-    },
-    codeWrapper: {
-      display: 'flex',
-      maxHeight: '15em',
-      overflow: 'auto',
-      backgroundColor: `${codeBlockColors.background}`,
-      margin: '8px 0 0 0',
-    },
-    gutters: {
-      ...codeBaseStyle,
-      width: '33px',
-      backgroundColor: codeBlockColors.gutters,
-      userSelect: 'none' as 'none',
-    },
-    codeBlock: {
-      ...codeBaseStyle,
-      width: 'calc(100% - 33px)',
-      backgroundColor: 'inherit !important',
-      '& span': {
-        color: codeBlockColors.text,
-      },
-      '& .token.number': {
-        color: codeBlockColors.numeric,
-      },
-      '& .token.boolean': {
-        color: codeBlockColors.numeric,
-      },
-      '& .token.property': {
-        color: codeBlockColors.variable,
-      },
-      '& .token.string': {
-        color: codeBlockColors.string,
-      },
-      '& .token': {
-        color: codeBlockColors.text,
-      },
-      '& .token.operator': {
-        color: codeBlockColors.text,
-      },
-      '& .token.punctuation': {
-        color: codeBlockColors.text,
-      },
-    },
  }
 }

--- a/app/src/components/Sidebar/HistoryDrawer.tsx
+++ b/app/src/components/Sidebar/HistoryDrawer.tsx
@@ -34,7 +34,7 @@ class HistoryDrawer extends React.Component<Props, State> {
    this.setState({ collapsed: !this.state.collapsed })
  }

-  private handleCtrlA = selectTextWithCtrlA({targetSelector: 'pre'})
+  private handleCtrlA = selectTextWithCtrlA({ targetSelector: 'pre' })

  public renderHistory() {
    const style = (element: HistoryItem) => ({
--- a/backend/src/Model/RingBuffer.ts
+++ b/backend/src/Model/RingBuffer.ts
@@ -62,6 +62,10 @@ export class RingBuffer<T extends Lengthwise> {
    return this.items.slice(this.start, this.end)
  }

+  public count() {
+    return this.end - this.start
+  }
+
  public add(item: T) {
    const size = item.length
    this.enforceCapacityConstraints(size)