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

Feature chat app #345

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

sadath2001
Copy link
Contributor

What does this PR do?

Added chat functionality using sockets
Displays last time of the messages (eg: 1 hour ago etc)
Displays user avatar as well to distinguish messages

Fixes #2
@AlfiyaSiddique , kindly review

https://drive.google.com/file/d/1hPuEonRq0Jq0Q7km4txaLOX11oc2y8To/view?usp=sharing

How should this be tested?

  • Create a new chat for new users
  • Add message and send it out
  • Paralelly login to a new user and try chatting, you'll see realtime sync between the messages

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Contributing guide
  • Self-reviewed my own code
  • Checked for warnings, there are none
  • Removed all console.logs
  • The changes don't cause any responsiveness issues

Copy link

netlify bot commented Oct 24, 2024

Deploy Preview for delightful-daifuku-a9f6ea failed. Why did it fail? →

Name Link
🔨 Latest commit 480e080
🔍 Latest deploy log https://app.netlify.com/sites/delightful-daifuku-a9f6ea/deploys/672a98026632eb0008aaebe1

@AlfiyaSiddique
Copy link
Owner

@sadath2001 any updates on this PR?

@sadath2001
Copy link
Contributor Author

hi @AlfiyaSiddique , i have already raised this PR. been more than a week.

@sadath2001
Copy link
Contributor Author

ok i see there are merge conflicts
, ill resolve them, any other changes apart from the merge conflicts?

@AlfiyaSiddique
Copy link
Owner

In the last conversation in this PR issue section you said that there are some changes remain:
image
if that's done all good, just upload a final video of output in this comment section

@sadath2001
Copy link
Contributor Author

@AlfiyaSiddique , kindly review

@sadath2001
Copy link
Contributor Author

In the last conversation in this PR issue section you said that there are some changes remain: image if that's done all good, just upload a final video of output in this comment section

What i actually mean by this was, i will raise another PR, apart from this. (since we discussed that we can create 2 PR's for this)

https://drive.google.com/file/d/17VSOzIsUSNPUIdU57UCwtWR3Gc8RhAR6/view?usp=sharing (demo)

@AlfiyaSiddique
Copy link
Owner

@sadath2001 there are random unknown characters in some files, due to improper merge conflicts resolution:
image

Can you just check all changed files and remove such lines?

@AlfiyaSiddique
Copy link
Owner

Also, I will complete the review for this by Friday as I am a bit occupied

@sadath2001
Copy link
Contributor Author

Hi, sure I will resolve them, i apologise.

@sadath2001
Copy link
Contributor Author

Also, I will complete the review for this by Friday as I am a bit occupied

Sure, take your time please.

@AlfiyaSiddique
Copy link
Owner

@sadath2001 any updates on this:
#345 (comment)

@AlfiyaSiddique
Copy link
Owner

@sadath2001 can you complete this PR before tomorrow eve?

@AlfiyaSiddique
Copy link
Owner

@sadath2001 just to remind you that the points will not be counted if the pr will merge after 7

@sadath2001
Copy link
Contributor Author

  • Hi @AlfiyaSiddique , i apologise, I'm currently travelling due to some issues and i couldn't take a look at this. Thank you for your guidance and support on this.

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.

Adding Chat functionality
2 participants