Skip to content

add solution #125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ghost wants to merge 1 commit into from
Closed

add solution #125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"license": "GPL-3.0",
"devDependencies": {
"@mate-academy/eslint-config": "latest",
"@mate-academy/scripts": "^1.8.6",
"@mate-academy/scripts": "^2.1.1",
"eslint": "^8.57.0",
"eslint-plugin-jest": "^28.6.0",
"eslint-plugin-node": "^11.1.0",
Expand Down
26 changes: 25 additions & 1 deletion src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,25 @@
'use strict';
/* eslint-disable no-console */

const express = require('express');
const http = require('http');
const { Server } = require('socket.io');
const path = require('path');
const { setupSocketHandlers } = require('./socket/socketHandlers');

const app = express();
const server = http.createServer(app);
const io = new Server(server);

const PORT = process.env.PORT || 3000;
Comment on lines +5 to +13

@mateacademy-ai-mentor mateacademy-ai-mentor Aug 9, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic for handling the rooms list event is not robust enough to handle modifications (rename) or deletions of the current room by another user. This leads to two issues:

  1. Rename Bug: If another user renames the room you are currently in, the room list in the sidebar updates, but the main chat header does not, showing a stale name.
  2. Delete Bug: If another user deletes the room you are in, your client becomes stuck in a non-existent "ghost" room. You are not automatically moved to a valid room.

A better approach would be to check if state.currentRoomId still exists in the updated rooms array. If not, the client should automatically join a default room. If it does, you should refresh the chat header to reflect any potential name change.


app.use(express.static(path.join(__dirname, 'public')));

app.get('/', (req, res) => {
res.sendFile(path.join(__dirname, 'public', 'index.html'));
});

setupSocketHandlers(io);

server.listen(PORT, () => {
console.log(`Server listening on port:${PORT}`);
});
70 changes: 70 additions & 0 deletions src/models/roomManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
const rooms = {
general: { name: 'General', messages: [] },
tech: { name: 'Technology', messages: [] },
random: { name: 'Random', messages: [] },
};

function getAllRooms() {
return rooms;
}

function getRoomsList() {
return Object.entries(rooms).map(([id, data]) => ({ id, name: data.name }));
}

function getRoom(roomId) {
return rooms[roomId];
}

function createRoom(roomName) {
const roomId = `room_${Date.now()}`;

rooms[roomId] = { name: roomName, messages: [] };

return roomId;
}

function renameRoom(roomId, newName) {
if (rooms[roomId]) {
rooms[roomId].name = newName;

return true;
}

return false;
}
Comment on lines +27 to +35

@mateacademy-ai-mentor mateacademy-ai-mentor Aug 9, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The renameRoom function does not prevent the 'general' room from being renamed on the server side. While the client-side code correctly blocks this action, it's best practice for the data model to enforce its own integrity rules. To make the backend more robust, you should add a check here similar to the one in deleteRoom, ensuring the 'general' room cannot be modified. For example: if (rooms[roomId] && roomId !== 'general') { ... }.


function deleteRoom(roomId) {
if (rooms[roomId] && roomId !== 'general') {
delete rooms[roomId];

return true;
}

return false;
}

function addMessage(roomId, message) {
if (rooms[roomId]) {
rooms[roomId].messages.push(message);

return true;
}

return false;
}

function roomExists(roomId) {
return !!rooms[roomId];
}

module.exports = {
getAllRooms,
getRoomsList,
getRoom,
createRoom,
renameRoom,
deleteRoom,
addMessage,
roomExists,
};
50 changes: 50 additions & 0 deletions src/models/userManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
const users = {};

function createUser(socketId, username) {
users[socketId] = { username, currentRoom: null };
}
Comment on lines +3 to +5

@mateacademy-ai-mentor mateacademy-ai-mentor Aug 9, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the functions in this module are clear, adding JSDoc comments would improve overall documentation and long-term maintainability. This is especially helpful as a project grows or when other developers need to quickly understand the module's API. For example:

/**
 * Creates a new user or updates an existing one.
 * @param {string} socketId - The unique socket ID for the user's connection.
 * @param {string} username - The display name for the user.
 */
function createUser(socketId, username) { ... }

Applying this practice to all exported functions would be beneficial.


function getUser(socketId) {
return users[socketId];
}

function getUserRoom(socketId) {
const user = users[socketId];

return user ? user.currentRoom : null;
}

function setUserRoom(socketId, roomId) {
if (users[socketId]) {
users[socketId].currentRoom = roomId;

return true;
}

return false;
}

function removeUser(socketId) {
const user = users[socketId];

if (user) {
delete users[socketId];

return user;
}

return null;
}

function userExists(socketId) {
return !!users[socketId];
}

module.exports = {
createUser,
getUser,
getUserRoom,
setUserRoom,
removeUser,
userExists,
};
130 changes: 130 additions & 0 deletions src/public/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Socket.io Chat</title>
<script src="https://cdn.tailwindcss.com"></script>
<style>
::-webkit-scrollbar {
width: 8px;
}
::-webkit-scrollbar-track {
background: #2d3748;
}
::-webkit-scrollbar-thumb {
background: #4a5568;
border-radius: 4px;
}
::-webkit-scrollbar-thumb:hover {
background: #718096;
}
.system-message {
color: #a0aec0;
font-style: italic;
}
</style>
</head>
<body class="bg-gray-800 text-white font-sans">
<!-- Username Modal Overlay -->
<div
id="username-modal"
class="fixed inset-0 bg-black bg-opacity-75 flex items-center justify-center z-50"
>
<div class="bg-gray-700 p-8 rounded-lg shadow-xl text-center">
<h2 class="text-2xl font-bold mb-4">Enter Your Username</h2>
<form id="username-form">
<input
id="username-input"
type="text"
placeholder="Your cool name..."
class="w-full bg-gray-800 border border-gray-600 rounded-md px-4 py-2 text-white focus:outline-none focus:ring-2 focus:ring-indigo-500"
required
/>
<button
type="submit"
class="mt-4 w-full bg-indigo-600 hover:bg-indigo-700 text-white font-bold py-2 px-4 rounded-md transition duration-300"
>
Enter Chat
</button>
</form>
</div>
</div>

<!-- Main Chat Interface -->
<div id="chat-container" class="hidden h-screen w-screen flex">
<!-- Sidebar for Rooms -->
<div class="w-1/4 bg-gray-900 flex flex-col p-4 border-r border-gray-700">
<div class="flex-shrink-0 mb-4">
<h1 class="text-2xl font-bold">Chat Rooms</h1>
<p class="text-sm text-gray-400">
Welcome, <span id="display-username" class="font-bold"></span>!
</p>
</div>

<div id="room-list" class="flex-grow overflow-y-auto mb-4">
<!-- Room items will be populated by JS -->
</div>

<div class="flex-shrink-0 space-y-2">
<button
id="create-room-btn"
class="w-full bg-green-600 hover:bg-green-700 text-white font-bold py-2 px-4 rounded-md text-sm"
>
Create Room
</button>
<button
id="rename-room-btn"
class="w-full bg-yellow-600 hover:bg-yellow-700 text-white font-bold py-2 px-4 rounded-md text-sm"
>
Rename Current
</button>
<button
id="delete-room-btn"
class="w-full bg-red-600 hover:bg-red-700 text-white font-bold py-2 px-4 rounded-md text-sm"
>
Delete Current
</button>
</div>
</div>

<!-- Main Chat Area -->
<div class="w-3/4 flex flex-col">
<div id="chat-header" class="p-4 bg-gray-700 border-b border-gray-600">
<h2 id="current-room-name" class="text-xl font-semibold">
No room selected
</h2>
</div>
<!-- Messages Display -->
<div
id="messages"
class="flex-grow p-4 overflow-y-auto bg-gray-800 flex flex-col space-y-4"
>
<!-- Messages will be populated by JS -->
</div>

<!-- Message Input Form -->
<div class="p-4 bg-gray-700 border-t border-gray-600">
<form id="message-form" class="flex space-x-4">
<input
id="message-input"
type="text"
placeholder="Type a message..."
autocomplete="off"
class="flex-grow bg-gray-800 border border-gray-600 rounded-md px-4 py-2 text-white focus:outline-none focus:ring-2 focus:ring-indigo-500"
/>
<button
type="submit"
class="bg-indigo-600 hover:bg-indigo-700 text-white font-bold py-2 px-4 rounded-md"
>
Send
</button>
</form>
</div>
</div>
</div>
Comment on lines +55 to +125

@mateacademy-ai-mentor mateacademy-ai-mentor Aug 9, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the current structure using <div> elements is functional, you could enhance the document's semantics and accessibility by using more specific HTML5 tags. For example:

  • The sidebar (line 57) could be wrapped in an <aside> tag.
  • The main chat area (line 92) could be a <main> element.
  • The chat header (line 93) could be a <header> tag.
  • The container for the message input form (line 107) could be a <footer>.

Using these tags provides more meaning to the document structure for both developers and assistive technologies.


<script src="/socket.io/socket.io.js"></script>
<script type="module" src="js/main.js"></script>
</body>
</html>
33 changes: 33 additions & 0 deletions src/public/js/auth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
function initializeAuth(state) {
const usernameModal = document.getElementById('username-modal');
const usernameForm = document.getElementById('username-form');
const usernameInput = document.getElementById('username-input');
const displayUsername = document.getElementById('display-username');
const chatContainer = document.getElementById('chat-container');

if (state.username) {
showChatInterface();
}

usernameForm.addEventListener('submit', (e) => {
e.preventDefault();

const newUsername = usernameInput.value.trim();

if (newUsername) {
state.username = newUsername;
window.localStorage.setItem('chat_username', state.username);
showChatInterface();
}
});
Comment on lines +12 to +22

@mateacademy-ai-mentor mateacademy-ai-mentor Aug 9, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The form submission logic is functional, but it could be improved to provide better user feedback. Currently, if a user submits an empty or whitespace-only username, the form does nothing, which can be slightly confusing. It's also possible to click the submit button multiple times before the UI transitions.

Consider adding a disabled state to the button upon submission to prevent multiple clicks and provide immediate feedback that an action is in progress. For example:

usernameForm.addEventListener('submit', (e) => {
  e.preventDefault();
  const submitButton = e.target.querySelector('button[type="submit"]');
  const newUsername = usernameInput.value.trim();

  if (newUsername) {
    submitButton.disabled = true;
    submitButton.textContent = 'Entering...';

    state.username = newUsername;
    window.localStorage.setItem('chat_username', state.username);
    showChatInterface();
  }
});


function showChatInterface() {
displayUsername.textContent = state.username;
usernameModal.classList.add('hidden');
chatContainer.classList.remove('hidden');
chatContainer.classList.add('flex');
state.socket.emit('set username', state.username);
}
}

export { initializeAuth };
17 changes: 17 additions & 0 deletions src/public/js/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { initializeAuth } from './auth.js';
import { initializeSocket } from './socket.js';
import { initializeUI } from './ui.js';

document.addEventListener('DOMContentLoaded', () => {
const socket = window.io();

const state = {
socket,
username: window.localStorage.getItem('chat_username'),
currentRoomId: null,
};
Comment on lines +8 to +12

@mateacademy-ai-mentor mateacademy-ai-mentor Aug 9, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of a shared, mutable state object is effective for an application of this size. As a project grows, tracking down which part of the code modifies the state can become challenging. For future enhancements or larger projects, you might consider wrapping the state object in a Proxy. A Proxy would allow you to intercept state changes (e.g., setting currentRoomId) to automatically log mutations or trigger UI updates, centralizing state-change logic instead of spreading it across different modules. However, for the current scope, this implementation is perfectly suitable and clean.


initializeAuth(state);
initializeSocket(state);
initializeUI(state);
});
Loading