Refactoring of code smells

.gitignore

@@ -28,10 +28,14 @@ yarn-error.log*
 .env
 .env.*
 .vscode
+*.csv
 
 test-reports
 junit.xml
 serviceAccountKey.json
 
 public/noisecancellation/
 public/virtualbackground/
+files_smells.csv
+components_smells.csv
+*.components_smells.csv

src/components/AudioLevelIndicator/AudioLevelIndicator.tsx

@@ -1,6 +1,6 @@
+import { interval } from 'd3-timer';
 import React, { useEffect, useRef, useState } from 'react';
 import { AudioTrack, LocalAudioTrack, RemoteAudioTrack } from 'twilio-video';
-import { interval } from 'd3-timer';
 import useIsTrackEnabled from '../../hooks/useIsTrackEnabled/useIsTrackEnabled';
 import useMediaStreamTrack from '../../hooks/useMediaStreamTrack/useMediaStreamTrack';
 
@@ -20,8 +20,7 @@ export function initializeAnalyser(stream: MediaStream) {
 
   audioSource.connect(analyser);
 
-  // Here we provide a way for the audioContext to be closed.
-  // Closing the audioContext allows the unused audioSource to be garbage collected.
+  // Here we provide a way for the audioContext to be closed. Closing the audioContext allows the unused audioSource to be garbage collected.
   stream.addEventListener('cleanup', () => {
     if (audioContext.state !== 'closed') {
       audioContext.close();
@@ -41,19 +40,14 @@ function AudioLevelIndicator({ audioTrack, color = 'white' }: { audioTrack?: Aud
 
   useEffect(() => {
     if (audioTrack && mediaStreamTrack && isTrackEnabled) {
-      // Here we create a new MediaStream from a clone of the mediaStreamTrack.
-      // A clone is created to allow multiple instances of this component for a single
-      // AudioTrack on iOS Safari. We only clone the mediaStreamTrack on iOS.
+      // Cloning the mediaStreamTrack allows for multiple instances of the AudioLevelIndicator component for a single AudioTrack on iOS Safari.
       let newMediaStream = new MediaStream([isIOS ? mediaStreamTrack.clone() : mediaStreamTrack]);
 
-      // Here we listen for the 'stopped' event on the audioTrack. When the audioTrack is stopped,
-      // we stop the cloned track that is stored in 'newMediaStream'. It is important that we stop
-      // all tracks when they are not in use. Browsers like Firefox don't let you create a new stream
-      // from a new audio device while the active audio device still has active tracks.
+      // It is important that we stop all tracks when they are not in use. Browsers like Firefox
+      // don't let you create a new stream from a new audio device while the active audio device still has active tracks.
       const stopAllMediaStreamTracks = () => {
         if (isIOS) {
           // If we are on iOS, then we want to stop the MediaStreamTrack that we have previously cloned.
-          // If we are not on iOS, then we do not stop the MediaStreamTrack since it is the original and still in use.
           newMediaStream.getTracks().forEach(track => track.stop());
         }
         newMediaStream.dispatchEvent(new Event('cleanup')); // Stop the audioContext
@@ -62,16 +56,14 @@ function AudioLevelIndicator({ audioTrack, color = 'white' }: { audioTrack?: Aud
 
       const reinitializeAnalyser = () => {
         stopAllMediaStreamTracks();
-        // We only clone the mediaStreamTrack on iOS.
+
         newMediaStream = new MediaStream([isIOS ? mediaStreamTrack.clone() : mediaStreamTrack]);
         setAnalyser(initializeAnalyser(newMediaStream));
       };
 
       setAnalyser(initializeAnalyser(newMediaStream));
 
       // Here we reinitialize the AnalyserNode on focus to avoid an issue in Safari
-      // where the analysers stop functioning when the user switches to a new tab
-      // and switches back to the app.
       window.addEventListener('focus', reinitializeAnalyser);
 
       return () => {
@@ -109,7 +101,6 @@ function AudioLevelIndicator({ audioTrack, color = 'white' }: { audioTrack?: Aud
     }
   }, [isTrackEnabled, analyser]);
 
-  // Each instance of this component will need a unique HTML ID
   const clipPathId = `audio-level-clip-${getUniqueClipId()}`;
 
   return isTrackEnabled ? (

src/components/ChatWindow/MessageList/MessageList.tsx

@@ -1,10 +1,10 @@
-import React from 'react';
 import { Message } from '@twilio/conversations';
+import React from 'react';
+import useVideoContext from '../../../hooks/useVideoContext/useVideoContext';
+import MediaMessage from './MediaMessage/MediaMessage';
 import MessageInfo from './MessageInfo/MessageInfo';
 import MessageListScrollContainer from './MessageListScrollContainer/MessageListScrollContainer';
 import TextMessage from './TextMessage/TextMessage';
-import useVideoContext from '../../../hooks/useVideoContext/useVideoContext';
-import MediaMessage from './MediaMessage/MediaMessage';
 
 interface MessageListProps {
   messages: Message[];
@@ -15,7 +15,8 @@ const getFormattedTime = (message?: Message) =>
 
 export default function MessageList({ messages }: MessageListProps) {
   const { room } = useVideoContext();
-  const localParticipant = room!.localParticipant;
+
+  if (!room) return <></>;
 
   return (
     <MessageListScrollContainer messages={messages}>
@@ -26,15 +27,17 @@ export default function MessageList({ messages }: MessageListProps) {
         // Display the MessageInfo component when the author or formatted timestamp differs from the previous message
         const shouldDisplayMessageInfo = time !== previousTime || message.author !== messages[idx - 1]?.author;
 
-        const isLocalParticipant = localParticipant.identity === message.author;
+        const isLocalParticipant = room.localParticipant.identity === message.author;
 
         return (
           <React.Fragment key={message.sid}>
-            {shouldDisplayMessageInfo && (
-              <MessageInfo author={message.author!} isLocalParticipant={isLocalParticipant} dateCreated={time} />
+            {shouldDisplayMessageInfo && message.author && (
+              <MessageInfo author={message.author} isLocalParticipant={isLocalParticipant} dateCreated={time} />
+            )}
+            {message.type === 'text' && message.body && (
+              <TextMessage body={message.body} isLocalParticipant={isLocalParticipant} />
             )}
-            {message.type === 'text' && <TextMessage body={message.body!} isLocalParticipant={isLocalParticipant} />}
-            {message.type === 'media' && <MediaMessage media={message.attachedMedia![0]} />}
+            {message.type === 'media' && message.attachedMedia && <MediaMessage media={message.attachedMedia[0]} />}
           </React.Fragment>
         );
       })}

src/components/ChatWindow/MessageList/MessageListScrollContainer/MessageListScrollContainer.tsx

@@ -1,11 +1,11 @@
 /* istanbul ignore file */
-import React from 'react';
-import ArrowDownwardIcon from '@material-ui/icons/ArrowDownward';
 import Button from '@material-ui/core/Button';
-import clsx from 'clsx';
+import { WithStyles, createStyles, withStyles } from '@material-ui/core/styles';
+import ArrowDownwardIcon from '@material-ui/icons/ArrowDownward';
 import { Message } from '@twilio/conversations';
+import clsx from 'clsx';
 import throttle from 'lodash.throttle';
-import { withStyles, WithStyles, createStyles } from '@material-ui/core/styles';
+import React, { useEffect, useRef, useState } from 'react';
 
 const styles = createStyles({
   outerContainer: {
@@ -45,12 +45,6 @@ interface MessageListScrollContainerProps extends WithStyles<typeof styles> {
   messages: Message[];
 }
 
-interface MessageListScrollContainerState {
-  isScrolledToBottom: boolean;
-  showButton: boolean;
-  messageNotificationCount: number;
-}
-
 /*
  * This component is a scrollable container that wraps around the 'MessageList' component.
  * The MessageList will ultimately grow taller than its container as it continues to receive
@@ -62,103 +56,81 @@ interface MessageListScrollContainerState {
  *
  * Note that this component is tested with Cypress only.
  */
-export class MessageListScrollContainer extends React.Component<
-  MessageListScrollContainerProps,
-  MessageListScrollContainerState
-> {
-  chatThreadRef = React.createRef<HTMLDivElement>();
-  state = { isScrolledToBottom: true, showButton: false, messageNotificationCount: 0 };
 
-  scrollToBottom() {
-    const innerScrollContainerEl = this.chatThreadRef.current!;
-    innerScrollContainerEl.scrollTop = innerScrollContainerEl!.scrollHeight;
-  }
+const MessageListScrollContainer: React.FC<MessageListScrollContainerProps> = ({ classes, messages, children }) => {
+  const chatThreadRef = useRef<HTMLDivElement>(null);
+  const [isScrolledToBottom, setIsScrolledToBottom] = useState(true);
+  const [showButton, setShowButton] = useState(false);
+  const [messageNotificationCount, setMessageNotificationCount] = useState(0);
 
-  componentDidMount() {
-    this.scrollToBottom();
-    this.chatThreadRef.current!.addEventListener('scroll', this.handleScroll);
-  }
-
-  // This component updates as users send new messages:
-  componentDidUpdate(prevProps: MessageListScrollContainerProps, prevState: MessageListScrollContainerState) {
-    const hasNewMessages = this.props.messages.length !== prevProps.messages.length;
-
-    if (prevState.isScrolledToBottom && hasNewMessages) {
-      this.scrollToBottom();
-    } else if (hasNewMessages) {
-      const numberOfNewMessages = this.props.messages.length - prevProps.messages.length;
-
-      this.setState(previousState => ({
-        // If there's at least one new message, show the 'new message' button:
-        showButton: !previousState.isScrolledToBottom,
-        // If 'new message' button is visible,
-        // messageNotificationCount will be the number of previously unread messages + the number of new messages.
-        // Otherwise, messageNotificationCount is set to 1:
-        messageNotificationCount: previousState.showButton
-          ? previousState.messageNotificationCount + numberOfNewMessages
-          : 1,
-      }));
-    }
-  }
+  const scrollToBottom = () => {
+    const innerScrollContainerEl = chatThreadRef.current;
+    if (!innerScrollContainerEl) return;
+    innerScrollContainerEl.scrollTop = innerScrollContainerEl.scrollHeight;
+  };
 
-  handleScroll = throttle(() => {
-    const innerScrollContainerEl = this.chatThreadRef.current!;
-    // Because this.handleScroll() is a throttled method,
-    // it's possible that it can be called after this component unmounts, and this element will be null.
-    // Therefore, if it doesn't exist, don't do anything:
+  const handleScroll = throttle(() => {
+    const innerScrollContainerEl = chatThreadRef.current;
     if (!innerScrollContainerEl) return;
 
-    // On systems using display scaling, scrollTop may return a decimal value, so we need to account for this in the
-    // "isScrolledToBottom" calculation.
-    const isScrolledToBottom =
+    const scrolledToBottom =
       Math.abs(
-        innerScrollContainerEl.clientHeight + innerScrollContainerEl.scrollTop - innerScrollContainerEl!.scrollHeight
+        innerScrollContainerEl.clientHeight + innerScrollContainerEl.scrollTop - innerScrollContainerEl.scrollHeight
       ) < 1;
 
-    this.setState(prevState => ({
-      isScrolledToBottom,
-      showButton: isScrolledToBottom ? false : prevState.showButton,
-    }));
+    setIsScrolledToBottom(scrolledToBottom);
+    if (!scrolledToBottom) setShowButton(true);
   }, 300);
 
-  handleClick = () => {
-    const innerScrollContainerEl = this.chatThreadRef.current!;
-
-    innerScrollContainerEl.scrollTo({ top: innerScrollContainerEl.scrollHeight, behavior: 'smooth' });
+  useEffect(() => {
+    scrollToBottom();
+    const innerScrollContainerEl = chatThreadRef.current;
+    if (innerScrollContainerEl) {
+      innerScrollContainerEl.addEventListener('scroll', handleScroll);
+    }
+    return () => {
+      if (innerScrollContainerEl) {
+        innerScrollContainerEl.removeEventListener('scroll', handleScroll);
+      }
+    };
+  }, []);
+
+  useEffect(() => {
+    const hasNewMessages = messages.length !== 0;
+    if (isScrolledToBottom && hasNewMessages) {
+      scrollToBottom();
+    } else if (hasNewMessages) {
+      const numberOfNewMessages = messages.length;
+      // If 'new message' btn is visible, messageNotificationCount will be the number of prev. unread msgs + the number of new msgs.
+      setShowButton(!isScrolledToBottom);
+      setMessageNotificationCount(showButton ? messageNotificationCount + numberOfNewMessages : 1);
+    }
+  }, [messages]);
 
-    this.setState({ showButton: false });
+  const handleClick = () => {
+    scrollToBottom();
+    setShowButton(false);
   };
 
-  componentWillUnmount() {
-    const innerScrollContainerEl = this.chatThreadRef.current!;
-
-    innerScrollContainerEl.removeEventListener('scroll', this.handleScroll);
-  }
-
-  render() {
-    const { classes } = this.props;
-
-    return (
-      <div className={classes.outerContainer}>
-        <div className={classes.innerScrollContainer} ref={this.chatThreadRef} data-cy-message-list-inner-scroll>
-          <div className={classes.messageListContainer}>
-            {this.props.children}
-            <Button
-              className={clsx(classes.button, { [classes.showButton]: this.state.showButton })}
-              onClick={this.handleClick}
-              startIcon={<ArrowDownwardIcon />}
-              color="primary"
-              variant="contained"
-              data-cy-new-message-button
-            >
-              {this.state.messageNotificationCount} new message
-              {this.state.messageNotificationCount > 1 && 's'}
-            </Button>
-          </div>
+  return (
+    <div className={classes.outerContainer}>
+      <div className={classes.innerScrollContainer} ref={chatThreadRef} data-cy-message-list-inner-scroll>
+        <div className={classes.messageListContainer}>
+          {children}
+          <Button
+            className={clsx(classes.button, { [classes.showButton]: showButton })}
+            onClick={handleClick}
+            startIcon={<ArrowDownwardIcon />}
+            color="primary"
+            variant="contained"
+            data-cy-new-message-button
+          >
+            {messageNotificationCount} new message{messageNotificationCount > 1 && 's'}
+          </Button>
         </div>
       </div>
-    );
-  }
-}
+    </div>
+  );
+};
 
 export default withStyles(styles)(MessageListScrollContainer);

src/components/GalleryView/GalleryView.tsx

@@ -1,17 +1,16 @@
-import React from 'react';
+import { IconButton, Theme, createStyles, makeStyles } from '@material-ui/core';
 import ArrowBack from '@material-ui/icons/ArrowBack';
 import ArrowForward from '@material-ui/icons/ArrowForward';
+import { Pagination } from '@material-ui/lab';
 import clsx from 'clsx';
 import { GALLERY_VIEW_ASPECT_RATIO, GALLERY_VIEW_MARGIN } from '../../constants';
-import { IconButton, makeStyles, createStyles, Theme } from '@material-ui/core';
-import { Pagination } from '@material-ui/lab';
-import Participant from '../Participant/Participant';
+import useDominantSpeaker from '../../hooks/useDominantSpeaker/useDominantSpeaker';
 import useGalleryViewLayout from '../../hooks/useGalleryViewLayout/useGalleryViewLayout';
-import useVideoContext from '../../hooks/useVideoContext/useVideoContext';
 import useParticipantsContext from '../../hooks/useParticipantsContext/useParticipantsContext';
-import { usePagination } from './usePagination/usePagination';
-import useDominantSpeaker from '../../hooks/useDominantSpeaker/useDominantSpeaker';
+import useVideoContext from '../../hooks/useVideoContext/useVideoContext';
 import { useAppState } from '../../state';
+import Participant from '../Participant/Participant';
+import { usePagination } from './usePagination/usePagination';
 
 const CONTAINER_GUTTER = '50px';
 
@@ -85,10 +84,10 @@ export function GalleryView() {
   const { galleryViewParticipants } = useParticipantsContext();
   const dominantSpeaker = useDominantSpeaker(true);
 
-  const { paginatedParticipants, setCurrentPage, currentPage, totalPages } = usePagination([
-    room!.localParticipant,
-    ...galleryViewParticipants,
-  ]);
+  const participants =
+    room && room.localParticipant ? [room.localParticipant, ...galleryViewParticipants] : [...galleryViewParticipants];
+
+  const { paginatedParticipants, setCurrentPage, currentPage, totalPages } = usePagination(participants);
 
   const galleryViewLayoutParticipantCount =
     currentPage === 1 ? paginatedParticipants.length : maxGalleryViewParticipants;
@@ -97,6 +96,10 @@ export function GalleryView() {
   const participantWidth = `${participantVideoWidth}px`;
   const participantHeight = `${Math.floor(participantVideoWidth * GALLERY_VIEW_ASPECT_RATIO)}px`;
 
+  if (!room) {
+    return <></>;
+  }
+
   return (
     <div className={classes.container}>
       <div className={clsx(classes.buttonContainer, classes.buttonContainerLeft)}>
@@ -136,7 +139,7 @@ export function GalleryView() {
           >
             <Participant
               participant={participant}
-              isLocalParticipant={participant === room!.localParticipant}
+              isLocalParticipant={participant === room.localParticipant}
               isDominantSpeaker={participant === dominantSpeaker}
             />
           </div>