Review 11: Chat Feature

[silviadebenedictis]

Overview

The focus for this code review will be centered around the chat page.
Please pay attention too:

• Javascript issues
• React components

Review Branch

review-xxx

Files to review

• Any files in the chat directory such as: ChatComponent.jsx, MessageForm.jsx, etc..

Checklists

• Architecture
• Design
• JS
• React
• Testing

Due date

Monday, April 15th 11:55pm

For more information

The review process is documented at: http://courses.ics.hawaii.edu/ics414s21/morea/review/reading-idpm-review.html

[silviadebenedictis]

ChatComponent.jsx

• JS-07: L132 Avoid console.logs

Chat

• There's two chat directories, one in ui dir and one in components dir with same file names. It would be better to merge or rename the directories to avoid confusion
• What happens after accepting or denying a message? Does accepting mean you can keep chatting?

[gavyngostage]

Yep, nearly duplicate chat directories should be merged.

[zhuochenggan]

Both ChatInterface.jsx

• JS-07: Avoid console.logs, Multiple console.log through these two files

[ThanhLy1]

ChatComponent.jsx

• Ref Usage: Properly utilize useRef for focusing messages, but ensure cleanup to prevent memory leaks, especially if references change frequently.
• Subscription Management: Ensure subscriptions (e.g., Meteor.subscribe) are managed correctly to avoid unnecessary re-renders or data fetching.
• State Management: Consider debouncing the setCurrentMessage function or managing rapid state changes better to optimize rendering performance.
• Error Handling: Implement robust error handling for Meteor method calls, especially in sendMessage and handleUserSelected functions, to enhance user experience.
• UI Feedback: Provide clear UI feedback for loading states (both conversations and messages) and error states to inform the user appropriately.
• Message Sorting: Verify the sorting logic in both messages and conversations to ensure that the UI always displays the latest information correctly.
• Accessibility: Improve accessibility, especially in dynamic content updates and focus management within the chat interface.
• Code Duplication: Reduce duplication and enhance modularity, possibly by breaking down the component into smaller subcomponents.
• Styling Consistency: Ensure consistent styling across different browsers and devices, especially for custom scroll areas and interactive elements.

Chatinterface.jsx

• Logging: Remove console.log statements for production readiness to keep the console clean and secure sensitive information.
• Error Handling: Expand error handling in Meteor.call for conversations.initiate to provide user feedback through UI notifications rather than just console errors.
• UseEffect Dependency: Ensure that useEffect hook's dependencies are properly managed; currently, it only depends on activeConversationId. If the Conversations data might change externally, consider additional dependencies or refetch strategies.
• State Management: Simplify state updates by ensuring that all state-related logic is contained and does not rely on external mutable references that could lead to bugs in asynchronous operations.
• UI Responsiveness: Verify that the layout (split into 1/3 and 2/3 columns) adjusts well to different screen sizes and enhances usability on mobile devices.
• Component Separation: Good use of component separation (ConversationsList, ConversationDetails, MessageForm) aids in maintainability. Ensure that each component properly handles its props and maintains its own state as needed.
• Accessibility: Improve accessibility features, such as keyboard navigability and ARIA attributes, especially in dynamic content like conversations and messages lists.
• Code Documentation: Add comments describing the logic and purpose of major blocks of code, especially where the code interacts with external APIs or performs complex state management.
• Performance Optimization: Consider memoizing components or using callbacks to optimize re-renders, especially for components like MessageForm that might re-render frequently due to state changes in the parent.

[joelvanta]

ChatComponent.jsx

• There are a lot of ESLint errors regarding unexpected console statement and alerts.
• There are several unresolved variables.
• There are invalid number of arguments.

MessageForm.jsx

• There are a lot of ESLint errors regarding unexpected console statement and alerts.