From ec31ea2a6bdf173c044fb707f491b7406f12c98f Mon Sep 17 00:00:00 2001
From: Jerko Steiner <jerko.steiner@gmail.com>
Date: Sun, 7 Jul 2019 18:37:08 +0800
Subject: [PATCH] Fix broken chat button, remove refs

---
 src/client/components/App.js                  | 47 ++++++++++++-------
 src/client/components/Chat.js                 | 25 ++++++----
 src/client/components/Toolbar.js              | 32 ++++---------
 .../components/__tests__/Toolbar-test.js      | 17 +++----
 4 files changed, 63 insertions(+), 58 deletions(-)

diff --git a/src/client/components/App.js b/src/client/components/App.js
index 2f9b7fe..955f003 100644
--- a/src/client/components/App.js
+++ b/src/client/components/App.js
@@ -1,10 +1,10 @@
-import Alerts, { AlertPropType } from './Alerts.js'
 import * as constants from '../constants.js'
-import Toolbar from './Toolbar.js'
-import Notifications, { NotificationPropTypes } from './Notifications.js'
+import Alerts, { AlertPropType } from './Alerts.js'
 import Chat, { MessagePropTypes } from './Chat.js'
+import Notifications, { NotificationPropTypes } from './Notifications.js'
 import PropTypes from 'prop-types'
 import React from 'react'
+import Toolbar from './Toolbar.js'
 import Video, { StreamPropType } from './Video.js'
 import _ from 'underscore'
 
@@ -25,10 +25,24 @@ export default class App extends React.PureComponent {
   constructor () {
     super()
     this.state = {
-      videos: {}
+      videos: {},
+      chatVisible: false
     }
-    this.chatRef = React.createRef()
-    this.toolbarRef = React.createRef()
+  }
+  handleShowChat = () => {
+    this.setState({
+      chatVisible: true
+    })
+  }
+  handleHideChat = () => {
+    this.setState({
+      chatVisible: false
+    })
+  }
+  handleToggleChat = () => {
+    return this.state.chatVisible
+      ? this.handleHideChat()
+      : this.handleShowChat()
   }
   componentDidMount () {
     const { init } = this.props
@@ -53,22 +67,21 @@ export default class App extends React.PureComponent {
     return (
       <div className="app">
         <Toolbar
-          chatRef={this.chatRef}
+          chatVisible={this.state.chatVisible}
           messages={messages}
+          onToggleChat={this.handleToggleChat}
           stream={streams[constants.ME]}
-          ref={this.toolbarRef}
         />
         <Alerts alerts={alerts} dismiss={dismissAlert} />
         <Notifications notifications={notifications} />
-        <div className="chat-container" ref={this.chatRef}>
-          <Chat
-            messages={messages}
-            videos={videos}
-            notify={notify}
-            sendMessage={sendMessage}
-            toolbarRef={this.toolbarRef}
-          />
-        </div>
+        <Chat
+          messages={messages}
+          notify={notify}
+          onClose={this.handleHideChat}
+          sendMessage={sendMessage}
+          videos={videos}
+          visible={this.state.chatVisible}
+        />
         <div className="videos">
           <Video
             videos={videos}
diff --git a/src/client/components/Chat.js b/src/client/components/Chat.js
index e32f619..cc4e428 100644
--- a/src/client/components/Chat.js
+++ b/src/client/components/Chat.js
@@ -1,7 +1,8 @@
+import Input from './Input.js'
 import PropTypes from 'prop-types'
 import React from 'react'
+import classnames from 'classnames'
 import socket from '../socket.js'
-import Input from './Input.js'
 
 export const MessagePropTypes = PropTypes.shape({
   userId: PropTypes.string.isRequired,
@@ -12,18 +13,20 @@ export const MessagePropTypes = PropTypes.shape({
 
 export default class Chat extends React.PureComponent {
   static propTypes = {
+    visible: PropTypes.bool.isRequired,
     messages: PropTypes.arrayOf(MessagePropTypes).isRequired,
-    videos: PropTypes.object.isRequired,
     notify: PropTypes.func.isRequired,
+    onClose: PropTypes.func.isRequired,
     sendMessage: PropTypes.func.isRequired,
-    toolbarRef: PropTypes.object.isRequired
+    videos: PropTypes.object.isRequired
   }
-  handleCloseChat = e => {
-    const { toolbarRef } = this.props
-    toolbarRef.chatButton.click()
+  constructor () {
+    super()
+    this.chatHistoryRef = React.createRef()
   }
   scrollToBottom = () => {
-    this.chatScroll.scrollTop = this.chatScroll.scrollHeight
+    const chatHistoryRef = this.chatHistoryRef.current
+    chatHistoryRef.scrollTop = chatHistoryRef.scrollHeight
   }
   componentDidMount () {
     this.scrollToBottom()
@@ -34,16 +37,18 @@ export default class Chat extends React.PureComponent {
   render () {
     const { messages, videos, notify, sendMessage } = this.props
     return (
-      <div>
+      <div className={classnames('chat-container', {
+        show: this.props.visible
+      })}>
         <div className="chat-header">
-          <div className="chat-close" onClick={this.handleCloseChat}>
+          <div className="chat-close" onClick={this.props.onClose}>
             <div className="button button-icon">
               <span className="icon icon-arrow_forward" />
             </div>
           </div>
           <div className="chat-title">Chat</div>
         </div>
-        <div className="chat-history" ref={div => { this.chatScroll = div }}>
+        <div className="chat-history" ref={this.chatHistoryRef}>
 
           {messages.length ? (
             messages.map((message, i) => (
diff --git a/src/client/components/Toolbar.js b/src/client/components/Toolbar.js
index c07b9c4..65c52f9 100644
--- a/src/client/components/Toolbar.js
+++ b/src/client/components/Toolbar.js
@@ -1,30 +1,16 @@
 import PropTypes from 'prop-types'
 import React from 'react'
+import classnames from 'classnames'
 import screenfull from 'screenfull'
 import { MessagePropTypes } from './Chat.js'
 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
-  }
-  constructor () {
-    super()
-    this.state = {
-      isChatOpen: false,
-      totalMessages: 0
-    }
-  }
-  handleChatClick = () => {
-    const { chatRef, messages } = this.props
-    chatRef.classList.toggle('show')
-    this.chatButton.classList.toggle('on')
-    this.setState({
-      isChatOpen: chatRef.classList.contains('show'),
-      totalMessages: messages.length
-    })
+    stream: StreamPropType,
+    onToggleChat: PropTypes.func.isRequired,
+    chatVisible: PropTypes.bool.isRequired
   }
   handleMicClick = () => {
     const { stream } = this.props
@@ -51,14 +37,14 @@ export default class Toolbar extends React.PureComponent {
   }
   render () {
     const { messages, stream } = this.props
-    const { isChatOpen, totalMessages } = this.state
 
     return (
       <div className="toolbar active">
-        <div onClick={this.handleChatClick}
-          ref={node => { this.chatButton = node }}
-          className="button chat"
-          data-blink={messages.length !== totalMessages && !isChatOpen}
+        <div onClick={this.props.onToggleChat}
+          className={classnames('button chat', {
+            on: this.props.chatVisible
+          })}
+          data-blink={this.props.chatVisible && messages.length}
           title="Chat"
         >
           <span className="icon icon-question_answer" />
diff --git a/src/client/components/__tests__/Toolbar-test.js b/src/client/components/__tests__/Toolbar-test.js
index 015e996..6b0d680 100644
--- a/src/client/components/__tests__/Toolbar-test.js
+++ b/src/client/components/__tests__/Toolbar-test.js
@@ -5,7 +5,7 @@ import TestUtils from 'react-dom/test-utils'
 import Toolbar from '../Toolbar.js'
 import { MediaStream } from '../../window.js'
 
-describe('components/Video', () => {
+describe('components/Toolbar', () => {
 
   class ToolbarWrapper extends React.PureComponent {
     static propTypes = Toolbar.propTypes
@@ -15,22 +15,22 @@ describe('components/Video', () => {
     }
     render () {
       return <Toolbar
-        chatRef={this.props.chatRef}
+        chatVisible={this.props.chatVisible}
+        onToggleChat={this.props.onToggleChat}
         messages={this.props.messages}
         stream={this.state.stream || this.props.stream}
       />
     }
   }
 
-  let component, node, chatRef, mediaStream, url
+  let component, node, mediaStream, url, onToggleChat
   function render () {
     mediaStream = new MediaStream()
-    chatRef = ReactDOM.findDOMNode(
-      TestUtils.renderIntoDocument(<div />)
-    )
+    onToggleChat = jest.fn()
     component = TestUtils.renderIntoDocument(
       <ToolbarWrapper
-        chatRef={chatRef}
+        chatVisible
+        onToggleChat={onToggleChat}
         messages={[]}
         stream={{ mediaStream, url }}
       />
@@ -46,9 +46,10 @@ describe('components/Video', () => {
 
   describe('handleChatClick', () => {
     it('toggle chat', () => {
+      expect(onToggleChat.mock.calls.length).toBe(0)
       const button = node.querySelector('.chat')
       TestUtils.Simulate.click(button)
-      expect(button.classList.contains('on')).toBe(true)
+      expect(onToggleChat.mock.calls.length).toBe(1)
     })
   })