Skip to content
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.

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

Frontend: Added a chat box in the game page #135

Merged
merged 1 commit into from
Jun 22, 2024

Conversation

sksmagr23
Copy link
Contributor

@sksmagr23 sksmagr23 commented Jun 20, 2024

Description

Added a collapsible chat box in the gamepage, which contains messagelist, input box, reactions to messages. The necessary files are added in a folder Chatbox in src/library. The folder include files namely
1.types.ts
2.chatbox.tsx
3.Message.tsx
4.MessageInput.tsx
5.MessageList.tsx

The packages emoji picker react and react icons are also installed.

fixes #125

How to Test

  • Run "npm run dev" in frontend directory
  • navigate to /game
  • Click on chat icon at bottom

Related Issues

Checklist

  • I have tested these changes locally.
  • I have reviewed the code and ensured it follows the project's coding guidelines.
  • I have updated the documentation, if necessary.
  • I have assigned reviewers to this pull request.

Screenshots

Screenshot from 2024-06-20 22-17-33
Screenshot from 2024-06-20 22-17-51

Copy link

vercel bot commented Jun 20, 2024

@sksmagr23 is attempting to deploy a commit to the Shivansh Bhatanagar's projects Team on Vercel.

A member of the Team first needs to authorize it.

@kuv2707
Copy link
Collaborator

kuv2707 commented Jun 20, 2024

So i should clarify the message sending structure. When the Chatbox mounts, it registers a handler in channel.ts for chat events. (similar to how we subscribe to game events, see GameContext) When the user sends a message, we don't immediately append the message to the message array, rather we update the array when the server sends the MESSAGE_SENT event. (For better UX, we should display the message as sent even before the server sends a response, but that will be handled later)

Finally we should store messages in a JS object (or better, the Map class) instead of array, so that we can find them in constant time.
so we store (messageid, messageobject) as the key value pair in the object.

Copy link
Collaborator

@kuv2707 kuv2707 left a comment

Choose a reason for hiding this comment

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

Looks phenomenal in manual testing!

We'd need to change the structure of sending messages and reactions.

Event triggering is done via channel.triggerEvent

@sksmagr23 sksmagr23 force-pushed the chatbox-#125 branch 3 times, most recently from 0d152e8 to 6bb625c Compare June 21, 2024 15:31
@sksmagr23
Copy link
Contributor Author

sksmagr23 commented Jun 21, 2024

@kuv2707 done the review changes, you can have a look if any other change required
Screenshot from 2024-06-21 21-06-13

@kuv2707
Copy link
Collaborator

kuv2707 commented Jun 21, 2024

@sksmagr23 It is a good practice to respond to the feedback (even with a reaction) instead of resolving the conversation yourself

Copy link
Collaborator

@kuv2707 kuv2707 left a comment

Choose a reason for hiding this comment

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

Suggested some changes and unresolved conversations which were not properly addressed.

It can be merged once these changes are addressed

@kuv2707
Copy link
Collaborator

kuv2707 commented Jun 21, 2024

You can add some drop shadow to the chat box

@sksmagr23
Copy link
Contributor Author

Added the drop shadow to chatbox

Added a collapsible chat box in the gamepage,
the necessary files are added in a folder Chatbox
in src/library

fixes shivansh-bhatnagar18#125
@sksmagr23
Copy link
Contributor Author

sksmagr23 commented Jun 22, 2024

@kuv2707 done all the reviewed changes and resolved the previous unresolved reviews

The lint errors are coming because I have removed api call in messageinput and event listeners in chatbox.tsx as function created are not used,
Once they are implemented further, it will be fixed

@kuv2707
Copy link
Collaborator

kuv2707 commented Jun 22, 2024

We can merge it at this point, and address the regressions as a follow-up
Thanks for all the work @sksmagr23 !

@kuv2707 kuv2707 merged commit 3a8332e into shivansh-bhatnagar18:master Jun 22, 2024
4 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make a chat box
2 participants