Revert "Revert "Rework Home component phase 2" (#152)" (#153)

This reverts commit 16be9dea8349d076ea67783ab25ca261f45b7a2d.

client/src/components/Home/Activity.js

@@ -1,4 +1,4 @@
-import React, { Component } from 'react'
+import React from 'react'
 import PropTypes from 'prop-types'
 import Message from 'components/Message'
 import Username from 'components/Username'
@@ -8,37 +8,29 @@ import { getObjectUrl } from 'utils/file'
 
 import T from 'components/T'
 
-class Activity extends Component {
+const FileDisplay = ({ activity: { fileType, encodedFile, fileName, username }, scrollToBottom }) => {
-  constructor(props) {
+  const zoomableImage = React.useRef(null)
-    super(props)
 
-    this.state = {
+  const handleImageDisplay = () => {
-      zoomableImages: [],
+    Zoom(zoomableImage.current)
-    }
+    scrollToBottom()
   }
 
-  getFileDisplay(activity) {
+  if (fileType.match('image.*')) {
-    const type = activity.fileType
-    if (type.match('image.*')) {
     return (
       <img
-          ref={c => this._zoomableImage = c}
+        ref={zoomableImage}
         className="image-transfer zoomable"
-          src={`data:${activity.fileType};base64,${activity.encodedFile}`}
+        src={`data:${fileType};base64,${encodedFile}`}
-          alt={`${activity.fileName} from ${activity.username}`}
+        alt={`${fileName} from ${username}`}
-          onLoad={this.handleImageDisplay.bind(this)}
+        onLoad={handleImageDisplay}
       />
     )
   }
   return null
-  }
+}
 
-  handleImageDisplay() {
+const Activity = ({ activity, scrollToBottom }) => {
-    Zoom(this._zoomableImage)
-    this.props.scrollToBottom()
-  }
-
-  getActivityComponent(activity) {
   switch (activity.type) {
     case 'TEXT_MESSAGE':
       return (
@@ -116,12 +108,12 @@ class Activity extends Component {
             username: <Username key={0} username={activity.username} />,
           }} path='userSentFile'/>&nbsp;
 
-            <a target="_blank" href={downloadUrl} rel="noopener noreferrer" download={activity.fileName}>
+          <a target="_blank" href={downloadUrl} rel="noopener noreferrer" download={activity.fileName} >
             <T data={{
               filename: activity.fileName,
             }} path='downloadFile'/>
           </a>
-            {this.getFileDisplay(activity)}
+          <FileDisplay activity={activity} scrollToBottom={scrollToBottom} />
         </div>
       )
     case 'SEND_FILE':
@@ -133,19 +125,12 @@ class Activity extends Component {
               filename: <a key={0} target="_blank" href={url} rel="noopener noreferrer" download={activity.fileName}>{activity.fileName}</a>,
             }} path='sentFile'/>&nbsp;
           </div>
-            {this.getFileDisplay(activity)}
+          <FileDisplay activity={activity} scrollToBottom={scrollToBottom} />
         </Notice>
       )
     default:
       return false
   }
-  }
-
-  render() {
-    return (
-      this.getActivityComponent(this.props.activity)
-    )
-  }
 }
 
 Activity.propTypes = {
@@ -153,4 +138,4 @@ Activity.propTypes = {
   scrollToBottom: PropTypes.func.isRequired,
 }
 
-export default Activity;
+export default Activity

client/src/components/Home/ActivityList.js

@@ -1,100 +1,90 @@
-import React, { Component } from 'react'
+import React from 'react'
 import PropTypes from 'prop-types'
 import ChatInput from 'components/Chat'
-import { defer } from 'lodash'
 import Activity from './Activity'
-
 import T from 'components/T'
+import { defer } from 'lodash'
 
 import styles from './styles.module.scss'
 
-class ActivityList extends Component {
+const ActivityList = ({ activities, openModal }) => {
-  constructor(props) {
+  const [focusChat, setFocusChat] = React.useState(false);
-    super(props)
+  const [scrolledToBottom, setScrolledToBottom] = React.useState(true);
+  const messageStream = React.useRef(null);
+  const activitiesList = React.useRef(null);
 
-    this.state = {
+  React.useEffect(() => {
-      zoomableImages: [],
+    const currentMessageStream = messageStream.current;
-      focusChat: false,
-    }
-  }
 
-  componentDidMount() {
+    // Update scrolledToBottom state if we scroll the activity stream
-    this.bindEvents()
+    const onScroll = () => {
-  }
+      const messageStreamHeight = messageStream.current.clientHeight
-
+      const activitiesListHeight = activitiesList.current.clientHeight
-  componentDidUpdate(prevProps) {
-    if (prevProps.activities.length < this.props.activities.length) {
-      this.scrollToBottomIfShould()
-    }
-  }
-
-  onScroll() {
-    const messageStreamHeight = this.messageStream.clientHeight
-    const activitiesListHeight = this.activitiesList.clientHeight
 
       const bodyRect = document.body.getBoundingClientRect()
-    const elemRect = this.activitiesList.getBoundingClientRect()
+      const elemRect = activitiesList.current.getBoundingClientRect()
       const offset = elemRect.top - bodyRect.top
       const activitiesListYPos = offset
 
-    const scrolledToBottom = (activitiesListHeight + (activitiesListYPos - 60)) <= messageStreamHeight
+      const newScrolledToBottom = (activitiesListHeight + (activitiesListYPos - 60)) <= messageStreamHeight
+      if (newScrolledToBottom) {
+        if (!scrolledToBottom) {
+          setScrolledToBottom(true)
+        }
+      } else if (scrolledToBottom) {
+        setScrolledToBottom(false)
+      }
+    }
+
+    currentMessageStream.addEventListener('scroll', onScroll)
+    return () => {
+      // Unbind event if component unmounted
+      currentMessageStream.removeEventListener('scroll', onScroll)
+    }
+  }, [scrolledToBottom])
+
+  const scrollToBottomIfShould = React.useCallback(() => {
     if (scrolledToBottom) {
-      if (!this.props.scrolledToBottom) {
+      messageStream.current.scrollTop = messageStream.current.scrollHeight
-        this.props.setScrolledToBottom(true)
-      }
-    } else if (this.props.scrolledToBottom) {
-      this.props.setScrolledToBottom(false)
     }
+  }, [scrolledToBottom])
+
+  React.useEffect(() => {
+    scrollToBottomIfShould(); // Only if activities.length bigger
+  }, [scrollToBottomIfShould, activities]);
+
+  const scrollToBottom = React.useCallback(() => {
+    messageStream.current.scrollTop = messageStream.current.scrollHeight
+    setScrolledToBottom(true)
+  }, [])
+
+  const handleChatClick = () => {
+    setFocusChat(true);
+    defer(() => setFocusChat(false))
   }
 
-  scrollToBottomIfShould() {
-    if (this.props.scrolledToBottom) {
-      setTimeout(() => {
-        this.messageStream.scrollTop = this.messageStream.scrollHeight
-      }, 0)
-    }
-  }
-
-  scrollToBottom() {
-    this.messageStream.scrollTop = this.messageStream.scrollHeight
-    this.props.setScrolledToBottom(true)
-  }
-
-  bindEvents() {
-    this.messageStream.addEventListener('scroll', this.onScroll.bind(this))
-  }
-
-  handleChatClick() {
-    this.setState({ focusChat: true })
-    defer(() => this.setState({ focusChat: false }))
-  }
-
-  render() {
   return (
     <div className="main-chat">
-        <div onClick={this.handleChatClick.bind(this)} className="message-stream h-100" ref={el => this.messageStream = el} data-testid="main-div">
+      <div onClick={handleChatClick} className="message-stream h-100" ref={messageStream} data-testid="main-div">
-          <ul className="plain" ref={el => this.activitiesList = el}>
+        <ul className="plain" ref={activitiesList}>
-            <li><p className={styles.tos}><button className='btn btn-link' onClick={this.props.openModal.bind(this, 'About')}> <T path='agreement'/></button></p></li>
+          <li><p className={styles.tos}><button className='btn btn-link' onClick={() => openModal('About')}> <T path='agreement'/></button></p></li>
-            {this.props.activities.map((activity, index) => (
+          {activities.map((activity, index) => (
             <li key={index} className={`activity-item ${activity.type}`}>
-                <Activity activity={activity} scrollToBottom={this.scrollToBottomIfShould.bind(this)} />
+              <Activity activity={activity} scrollToBottom={scrollToBottomIfShould} />
             </li>
           ))}
         </ul>
       </div>
       <div className="chat-container">
-          <ChatInput scrollToBottom={this.scrollToBottom.bind(this)} focusChat={this.state.focusChat} />
+        <ChatInput scrollToBottom={scrollToBottom} focusChat={focusChat} />
       </div>
     </div>
   )
-  }
 }
 
 ActivityList.propTypes = {
   activities: PropTypes.array.isRequired,
   openModal: PropTypes.func.isRequired,
-  setScrolledToBottom: PropTypes.func.isRequired,
-  scrolledToBottom: PropTypes.bool.isRequired,
 }
 
-export default ActivityList;
+export default ActivityList

client/src/components/Home/ActivityList.test.js

@@ -1,5 +1,5 @@
 import React from 'react';
-import { render, fireEvent, waitFor } from '@testing-library/react';
+import { render, fireEvent  } from '@testing-library/react';
 import ActivityList from './ActivityList';
 import { Provider } from 'react-redux';
 import configureStore from 'store';
@@ -12,7 +12,7 @@ describe('ActivityList component', () => {
   it('should display', () => {
     const { asFragment } = render(
       <Provider store={store}>
-        <ActivityList openModal={jest.fn()} activities={[]} setScrolledToBottom={jest.fn()} scrolledToBottom />
+        <ActivityList openModal={jest.fn()} activities={[]} />
       </Provider>,
     );
 
@@ -39,7 +39,7 @@ describe('ActivityList component', () => {
     ];
     const { asFragment } = render(
       <Provider store={store}>
-        <ActivityList openModal={jest.fn()} activities={activities} setScrolledToBottom={jest.fn()} scrolledToBottom />
+        <ActivityList openModal={jest.fn()} activities={activities} />
       </Provider>,
     );
 
@@ -51,7 +51,7 @@ describe('ActivityList component', () => {
 
     const { getByText } = render(
       <Provider store={store}>
-        <ActivityList openModal={mockOpenModal} activities={[]} setScrolledToBottom={jest.fn()} scrolledToBottom />
+        <ActivityList openModal={mockOpenModal} activities={[]} />
       </Provider>,
     );
 
@@ -59,79 +59,33 @@ describe('ActivityList component', () => {
     jest.runAllTimers();
 
     expect(mockOpenModal.mock.calls[0][0]).toBe('About');
+    jest.runAllTimers();
   });
 
   it('should focus chat', () => {
     const { getByTestId } = render(
       <Provider store={store}>
-        <ActivityList openModal={jest.fn()} activities={[]} setScrolledToBottom={jest.fn()} scrolledToBottom />
+        <ActivityList openModal={jest.fn()} activities={[]} />
       </Provider>,
     );
     fireEvent.click(getByTestId('main-div'));
     jest.runAllTimers();
   });
 
-  it('should handle scroll', () => {
+  it('should scroll to bottom on new message if not scrolled', () => {
-    const mockSetScrollToBottom = jest.fn();
+    jest.spyOn(Element.prototype, 'clientHeight', 'get').mockReturnValueOnce(400).mockReturnValueOnce(200);
-    jest
-      .spyOn(Element.prototype, 'clientHeight', 'get')
-      .mockReturnValueOnce(400)
-      .mockReturnValueOnce(200)
-      .mockReturnValueOnce(400)
-      .mockReturnValueOnce(200);
 
     Element.prototype.getBoundingClientRect = jest
       .fn()
       .mockReturnValueOnce({ top: 0 })
-      .mockReturnValueOnce({ top: 60 })
-      .mockReturnValueOnce({ top: 0 })
       .mockReturnValueOnce({ top: 261 });
 
-    const { getByTestId, rerender } = render(
-      <Provider store={store}>
-        <ActivityList
-          openModal={jest.fn()}
-          activities={[]}
-          setScrolledToBottom={mockSetScrollToBottom}
-          scrolledToBottom={false}
-        />
-      </Provider>,
-    );
-    fireEvent.scroll(getByTestId('main-div'));
-
-    expect(mockSetScrollToBottom).toHaveBeenLastCalledWith(true);
-
-    rerender(
-      <Provider store={store}>
-        <ActivityList
-          openModal={jest.fn()}
-          activities={[]}
-          setScrolledToBottom={mockSetScrollToBottom}
-          scrolledToBottom={true}
-        />
-      </Provider>,
-    );
-
-    fireEvent.scroll(getByTestId('main-div'));
-
-    expect(mockSetScrollToBottom).toHaveBeenCalledTimes(2);
-    expect(mockSetScrollToBottom).toHaveBeenLastCalledWith(false);
-  });
-
-  it('should scroll to bottom on new message', () => {
-    const mockSetScrollToBottom = jest.fn();
-
     jest.spyOn(Element.prototype, 'scrollHeight', 'get').mockReturnValue(42);
     const mockScrollTop = jest.spyOn(Element.prototype, 'scrollTop', 'set');
 
-    const { rerender } = render(
+    const { rerender, getByTestId } = render(
       <Provider store={store}>
-        <ActivityList
+        <ActivityList openModal={jest.fn()} activities={[]} />
-          openModal={jest.fn()}
-          activities={[]}
-          setScrolledToBottom={mockSetScrollToBottom}
-          scrolledToBottom={true}
-        />
       </Provider>,
     );
 
@@ -147,14 +101,39 @@ describe('ActivityList component', () => {
               text: 'Hi!',
             },
           ]}
-          setScrolledToBottom={mockSetScrollToBottom}
-          scrolledToBottom={true}
         />
       </Provider>,
     );
 
     jest.runAllTimers();
 
+    expect(mockScrollTop).toHaveBeenCalledTimes(2);
     expect(mockScrollTop).toHaveBeenLastCalledWith(42);
+
+    fireEvent.scroll(getByTestId('main-div'));
+
+    rerender(
+      <Provider store={store}>
+        <ActivityList
+          openModal={jest.fn()}
+          activities={[
+            {
+              type: 'TEXT_MESSAGE',
+              username: 'alice',
+              timestamp: new Date('2020-03-14T11:01:58.135Z').valueOf(),
+              text: 'Hi!',
+            },
+            {
+              type: 'TEXT_MESSAGE',
+              username: 'alice',
+              timestamp: new Date('2020-03-14T11:01:59.135Z').valueOf(),
+              text: 'Hi! every body',
+            },
+          ]}
+        />
+      </Provider>,
+    );
+
+    expect(mockScrollTop).toHaveBeenCalledTimes(2);
   });
 });

client/src/components/Home/Home.js

@@ -11,7 +11,6 @@ import Settings from 'components/Settings'
 import Welcome from 'components/Welcome'
 import RoomLocked from 'components/RoomLocked'
 import { X, AlertCircle } from 'react-feather'
-import { defer } from 'lodash'
 import Tinycon from 'tinycon'
 import beepFile from 'audio/beep.mp3'
 import classNames from 'classnames'
@@ -24,13 +23,6 @@ const crypto = new Crypto()
 Modal.setAppElement('#root');
 
 class Home extends Component {
-  constructor(props) {
-    super(props)
-
-    this.state = {
-      zoomableImages: []
-    }
-  }
 
   async componentWillMount() {
     const roomId = encodeURI(this.props.match.params.roomId)
@@ -145,7 +137,6 @@ class Home extends Component {
   }
 
   bindEvents() {
-
     window.onfocus = () => {
       this.props.toggleWindowFocus(true)
     }
@@ -175,11 +166,6 @@ class Home extends Component {
     })
   }
 
-  handleChatClick() {
-    this.setState({ focusChat: true })
-    defer(() => this.setState({ focusChat: false }))
-  }
-
   render() {
     const modalOpts = this.getModal()
     return (
@@ -204,8 +190,6 @@ class Home extends Component {
         <ActivityList
           openModal={this.props.openModal}
           activities={this.props.activities}
-          setScrolledToBottom={this.props.setScrolledToBottom}
-          scrolledToBottom={this.props.scrolledToBottom}
         />
         <Modal
           isOpen={Boolean(this.props.modalComponent)}
@@ -261,8 +245,6 @@ Home.propTypes = {
   modalComponent: PropTypes.string,
   openModal: PropTypes.func.isRequired,
   closeModal: PropTypes.func.isRequired,
-  setScrolledToBottom: PropTypes.func.isRequired,
-  scrolledToBottom: PropTypes.bool.isRequired,
   iAmOwner: PropTypes.bool.isRequired,
   userId: PropTypes.string.isRequired,
   toggleWindowFocus: PropTypes.func.isRequired,

client/src/components/Home/index.js

@@ -5,7 +5,6 @@ import {
   createUser,
   openModal,
   closeModal,
-  setScrolledToBottom,
   toggleWindowFocus,
   toggleSoundEnabled,
   toggleSocketConnected,
@@ -28,7 +27,6 @@ const mapStateToProps = (state) => {
     roomId: state.room.id,
     roomLocked: state.room.isLocked,
     modalComponent: state.app.modalComponent,
-    scrolledToBottom: state.app.scrolledToBottom,
     iAmOwner: Boolean(me && me.isOwner),
     faviconCount: state.app.unreadMessageCount,
     soundIsEnabled: state.app.soundIsEnabled,
@@ -43,7 +41,6 @@ const mapDispatchToProps = {
   createUser,
   openModal,
   closeModal,
-  setScrolledToBottom,
   toggleWindowFocus,
   toggleSoundEnabled,
   toggleSocketConnected,