From 16be9dea8349d076ea67783ab25ca261f45b7a2d Mon Sep 17 00:00:00 2001 From: Alan Friedman Date: Thu, 11 Jun 2020 08:43:51 -0400 Subject: [PATCH] Revert "Rework Home component phase 2" (#152) This reverts commit db6831603123dfdaf763bdf2f0fae85f044b99a9. --- client/src/components/Home/Activity.js | 243 ++++++++++-------- client/src/components/Home/ActivityList.js | 150 ++++++----- .../src/components/Home/ActivityList.test.js | 95 ++++--- client/src/components/Home/Home.js | 20 +- client/src/components/Home/index.js | 3 + 5 files changed, 289 insertions(+), 222 deletions(-) diff --git a/client/src/components/Home/Activity.js b/client/src/components/Home/Activity.js index 61cd116..a531c51 100644 --- a/client/src/components/Home/Activity.js +++ b/client/src/components/Home/Activity.js @@ -1,4 +1,4 @@ -import React from 'react' +import React, { Component } from 'react' import PropTypes from 'prop-types' import Message from 'components/Message' import Username from 'components/Username' @@ -8,128 +8,143 @@ import { getObjectUrl } from 'utils/file' import T from 'components/T' -const FileDisplay = ({ activity: { fileType, encodedFile, fileName, username }, scrollToBottom }) => { - const zoomableImage = React.useRef(null) +class Activity extends Component { + constructor(props) { + super(props) - const handleImageDisplay = () => { - Zoom(zoomableImage.current) - scrollToBottom() + this.state = { + zoomableImages: [], + } } - if (fileType.match('image.*')) { - return ( - {`${fileName} - ) - } - return null -} - -const Activity = ({ activity, scrollToBottom }) => { - switch (activity.type) { - case 'TEXT_MESSAGE': + getFileDisplay(activity) { + const type = activity.fileType + if (type.match('image.*')) { return ( - this._zoomableImage = c} + className="image-transfer zoomable" + src={`data:${activity.fileType};base64,${activity.encodedFile}`} + alt={`${activity.fileName} from ${activity.username}`} + onLoad={this.handleImageDisplay.bind(this)} /> ) - case 'USER_ENTER': - return ( - -
- - }} path='userJoined'/> -
- - ) - case 'USER_EXIT': - return ( - -
- - }} path='userLeft'/> -
- - ) - case 'TOGGLE_LOCK_ROOM': - if (activity.locked) { - return ( - -
- }} path='lockedRoom'/>
- - ) - } else { - return ( - -
- }} path='unlockedRoom'/>
- - ) - } - case 'NOTICE': - return ( - -
{activity.message}
- - ) - case 'CHANGE_USERNAME': - return ( - -
, - newUsername: - }} path='nameChange'/> -
- - ) - case 'USER_ACTION': - return ( - -
* {activity.action}
- - ) - case 'RECEIVE_FILE': - const downloadUrl = getObjectUrl(activity.encodedFile, activity.fileType) - return ( -
- , - }} path='userSentFile'/>  + } + return null + } - - - - -
- ) - case 'SEND_FILE': - const url = getObjectUrl(activity.encodedFile, activity.fileType) - return ( - + handleImageDisplay() { + Zoom(this._zoomableImage) + this.props.scrollToBottom() + } + + getActivityComponent(activity) { + switch (activity.type) { + case 'TEXT_MESSAGE': + return ( + + ) + case 'USER_ENTER': + return ( + +
+ + }} path='userJoined'/> +
+ + ) + case 'USER_EXIT': + return ( + +
+ + }} path='userLeft'/> +
+ + ) + case 'TOGGLE_LOCK_ROOM': + if (activity.locked) { + return ( + +
+ }} path='lockedRoom'/>
+ + ) + } else { + return ( + +
+ }} path='unlockedRoom'/>
+ + ) + } + case 'NOTICE': + return ( + +
{activity.message}
+ + ) + case 'CHANGE_USERNAME': + return ( + +
, + newUsername: + }} path='nameChange'/> +
+ + ) + case 'USER_ACTION': + return ( + +
* {activity.action}
+ + ) + case 'RECEIVE_FILE': + const downloadUrl = getObjectUrl(activity.encodedFile, activity.fileType) + return (
{activity.fileName}, - }} path='sentFile'/>  + username: , + }} path='userSentFile'/>  + + + + + {this.getFileDisplay(activity)}
- - - ) - default: - return false + ) + case 'SEND_FILE': + const url = getObjectUrl(activity.encodedFile, activity.fileType) + return ( + +
+ {activity.fileName}, + }} path='sentFile'/>  +
+ {this.getFileDisplay(activity)} + + ) + default: + return false + } + } + + render() { + return ( + this.getActivityComponent(this.props.activity) + ) } } @@ -138,4 +153,4 @@ Activity.propTypes = { scrollToBottom: PropTypes.func.isRequired, } -export default Activity +export default Activity; diff --git a/client/src/components/Home/ActivityList.js b/client/src/components/Home/ActivityList.js index 59c0124..1e4f019 100644 --- a/client/src/components/Home/ActivityList.js +++ b/client/src/components/Home/ActivityList.js @@ -1,90 +1,100 @@ -import React from 'react' +import React, { Component } from 'react' import PropTypes from 'prop-types' import ChatInput from 'components/Chat' -import Activity from './Activity' -import T from 'components/T' import { defer } from 'lodash' +import Activity from './Activity' + +import T from 'components/T' import styles from './styles.module.scss' -const ActivityList = ({ activities, openModal }) => { - const [focusChat, setFocusChat] = React.useState(false); - const [scrolledToBottom, setScrolledToBottom] = React.useState(true); - const messageStream = React.useRef(null); - const activitiesList = React.useRef(null); +class ActivityList extends Component { + constructor(props) { + super(props) - React.useEffect(() => { - const currentMessageStream = messageStream.current; - - // Update scrolledToBottom state if we scroll the activity stream - const onScroll = () => { - const messageStreamHeight = messageStream.current.clientHeight - const activitiesListHeight = activitiesList.current.clientHeight - - const bodyRect = document.body.getBoundingClientRect() - const elemRect = activitiesList.current.getBoundingClientRect() - const offset = elemRect.top - bodyRect.top - const activitiesListYPos = offset - - const newScrolledToBottom = (activitiesListHeight + (activitiesListYPos - 60)) <= messageStreamHeight - if (newScrolledToBottom) { - if (!scrolledToBottom) { - setScrolledToBottom(true) - } - } else if (scrolledToBottom) { - setScrolledToBottom(false) - } + this.state = { + zoomableImages: [], + focusChat: false, } - - currentMessageStream.addEventListener('scroll', onScroll) - return () => { - // Unbind event if component unmounted - currentMessageStream.removeEventListener('scroll', onScroll) - } - }, [scrolledToBottom]) - - const scrollToBottomIfShould = React.useCallback(() => { - if (scrolledToBottom) { - messageStream.current.scrollTop = messageStream.current.scrollHeight - } - }, [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)) } - return ( -
-
-
    -

    • - {activities.map((activity, index) => ( + componentDidMount() { + this.bindEvents() + } + + 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 offset = elemRect.top - bodyRect.top + const activitiesListYPos = offset + + const scrolledToBottom = (activitiesListHeight + (activitiesListYPos - 60)) <= messageStreamHeight + if (scrolledToBottom) { + if (!this.props.scrolledToBottom) { + this.props.setScrolledToBottom(true) + } + } else if (this.props.scrolledToBottom) { + this.props.setScrolledToBottom(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 ( +
    +
    this.messageStream = el} data-testid="main-div"> +
      this.activitiesList = el}> +

      • + {this.props.activities.map((activity, index) => (
    • - +
      • - ))} -
    + ))} +
+
+
+ +
-
- -
- - ) + ) + } } ActivityList.propTypes = { activities: PropTypes.array.isRequired, openModal: PropTypes.func.isRequired, + setScrolledToBottom: PropTypes.func.isRequired, + scrolledToBottom: PropTypes.bool.isRequired, } -export default ActivityList +export default ActivityList; diff --git a/client/src/components/Home/ActivityList.test.js b/client/src/components/Home/ActivityList.test.js index ebe234c..af1c769 100644 --- a/client/src/components/Home/ActivityList.test.js +++ b/client/src/components/Home/ActivityList.test.js @@ -1,5 +1,5 @@ import React from 'react'; -import { render, fireEvent } from '@testing-library/react'; +import { render, fireEvent, waitFor } 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( - + , ); @@ -39,7 +39,7 @@ describe('ActivityList component', () => { ]; const { asFragment } = render( - + , ); @@ -51,7 +51,7 @@ describe('ActivityList component', () => { const { getByText } = render( - + , ); @@ -59,33 +59,79 @@ describe('ActivityList component', () => { jest.runAllTimers(); expect(mockOpenModal.mock.calls[0][0]).toBe('About'); - jest.runAllTimers(); }); it('should focus chat', () => { const { getByTestId } = render( - + , ); fireEvent.click(getByTestId('main-div')); jest.runAllTimers(); }); - it('should scroll to bottom on new message if not scrolled', () => { - jest.spyOn(Element.prototype, 'clientHeight', 'get').mockReturnValueOnce(400).mockReturnValueOnce(200); + it('should handle scroll', () => { + const mockSetScrollToBottom = jest.fn(); + 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( + + + , + ); + fireEvent.scroll(getByTestId('main-div')); + + expect(mockSetScrollToBottom).toHaveBeenLastCalledWith(true); + + rerender( + + + , + ); + + 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, getByTestId } = render( + const { rerender } = render( - + , ); @@ -101,39 +147,14 @@ describe('ActivityList component', () => { text: 'Hi!', }, ]} + setScrolledToBottom={mockSetScrollToBottom} + scrolledToBottom={true} /> , ); jest.runAllTimers(); - expect(mockScrollTop).toHaveBeenCalledTimes(2); expect(mockScrollTop).toHaveBeenLastCalledWith(42); - - fireEvent.scroll(getByTestId('main-div')); - - rerender( - - - , - ); - - expect(mockScrollTop).toHaveBeenCalledTimes(2); }); }); diff --git a/client/src/components/Home/Home.js b/client/src/components/Home/Home.js index a96fb65..ef2761b 100644 --- a/client/src/components/Home/Home.js +++ b/client/src/components/Home/Home.js @@ -11,6 +11,7 @@ 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' @@ -23,6 +24,13 @@ 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) @@ -137,6 +145,7 @@ class Home extends Component { } bindEvents() { + window.onfocus = () => { this.props.toggleWindowFocus(true) } @@ -166,6 +175,11 @@ class Home extends Component { }) } + handleChatClick() { + this.setState({ focusChat: true }) + defer(() => this.setState({ focusChat: false })) + } + render() { const modalOpts = this.getModal() return ( @@ -187,9 +201,11 @@ class Home extends Component { translations={this.props.translations} /> - { 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, @@ -41,6 +43,7 @@ const mapDispatchToProps = { createUser, openModal, closeModal, + setScrolledToBottom, toggleWindowFocus, toggleSoundEnabled, toggleSocketConnected,