feat: add typing indicator for better UI experience issue #8#39
feat: add typing indicator for better UI experience issue #8#39Shreenath-14 wants to merge 7 commits intoZenYukti:mainfrom
Conversation
Shreenath-14
requested review from
Blazzzeee and
ayushHardeniya
as code owners
|
Hi Team! This is the separated PR for the typing indicator (Issue #8). It includes the WebSocket logic and UI updates. I've verified it works locally with multiple users. |
ayushHardeniya
added
apertre3.0
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Hey @ayushHardeniya , @Blazzzeee |
|
yeah ill review it today night , sorry for being late |
|
Hey @Blazzzeee |
There was a problem hiding this comment.
Pull request overview
Adds a real-time “user is typing…” indicator to the room chat experience by introducing a new typing WebSocket message and UI rendering in the chat room.
Changes:
- Add a
typingWebSocket message type on both frontend and backend, including asendTypinghelper in the client hook. - Track and render currently-typing users in
ChatRoom. - Adjust API request header typing/merging logic in
apiFetch.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
frontend/src/lib/api.ts |
Changes header typing/merge approach for authenticated API calls. |
frontend/src/hooks/useWebSocket.ts |
Adds typing to message types and exposes sendTyping. |
frontend/src/components/ChatRoom.tsx |
Adds typing-users state, debounced typing emission, and typing indicator UI. |
backend/src/services/chatServer.ts |
Handles incoming typing messages and broadcasts typing state to other room members. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@Shreenath-14 bhot sari chizo ka type any hai , there have been no commits to fix that, |
|
@Shreenath-14 also websocket me resource leak hai useWebSocket me page change hone pe sb connectiobn purge krne hoge |
|
@Shreenath-14 fix alll these issues pls |
|
I'll fix all by tomorrow |
|
I have completed a full refactor to address all the issues raised during review. The codebase is now fully type-safe, and both frontend and backend builds are passing perfectly. Summary of Key Fixes:
Branch has been synced with main. Ready for final review! cc @Blazzzeee |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- backend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@Shreenath-14 please review the code before submitting ai generated code , be mindfull i have to manually review a lot of slop code , the goal isnt getting a issue closed ....... |
|
I apologies for the previous overhead. I went back through and refactored the logic manually to keep it minimal and efficient. |
|
@Blazzzeee Bro please pay attention on this |
|
yeah i have work to do too , please mind a buffer of 3-4 days atleast to reviewers |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- backend/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
frontend/src/components/ChatRoom.tsx:154
handleSendMessagedoesn’t clear the typing indicator. If the user submits right after typing, other clients may continue seeing “typing…” until the 2s debounce fires. Clear any pending timeout and sendsendTyping(false)as part of the submit path (and consider doing the same on input blur) so the indicator clears immediately on send.
const handleSendMessage = (e: React.FormEvent) => {
e.preventDefault();
if (!inputValue.trim() || !isConnected) {
return;
}
const success = sendMessage(inputValue.trim());
if (success) {
setInputValue('');
}
};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Clear typing timeout and reset typing users on unmount | ||
| return () => { | ||
| if (typingTimeoutRef.current) { | ||
| clearTimeout(typingTimeoutRef.current); | ||
| } | ||
| setTypingUsers(prev => prev.size > 0 ? EMPTY_SET : prev); |
There was a problem hiding this comment.
The unmount cleanup calls setTypingUsers(...). Setting state during unmount is unnecessary (the component is being removed) and can lead to noisy warnings in some React setups. Consider removing the state update here; clearing the timeout is sufficient.
| // Clear typing timeout and reset typing users on unmount | |
| return () => { | |
| if (typingTimeoutRef.current) { | |
| clearTimeout(typingTimeoutRef.current); | |
| } | |
| setTypingUsers(prev => prev.size > 0 ? EMPTY_SET : prev); | |
| // Clear typing timeout on unmount | |
| return () => { | |
| if (typingTimeoutRef.current) { | |
| clearTimeout(typingTimeoutRef.current); | |
| } |
| type: 'system', | ||
| }, | ||
| ]); | ||
| } else if (message.type === 'crisis_alert') { | ||
| setShowCrisisAlert(true); | ||
| setTimeout(() => setShowCrisisAlert(false), 10000); | ||
| } else if (message.type === 'typing') { | ||
| const { nickname, isTyping } = message; | ||
| setTypingUsers((prev) => { | ||
| const newSet = new Set(prev); | ||
| if (isTyping) { | ||
| newSet.add(nickname); | ||
| } else { | ||
| newSet.delete(nickname); | ||
| } | ||
| return newSet; | ||
| }); | ||
| } |
There was a problem hiding this comment.
useWebSocket / backend can send a crisis_alert message (with a message field for resources), but this handler doesn’t currently handle message.type === 'crisis_alert'. That means the sender won’t see the crisis resources payload. Add a crisis_alert branch (or remove/replace the backend event) so the message type is either rendered or explicitly ignored with a clear UX decision.
| if (roomId && userId) { | ||
| const isTyping = typeof message.isTyping === 'boolean' ? message.isTyping : false; | ||
| // Broadcast to other users in the room | ||
| this.broadcastToRoom(roomId, { | ||
| type: 'typing', | ||
| userId, | ||
| nickname, | ||
| isTyping, | ||
| }, userId); // Exclude sender | ||
| } |
There was a problem hiding this comment.
handleTyping trusts ws.roomId/userId metadata without verifying the socket is still a member of that room. Because handleLeave removes the member from the room set but doesn’t clear ws.roomId/userId, a client that sends leave (but keeps the connection open) can still broadcast typing events to the room. Consider either clearing ws.roomId/ws.userId/ws.nickname on leave, and/or checking membership in this.rooms.get(roomId) before broadcasting.
| if (roomId && userId) { | |
| const isTyping = typeof message.isTyping === 'boolean' ? message.isTyping : false; | |
| // Broadcast to other users in the room | |
| this.broadcastToRoom(roomId, { | |
| type: 'typing', | |
| userId, | |
| nickname, | |
| isTyping, | |
| }, userId); // Exclude sender | |
| } | |
| // Ensure the socket is still a member of the room before broadcasting | |
| if (!roomId || !userId) { | |
| return; | |
| } | |
| const roomMembers = this.rooms.get(roomId); | |
| if (!roomMembers) { | |
| return; | |
| } | |
| const isMember = Array.from(roomMembers).some((member) => member.ws === ws); | |
| if (!isMember) { | |
| return; | |
| } | |
| const isTyping = typeof message.isTyping === 'boolean' ? message.isTyping : false; | |
| // Broadcast to other users in the room | |
| this.broadcastToRoom( | |
| roomId, | |
| { | |
| type: 'typing', | |
| userId, | |
| nickname, | |
| isTyping, | |
| }, | |
| userId, // Exclude sender | |
| ); |
|
@Blazzzeee Bro that's enough I know you have some other work too but it was my first issue which I got at the starting and its the last moment where the program got end in next day still you are saying make those changes if you see this is the easy task where I just have to implement indicator and I did it well and if you see the current state of project then there is already the indicator is working because as a beginner I don't know "How those things work" but then I get cleared how to work on a open source project issue. And now at this point I'm also frustrated because when I raised the PR all things are good and sorry if I disappointed you but understand this feature is implemented on consideration of easy task. If needed further enhancement then I think you have to raise new issue on enhancement in typing indicator. And one last the previous PR which add this in feature is also created by me. so think and respond please as quick as possible and as an ai tester it suggest everytime you ask. |
|
@Shreenath-14 the reason i have to merge code is to maintain code quality , your code produces what the issue needs , however it doesnt mean code quality can be neglected , besides alot of these issues are legit problems in your code . If you think if the thing working for a specific local use case means it should be merged , you are very wrong by that .... |
minor: change setkey to userid from nickname (duplication fix) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| // Clear typing timeout and reset typing users on unmount | ||
| return () => { | ||
| if (typingTimeoutRef.current) { | ||
| clearTimeout(typingTimeoutRef.current); | ||
| } | ||
| setTypingUsers(prev => prev.size > 0 ? EMPTY_SET : prev); |
| type: 'system', | ||
| }, | ||
| ]); | ||
| } else if (message.type === 'crisis_alert') { | ||
| setShowCrisisAlert(true); | ||
| setTimeout(() => setShowCrisisAlert(false), 10000); | ||
| } else if (message.type === 'typing') { | ||
| const { nickname, isTyping } = message; | ||
| setTypingUsers((prev) => { | ||
| const newSet = new Set(prev); | ||
| if (isTyping) { | ||
| newSet.add(nickname); | ||
| } else { | ||
| newSet.delete(nickname); | ||
| } | ||
| return newSet; | ||
| }); | ||
| } |
|
@ayushHardeniya take the pull |
Description
Implemented a real-time typing indicator for the chat interface. This provides visual feedback when another user is typing, improving the overall user experience and communication flow within rooms. Closes #8.
Type of Change
How Has This Been Tested?
Screenshots
Checklist
Additional Information
Learning Outcomes
I learned how to handle real-time event broadcasting using WebSockets to synchronize UI states across multiple clients.