From b91a1f2f810b62fbb655bfe4ad86ac7513e59f05 Mon Sep 17 00:00:00 2001
From: "Michael H. Arieli" <michaelharieli@gmail.com>
Date: Thu, 29 Nov 2018 17:47:29 -0800
Subject: [PATCH] Fix bug: Improve code to use `ref` and test units

---
 src/client/__mocks__/window.js                | 13 ++-
 src/client/components/App.js                  | 26 +++++-
 src/client/components/Chat.js                 | 35 +-------
 src/client/components/Input.js                | 19 ++--
 src/client/components/Toolbar.js              | 31 ++++---
 src/client/components/Video.js                |  6 +-
 src/client/components/__tests__/Input-test.js |  4 +-
 .../components/__tests__/Toolbar-test.js      | 87 +++++++++++++++++++
 src/client/components/__tests__/Video-test.js |  5 +-
 9 files changed, 166 insertions(+), 60 deletions(-)
 create mode 100644 src/client/components/__tests__/Toolbar-test.js

diff --git a/src/client/__mocks__/window.js b/src/client/__mocks__/window.js
index e589b8f..a56378b 100644
--- a/src/client/__mocks__/window.js
+++ b/src/client/__mocks__/window.js
@@ -4,7 +4,18 @@ export const createObjectURL = jest.fn()
 .mockImplementation(object => 'blob://' + String(object))
 export const revokeObjectURL = jest.fn()
 
-export class MediaStream {}
+export class MediaStream {
+  getVideoTracks () {
+    return [{
+      enabled: true
+    }]
+  }
+  getAudioTracks () {
+    return [{
+      enabled: true
+    }]
+  }
+}
 export function getUserMedia () {
   return !getUserMedia.shouldFail
   ? Promise.resolve(getUserMedia.stream)
diff --git a/src/client/components/App.js b/src/client/components/App.js
index f305d74..adc122f 100644
--- a/src/client/components/App.js
+++ b/src/client/components/App.js
@@ -23,6 +23,12 @@ export default class App extends React.PureComponent {
     streams: PropTypes.objectOf(StreamPropType).isRequired,
     toggleActive: PropTypes.func.isRequired
   }
+  constructor () {
+    super()
+    this.state = {
+      videos: {}
+    }
+  }
   componentDidMount () {
     const { init } = this.props
     init()
@@ -41,17 +47,29 @@ export default class App extends React.PureComponent {
       streams
     } = this.props
 
+    const { videos } = this.state
+
     return (
       <div className="app">
-        <Toolbar messages={messages} stream={streams[constants.ME]} />
+        <Toolbar
+          chatRef={this.chatRef}
+          messages={messages}
+          stream={streams[constants.ME]}
+          ref={node => { this.toolbarRef = node }}
+        />
         <Alerts alerts={alerts} dismiss={dismissAlert} />
         <Notifications notifications={notifications} />
-        <div id="chat">
-          <Chat messages={messages} />
-          <Input notify={notify} sendMessage={sendMessage} />
+        <div id="chat" ref={node => { this.chatRef = node }}>
+          <Chat toolbarRef={this.toolbarRef} messages={messages} />
+          <Input
+            videos={videos}
+            notify={notify}
+            sendMessage={sendMessage}
+          />
         </div>
         <div className="videos">
           <Video
+            videos={videos}
             active={active === constants.ME}
             onClick={toggleActive}
             stream={streams[constants.ME]}
diff --git a/src/client/components/Chat.js b/src/client/components/Chat.js
index 4d9e08e..71ce23f 100644
--- a/src/client/components/Chat.js
+++ b/src/client/components/Chat.js
@@ -10,42 +10,15 @@ export const MessagePropTypes = PropTypes.shape({
 
 export default class Chat extends React.PureComponent {
   static propTypes = {
+    toolbarRef: PropTypes.object.isRequired,
     messages: PropTypes.arrayOf(MessagePropTypes).isRequired
   }
   handleCloseChat = e => {
-    document.getElementById('chat').classList.remove('show')
-    document.querySelector('.toolbar .chat').classList.remove('on')
+    const { toolbarRef } = this.props
+    toolbarRef.chatButton.click()
   }
   scrollToBottom = () => {
-    // this.chatScroll.scrollTop = this.chatScroll.scrollHeight
-
-    const duration = 300
-    const start = this.chatScroll.scrollTop
-    const end = this.chatScroll.scrollHeight
-    const change = end - start
-    const increment = 20
-
-    const easeInOut = (currentTime, start, change, duration) => {
-      currentTime /= duration / 2
-      if (currentTime < 1) {
-        return change / 2 * currentTime * currentTime + start
-      }
-      currentTime -= 1
-      return -change / 2 * (currentTime * (currentTime - 2) - 1) + start
-    }
-
-    const animate = elapsedTime => {
-      elapsedTime += increment
-      const position = easeInOut(elapsedTime, start, change, duration)
-      this.chatScroll.scrollTop = position
-      if (elapsedTime < duration) {
-        setTimeout(() => {
-          animate(elapsedTime)
-        }, increment)
-      }
-    }
-
-    animate(0)
+    this.chatScroll.scrollTop = this.chatScroll.scrollHeight
   }
   componentDidMount () {
     this.scrollToBottom()
diff --git a/src/client/components/Input.js b/src/client/components/Input.js
index a38852e..011c8e9 100644
--- a/src/client/components/Input.js
+++ b/src/client/components/Input.js
@@ -4,6 +4,7 @@ import socket from '../socket.js'
 
 export default class Input extends React.PureComponent {
   static propTypes = {
+    videos: PropTypes.object.isRequired,
     notify: PropTypes.func.isRequired,
     sendMessage: PropTypes.func.isRequired
   }
@@ -29,7 +30,7 @@ export default class Input extends React.PureComponent {
     }
   }
   submit = () => {
-    const { notify, sendMessage } = this.props
+    const { videos, notify, sendMessage } = this.props
     const { message } = this.state
     if (message) {
       notify('You: ' + message)
@@ -41,13 +42,15 @@ export default class Input extends React.PureComponent {
 
       // take snapshoot
       try {
-        const video = document.getElementById(`video-${userId}`)
-        const canvas = document.createElement('canvas')
-        canvas.height = video.videoHeight
-        canvas.width = video.videoWidth
-        const avatar = canvas.getContext('2d')
-        avatar.drawImage(video, 0, 0, canvas.width, canvas.height)
-        image = canvas.toDataURL()
+        const video = videos[userId]
+        if (video) {
+          const canvas = document.createElement('canvas')
+          canvas.height = video.videoHeight
+          canvas.width = video.videoWidth
+          const avatar = canvas.getContext('2d')
+          avatar.drawImage(video, 0, 0, canvas.width, canvas.height)
+          image = canvas.toDataURL()
+        }
       } catch (e) {}
 
       const payload = { userId, message, timestamp, image }
diff --git a/src/client/components/Toolbar.js b/src/client/components/Toolbar.js
index 726fc64..f2fbaa0 100644
--- a/src/client/components/Toolbar.js
+++ b/src/client/components/Toolbar.js
@@ -6,6 +6,7 @@ import { StreamPropType } from './Video.js'
 
 export default class Toolbar extends React.PureComponent {
   static propTypes = {
+    chatRef: PropTypes.object.isRequired,
     messages: PropTypes.arrayOf(MessagePropTypes).isRequired,
     stream: StreamPropType
   }
@@ -16,36 +17,36 @@ export default class Toolbar extends React.PureComponent {
       totalMessages: 0
     }
   }
-  handleChatClick = e => {
-    const { messages } = this.props
-    document.getElementById('chat').classList.toggle('show')
-    e.currentTarget.classList.toggle('on')
+  handleChatClick = () => {
+    const { chatRef, messages } = this.props
+    chatRef.classList.toggle('show')
+    this.chatButton.classList.toggle('on')
     this.setState({
-      isChatOpen: document.getElementById('chat').classList.contains('show'),
+      isChatOpen: chatRef.classList.contains('show'),
       totalMessages: messages.length
     })
   }
-  handleMicClick = e => {
+  handleMicClick = () => {
     const { stream } = this.props
     stream.mediaStream.getAudioTracks().forEach(track => {
       track.enabled = !track.enabled
     })
-    e.currentTarget.classList.toggle('on')
+    this.mixButton.classList.toggle('on')
   }
-  handleCamClick = e => {
+  handleCamClick = () => {
     const { stream } = this.props
     stream.mediaStream.getVideoTracks().forEach(track => {
       track.enabled = !track.enabled
     })
-    e.currentTarget.classList.toggle('on')
+    this.camButton.classList.toggle('on')
   }
-  handleFullscreenClick = e => {
+  handleFullscreenClick = () => {
     if (screenfull.enabled) {
-      screenfull.toggle(e.target)
-      e.currentTarget.classList.toggle('on')
+      screenfull.toggle(this.fullscreenButton)
+      this.fullscreenButton.classList.toggle('on')
     }
   }
-  handleHangoutClick = e => {
+  handleHangoutClick = () => {
     window.location.href = '/'
   }
   render () {
@@ -55,6 +56,7 @@ export default class Toolbar extends React.PureComponent {
     return (
       <div className="toolbar active">
         <div onClick={this.handleChatClick}
+          ref={node => { this.chatButton = node }}
           className="button chat"
           data-blink={messages.length !== totalMessages && !isChatOpen}
           title="Chat"
@@ -65,6 +67,7 @@ export default class Toolbar extends React.PureComponent {
         {stream && (
           <div>
             <div onClick={this.handleMicClick}
+              ref={node => { this.mixButton = node }}
               className="button mute-audio"
               title="Mute audio"
             >
@@ -72,6 +75,7 @@ export default class Toolbar extends React.PureComponent {
               <span className="off icon icon-mic" />
             </div>
             <div onClick={this.handleCamClick}
+              ref={node => { this.camButton = node }}
               className="button mute-video"
               title="Mute video"
             >
@@ -82,6 +86,7 @@ export default class Toolbar extends React.PureComponent {
         )}
 
         <div onClick={this.handleFullscreenClick}
+          ref={node => { this.fullscreenButton = node }}
           className="button fullscreen"
           title="Enter fullscreen"
         >
diff --git a/src/client/components/Video.js b/src/client/components/Video.js
index 0c962c0..6905e48 100644
--- a/src/client/components/Video.js
+++ b/src/client/components/Video.js
@@ -11,6 +11,7 @@ export const StreamPropType = PropTypes.shape({
 
 export default class Video extends React.PureComponent {
   static propTypes = {
+    videos: PropTypes.object.isRequired,
     onClick: PropTypes.func,
     active: PropTypes.bool.isRequired,
     stream: StreamPropType,
@@ -29,7 +30,7 @@ export default class Video extends React.PureComponent {
     this.componentDidUpdate()
   }
   componentDidUpdate () {
-    const { stream } = this.props
+    const { videos, stream } = this.props
     const { video } = this.refs
     const mediaStream = stream && stream.mediaStream
     const url = stream && stream.url
@@ -40,6 +41,9 @@ export default class Video extends React.PureComponent {
     } else if (video.src !== url) {
       video.src = url
     }
+    if (socket.id) {
+      videos[socket.id] = video
+    }
   }
   render () {
     const { active } = this.props
diff --git a/src/client/components/__tests__/Input-test.js b/src/client/components/__tests__/Input-test.js
index ee7a0c9..42161f1 100644
--- a/src/client/components/__tests__/Input-test.js
+++ b/src/client/components/__tests__/Input-test.js
@@ -5,12 +5,14 @@ import TestUtils from 'react-dom/test-utils'
 
 describe('components/Input', () => {
 
-  let component, node, notify, sendMessage
+  let component, node, videos, notify, sendMessage
   function render () {
+    videos = {}
     notify = jest.fn()
     sendMessage = jest.fn()
     component = TestUtils.renderIntoDocument(
       <Input
+        videos={videos}
         sendMessage={sendMessage}
         notify={notify}
       />
diff --git a/src/client/components/__tests__/Toolbar-test.js b/src/client/components/__tests__/Toolbar-test.js
new file mode 100644
index 0000000..015e996
--- /dev/null
+++ b/src/client/components/__tests__/Toolbar-test.js
@@ -0,0 +1,87 @@
+jest.mock('../../window.js')
+import React from 'react'
+import ReactDOM from 'react-dom'
+import TestUtils from 'react-dom/test-utils'
+import Toolbar from '../Toolbar.js'
+import { MediaStream } from '../../window.js'
+
+describe('components/Video', () => {
+
+  class ToolbarWrapper extends React.PureComponent {
+    static propTypes = Toolbar.propTypes
+    constructor () {
+      super()
+      this.state = {}
+    }
+    render () {
+      return <Toolbar
+        chatRef={this.props.chatRef}
+        messages={this.props.messages}
+        stream={this.state.stream || this.props.stream}
+      />
+    }
+  }
+
+  let component, node, chatRef, mediaStream, url
+  function render () {
+    mediaStream = new MediaStream()
+    chatRef = ReactDOM.findDOMNode(
+      TestUtils.renderIntoDocument(<div />)
+    )
+    component = TestUtils.renderIntoDocument(
+      <ToolbarWrapper
+        chatRef={chatRef}
+        messages={[]}
+        stream={{ mediaStream, url }}
+      />
+    )
+    node = ReactDOM.findDOMNode(component)
+  }
+
+  describe('render', () => {
+    it('should not fail', () => {
+      render()
+    })
+  })
+
+  describe('handleChatClick', () => {
+    it('toggle chat', () => {
+      const button = node.querySelector('.chat')
+      TestUtils.Simulate.click(button)
+      expect(button.classList.contains('on')).toBe(true)
+    })
+  })
+
+  describe('handleMicClick', () => {
+    it('toggle mic', () => {
+      const button = node.querySelector('.mute-audio')
+      TestUtils.Simulate.click(button)
+      expect(button.classList.contains('on')).toBe(true)
+    })
+  })
+
+  describe('handleCamClick', () => {
+    it('toggle cam', () => {
+      const button = node.querySelector('.mute-video')
+      TestUtils.Simulate.click(button)
+      expect(button.classList.contains('on')).toBe(true)
+    })
+  })
+
+  describe('handleFullscreenClick', () => {
+    it('toggle fullscreen', () => {
+      const button = node.querySelector('.fullscreen')
+      TestUtils.Simulate.click(button)
+      expect(button.classList.contains('on')).toBe(false)
+    })
+  })
+
+  describe('handleHangoutClick', () => {
+    it('hangout', () => {
+      const button = node.querySelector('.hangup')
+      TestUtils.Simulate.click(button)
+      expect(window.location.href).toBe('http://localhost/')
+    })
+  })
+
+})
diff --git a/src/client/components/__tests__/Video-test.js b/src/client/components/__tests__/Video-test.js
index 074f6f5..2125746 100644
--- a/src/client/components/__tests__/Video-test.js
+++ b/src/client/components/__tests__/Video-test.js
@@ -14,6 +14,7 @@ describe('components/Video', () => {
     }
     render () {
       return <Video
+        videos={this.props.videos}
         active={this.props.active}
         stream={this.state.stream || this.props.stream}
         onClick={this.props.onClick}
@@ -22,12 +23,14 @@ describe('components/Video', () => {
     }
   }
 
-  let component, video, onClick, mediaStream, url
+  let component, videos, video, onClick, mediaStream, url
   function render () {
+    videos = {}
     onClick = jest.fn()
     mediaStream = new MediaStream()
     component = TestUtils.renderIntoDocument(
       <VideoWrapper
+        videos={videos}
         active
         stream={{ mediaStream, url }}
         onClick={onClick}