Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 34 additions & 0 deletions client/App.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
.messages {
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 task requires the client to render message objects containing author, time, and text (checklist items 1.4 and 3.6). This stylesheet defines the container .messages but does not include any styles for individual message elements such as .message, .author, .time, or .text. Add these classes so the UI clearly presents the required fields and so tests / reviewers can verify the rendered structure.

display: flex;
flex-direction: column-reverse;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using flex-direction: column-reverse will reverse visual order (new messages appear at the top). This can complicate keyboard navigation, screen readers, testing, and JS auto-scrolling. Consider: (a) using normal flex-direction: column and appending messages to the end of the container, then scroll to bottom in JS, or (b) keep DOM order reversed intentionally but document why and ensure screen-reader ordering is handled. If you keep column-reverse, you must ensure message history ordering and joins work as expected in the client logic (task requires room-specific history delivery).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using flex-direction: column-reverse here will reverse visual order of messages and interacts poorly with ad-hoc reverse() calls or prepending messages in JS. Pick one canonical ordering strategy (either keep column and append messages, or keep column-reverse and ensure all code and server history use that convention). See checklist: messages must be correctly ordered and history must be shown on join.

min-height: 200px;
Comment on lines +1 to +4
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider making the message container a normal column flow and using justify-content: flex-end or client-side scroll handling to show newest messages at the bottom. This avoids fragile reverse() logic in JS and makes scrolling predictable.

max-height: 200px;
Comment on lines +4 to +5
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using both min-height: 200px and max-height: 200px is redundant when you want a fixed size. Use height: 200px to make intent explicit. Also consider a CSS variable or relative unit if you need responsiveness.

overflow: scroll;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

overflow: scroll forces scrollbars to always show. Prefer overflow: auto so scrollbars only appear when needed. For mobile smooth scrolling add -webkit-overflow-scrolling: touch; if appropriate.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Prefer overflow: auto over overflow: scroll so a scrollbar only appears when necessary. This improves UX and avoids an always-visible disabled scrollbar.

}
Comment on lines +1 to +7
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There are no styles for individual message fields like .message, .author, .time, or .text. The task requires showing author and time for every message — add dedicated selectors so those fields are legible and consistent across messages.


.input {
display: flex;
gap: 24px;
}

.inputs {
Comment on lines +9 to +14
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There are two similar class names: .input and .inputs. This is likely to cause confusion (are both used? which is the canonical one?). Consider consolidating to a single clear name like .input-group or .controls and update the HTML accordingly to avoid accidental style omissions.

display: flex;
gap: 24px;
}

.room {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider adding styles for room items (e.g. .room-item, .room-name, .room-actions) so room operations required by the task (create, rename, join, delete) are clearly represented in the UI. Right now .room and .roomName exist, but there's no decorative or focus style for room list items — adding them helps with discoverability and testability of room features.

display: flex;
gap: 8px;
}

.roomsList {
display: flex;
flex-direction: column;

gap: 8px;
Comment on lines +26 to +28
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Small readability note: .roomsList block has an extra blank line between flex-direction and gap (lines 26–28). It's cosmetic only; no functional change required.

}

.roomName {
margin: 0;
margin-right: 16px;
}
259 changes: 259 additions & 0 deletions client/App.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,259 @@
import React, { useEffect, useState } from 'react';
import './App.css';
import Username from './Username';

type Message = {
id: number,
text: string,
author: string,
time: Date,
Comment on lines +5 to +9
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Type mismatch: Message.time is declared as Date but messages received from the server are serialized (strings). Either change the type to string or convert the server-provided string to a Date immediately after receiving messages. Keeping this mismatch can confuse consumers and TypeScript. This relates to requirement 3.2 (consistent time handling).

roomId: number,
}
Comment on lines +5 to +11
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your Message type declares time: Date. In practice fetch/WebSocket JSON will contain an ISO string for time and JSON.parse produces strings. This mismatch can cause TypeScript issues and confusion. Make the type reflect actual runtime values (e.g., time: string | Date) or convert incoming time strings to Date objects before storing them in state so all code expects the same shape (Reqs 3.2, 3.5).


type Room = {
id: number,
name: string,
}

const App: React.FC = () => {
const [messages, setMessages] = useState<Message[]>([]);
const [rooms, setRooms] = useState<Room[]>([]);

const [currentRoom, setCurrentRoom] = useState<number | null>(null);

const [messageInputValue, setMessageInputValue] = useState<string>('');
const [roomNameInputValue, setRoomNameInputValue] = useState<string>('');

const [roomRenameInputValue, setRoomRenameInputValue] = useState<string>('');

const [username, setUsername] =
useState(localStorage.getItem('username') || '');
Comment on lines +29 to +30
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You initialize username from localStorage here, but there is no code in this file that persists username back to localStorage when it changes or that explicitly notifies the server of the chosen username. The task requires that the client both persist the username in localStorage and send it to the server (Reqs 1.2, 1.3, 2.2, 3.1). If the Username component does not handle both persisting and notifying the server, add a useEffect here to write username into localStorage and send a REST/WS request to inform the server of the chosen username.


const [socket, setSocket] = useState<WebSocket | null>(null);

const handleMessageSend = () => {
if (!username || !currentRoom) return;
if (messageInputValue && socket) {
const message = {
text: messageInputValue,
time: new Date(),
author: username,
roomId: currentRoom,
};
socket.send(JSON.stringify(message));
Comment on lines +37 to +43
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

When constructing the outgoing message you include text, time, author, and roomId but no id. If the server assigns the id that's fine, but be explicit about this contract. The task requires every message to include author/time/text and that these persist — ensure the server returns the canonical saved message (with id/time) and that the client uses that server-sent message for rendering (Reqs 1.4, 3.2, 3.5).

Comment on lines +37 to +43
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Client sets and sends time: new Date() when sending messages. Relying on client clocks is fragile and violates the need for consistent timestamps; the server should canonicalize/override timestamps so all clients receive a consistent time. Remove time from the payload or let server ignore it. Also validate on the client before sending that author, text, and roomId are present (requirement 3.4).

setMessageInputValue('');
}
};

const handleCreateRoom = () => {
if (!roomNameInputValue) return;

fetch('http://localhost:3005/rooms', {
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify({ name: roomNameInputValue }),
})
.then(response => response.json())
.then(data => setRooms(prev => [data, ...prev]))
.then(() => setRoomNameInputValue(''))
.catch(error => console.log(error));
}

const handleRenameRoom = () => {
if (!roomRenameInputValue) return;

fetch(`http://localhost:3005/rooms/${currentRoom}`, {
method: 'PATCH',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify({ name: roomRenameInputValue }),
})
.then(response => response.json())
.then(data => {
setRooms((prev) =>
prev.map((room) =>
room.id === data.id ? { ...room, name: data.name } : room
)
);
})
.then(() => {
setRoomRenameInputValue('');
})
.catch(error => console.log(error));
}

const handleDeleteRoom = (id: number) => {
if (id === currentRoom) {
setCurrentRoom(null);
setMessages([]);
}

fetch(`http://localhost:3005/messages/${id}`, {
method: 'DELETE',
headers: {
'Content-Type': 'application/json',
}
})
.then(() => {
setMessages(prev => prev.filter(message => message.roomId !== id));
})
.catch(error => console.log(error));

fetch(`http://localhost:3005/rooms/${id}`, {
method: 'DELETE',
headers: {
'Content-Type': 'application/json',
}
})
.then(() => {
setRooms(prev => prev.filter(room => room.id !== id));
})
.catch(error => console.log(error));


}

const handleJoinRoom = (id: number) => {
fetch(`http://localhost:3005/rooms/${id}/join`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
}
})
.then(response => {
if (response.ok) {
return response.json();
} else {
throw new Error('Room not found or join failed');
}
})
.then(data => {
console.log('Room join confirmed:', data.message);
setCurrentRoom(id);

if (socket) {
socket.send(JSON.stringify({
type: 'join',
roomId: id
}));
}

return fetch(`http://localhost:3005/messages/${id}`);
})
.then(response => response.json())
.then(data => setMessages(data.reverse()))
Comment on lines +119 to +147
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

handleJoinRoom currently sends a WS join event and then fetches history via REST. The server also sends history on WS join. This duplicates history delivery and can cause inconsistencies or duplicated messages. Choose one flow (WS or REST) and remove the other. If you keep REST fetch, don't rely on WS history; if you keep WS history, don't fetch messages here. Also avoid calling reverse() ad-hoc — make ordering explicit at the server or a single place in the client.

}

useEffect(() => {
const newSocket = new WebSocket('ws://localhost:3005');

newSocket.onmessage = (event) => {
const data = JSON.parse(event.data);

if (data.type === 'history') {
setMessages(data.messages);
return;
}

if (data.type === 'join-success' || data.type === 'join-error') return;

setMessages(prev => [data, ...prev]);
Comment on lines +153 to +163
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Incoming WS messages are appended into state unconditionally (setMessages(prev => [data, ...prev])). You must filter incoming messages by data.roomId === currentRoom so messages from other rooms do not appear in the current room view (this enforces client-side room scoping even if server misroutes). This is required to uphold room isolation behavior (core requirement).

};

setSocket(newSocket);

if (currentRoom) {
newSocket.onopen = () => {
newSocket.send(JSON.stringify({
type: 'join',
roomId: currentRoom
}));
};
Comment on lines +166 to +174
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You set setSocket(newSocket) after configuring onmessage and onopen, but the onopen handler is set conditionally only when currentRoom is truthy. If the socket opens before a user joins (or if reconnect happens and currentRoom is set later) the join message may not be sent. Consolidate join sending logic (prefer sending join from handleJoinRoom when user clicks Join) and ensure set-username is sent on every open if username exists. This will make join and username notifications reliable on reconnects.

}

return () => newSocket.close();
}, [currentRoom]);
Comment on lines +150 to +178
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WebSocket is created inside a useEffect that depends on currentRoom, so a new socket is created and the previous socket closed whenever the user switches rooms. This is fragile and unnecessary. Create the WebSocket once (useEffect with empty deps) and reuse it. Also, on socket open you should send set-username if a username exists in localStorage so the server knows the client's identity after reconnect (requirement 3.1). Right now there is no guarantee the username will be sent on reconnect.


useEffect(() => {
fetch('http://localhost:3005/rooms')
.then(response => response.json())
.then(data => setRooms(data.reverse()))
Comment on lines +180 to +183
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You call setRooms(data.reverse()) when loading rooms and similarly setMessages(data.reverse()) after fetching messages. Using reverse() in multiple places leads to unclear ordering semantics and potential bugs. Decide on a canonical order (e.g., server returns messages ordered by time DESC or ASC) and remove client-side ad-hoc reversing so ordering is consistent. See message.service / server query for making ordering explicit.

.catch(err => console.log(err))
}, []);

return (
<div className="messager">
<Username username={username} setUsername={setUsername} socket={socket} />
<ul className="messages">
{messages.map(message => (
<li key={message.id}>
<span>{message.text}</span>
{` - FROM ${message.author} AT `}
{new Date(message.time).toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' })}
</li>
))}
</ul>
<div className="input">
<input
placeholder='Message'
type="text"
value={messageInputValue}
onChange={(event) => setMessageInputValue(event.target.value)}
onKeyDown={(event) => {
if (event.key === 'Enter') {
handleMessageSend();
}
}}
/>
<button type="submit" onClick={handleMessageSend}>Send</button>
</div>

<div className="rooms">
<p>Current room: {rooms.find(room => room.id === currentRoom)?.name}</p>
<p>Rooms:</p>
<div className="inputs">
<div>
<input
placeholder='Name'
type="text"
value={roomNameInputValue}
onChange={
(event) => setRoomNameInputValue(event?.target.value)
}
/>
<button onClick={handleCreateRoom}>Create</button>
</div>

<div>
<input
placeholder='New name'
type="text"
value={roomRenameInputValue}
onChange={
(event) => setRoomRenameInputValue(event?.target.value)
}
/>
<button
onClick={handleRenameRoom}
>Rename</button>
</div>
</div>

<ul className='roomsList'>
{rooms.map((room) => (
<li key={room.id} className='room'>
<p className='roomName'>{room.name}</p>
<button onClick={() => handleJoinRoom(room.id)}>Join</button>
<button onClick={() => handleDeleteRoom(room.id)}>Delete</button>
</li>
))}
</ul>
</div>
</div>
);
}

export default App;
47 changes: 47 additions & 0 deletions client/Username.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import React, { useState } from 'react';

type Props = {
username: string;
setUsername: React.Dispatch<React.SetStateAction<string>>;
socket: WebSocket | null;
}
const Username: React.FC<Props> = ({ username, setUsername, socket }) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ensure the server is informed of an already-saved username on reconnect/socket open. This component only sends on explicit save; when the app initializes with username from localStorage (handled in App.tsx), you should also send { type: 'set-username', username } when the socket opens or when socket becomes available. Implement a useEffect that watches socket and username and sends the message when socket.readyState === WebSocket.OPEN, or add that logic in the parent (client/App.tsx) where both values are available. This guarantees the server always knows the client's username after reconnects.

const [inputValue, setInputValue] = useState<string>('');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Currently inputValue is initialized as an empty string. If the parent supplies an existing username (for example loaded from localStorage on app init), consider initializing or syncing the input with that prop so the UI reflects the current username. Example approaches:

  • useState<string>(username || '') to initialize, or
  • useEffect that sets inputValue when username prop changes.
    This is a minor UX improvement but helps users who want to update their username.


function saveUsername() {
if (inputValue) {
setUsername(inputValue);
localStorage.setItem('username', inputValue);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good: storing the username into localStorage satisfies the requirement to persist the username across sessions (checklist items 1.3 and 3.1). Keep this behavior. You may want to ensure that you only store validated/non-empty usernames (trim whitespace) before persisting.

Comment on lines +11 to +14
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Trim and validate the input before saving. Right now you save whatever inputValue contains — this allows whitespace-only usernames. Consider:

  • const name = inputValue.trim()
  • if (!name) return;
  • then call setUsername(name) and localStorage.setItem('username', name).
    This aligns with the task requirement that a username is actually set/stored (non-empty).



if (socket) {
socket.send(JSON.stringify({
type: 'set-username',
username: inputValue
}));
}
Comment on lines +17 to +22
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Avoid calling socket.send when the socket exists but is not open — that can throw. Replace if (socket) { socket.send(...) } with something like:

  • if (socket && socket.readyState === WebSocket.OPEN) { socket.send(...) }
    Or queue the message and send it in an onopen handler if the socket isn't open yet. This prevents runtime exceptions when the WS connection is still connecting.


setInputValue('');
}
}

return (
<div>
<label htmlFor="username">Username: </label>
<input
id="username"
type="text"
value={inputValue}
onChange={e => setInputValue(e.target.value)}
placeholder="Enter your username"
/>
<button onClick={saveUsername}>Send</button>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Small UX suggestion (optional): provide feedback on save (e.g., disable the button while notifying the server, show confirmation or an error) and disable the save button when inputValue is empty to prevent user confusion. This is not required by the task, but improves user experience.


{username.length !== 0 && (
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 condition username.length !== 0 will work when username is a string, but is fragile against whitespace-only names. Consider a more defensive check such as username && username.trim().length > 0 to avoid showing an empty/whitespace username. Also, using a truthy check (username ? ...) is clearer and safer if username ever becomes undefined.

<div>Your username: {username}</div>
)}
</div>
);
};

export default Username;
3 changes: 3 additions & 0 deletions client/index.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ul {
list-style: none;
Comment on lines +1 to +2
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This file sets list-style: none for all ul elements. That works, but it is a global rule and can unintentionally remove bullets from lists that should keep them. Consider scoping this to the specific chat list classes (for example .messages ul or .roomsList) to avoid side effects.

}
Comment on lines +1 to +3
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ensure this stylesheet is actually imported by the app. In the client/App.tsx you provided earlier the imported CSS file is ./App.css, not index.css. If you expect these styles to apply, import index.css from a top-level file (e.g., index.tsx) or rename/merge into the CSS file that is currently imported.

Comment on lines +1 to +3
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Avoid the global ul selector. This rule will affect every unordered list in the app and can cause unintended layout changes. Scope it to the specific lists used in the UI (for example .messages ul or .roomsList) so other lists remain unaffected.

13 changes: 13 additions & 0 deletions client/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<link rel="icon" type="image/svg+xml" href="/vite.svg" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Chat</title>
</head>
<body>
<div id="root"></div>
<script type="module" src="main.tsx"></script>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Module entry reference: confirm the script path will be resolved by your bundler. In Vite projects the common convention is to point to /src/main.tsx (or ./src/main.tsx) from index.html. Using main.tsx may work depending on configuration, but switching to /src/main.tsx avoids resolution ambiguity and matches the typical Vite template.

</body>
</html>
11 changes: 11 additions & 0 deletions client/main.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { StrictMode } from 'react'
import { createRoot } from 'react-dom/client'
import './index.css'
import App from './App.tsx'
import React from 'react';
Comment on lines +4 to +5
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Optional: import React from 'react' may be unnecessary if your project uses the automatic JSX runtime (modern React + TypeScript setups). It's harmless but can be removed if your build/tsconfig supports the automatic transform to avoid an unused import.


createRoot(document.getElementById('root')!).render(
<StrictMode>
<App />
</StrictMode>
)
Loading