Complete chat app with rooms and history#173
Complete chat app with rooms and history#173gmarchese93 wants to merge 3 commits intomate-academy:masterfrom
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great start on the chat application! The client-side UI for sending and viewing messages is well-implemented. However, some changes are needed because the application is currently non-functional due to critical server-side errors and missing features.
Here are the main issues that need to be addressed:
-
Server Fails to Start: The server crashes on launch for two reasons:
- In
src/index.js, the importfrom './server.js'is incorrect. The server instance is exported fromsrc/db.js. src/db.jsattempts to importrooms,renameRoom,deleteRoom, andjoinRoomfromsrc/websocket.js, but these are not implemented or exported, causing a fatal error.
- In
-
Incomplete Room Management: A core requirement is to allow users to create, rename, and delete rooms. This functionality is missing from both the client UI in
src/App.jsxand the server-side logic insrc/websocket.js.
Once these blockers are fixed, the application should be fully functional. Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent progress on the chat application! You've successfully resolved the server startup errors and implemented almost all of the room management features. The client and server are communicating well for creating, deleting, and joining rooms.
This submission requires one final change to meet all requirements. The ability for a user to rename a room is a core feature, but the client-side UI for it is missing in src/App.jsx. Please add the necessary UI elements, such as a button and input fields, to allow users to trigger a rename_room event and send it to the server.
You're very close to finishing! I look forward to seeing the final version.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent work implementing all the required features, including the new rename functionality! Your server-side logic is very well done. I am approving this solution as an educational override to help you progress, but there is one important client-side issue to address.
In src/App.jsx, you're using the same room state for both the currently joined room and the room management input field. This leads to incorrect behavior, as actions like 'Delete' or 'Rename' will target the text in the input box instead of the room you've joined.
To fix this, please introduce a separate state variable for the input field (e.g., roomNameInput). This new state should be used for creating rooms, while the original room state should only track the currently active room for actions like deleting or renaming. Addressing this will make your app's logic much more robust. Keep up the fantastic effort!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| <div> | ||
| <input | ||
| placeholder="Room name" | ||
| value={room} | ||
| onChange={(e) => setRoom(e.target.value)} | ||
| /> | ||
| <button onClick={createRoom}>Create</button> | ||
| <button onClick={deleteRoom}>Delete</button> | ||
| </div> |
There was a problem hiding this comment.
This block of controls has a state management issue. It uses the room state for the input field's value, which is also meant to track the currently joined room. This leads to incorrect behavior for the Create, Delete, and Rename buttons.
For example:
- A user joins 'Room A'. The
roomstate becomes 'Room A'. - The user then types 'Room B' into this input field. The
roomstate is now 'Room B'. - If the user clicks 'Delete', the app will try to delete 'Room B', not 'Room A' which they originally joined.
To fix this, consider using a separate state variable for this input field (e.g., roomNameInput). The createRoom function should use this new state, while deleteRoom and renameRoom should operate on the room state, which should only represent the currently joined room.
2 participants
req met