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

user typing indicator + layout fix #46

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nobalpha
Copy link

There was a merge conflict with the roommate functionality, so I needed to adapt the typing section to that.

What the code does, is simply listens on the client for "change" event in input, if it gets changes, so it means the user is typing. Then, emitting a "typing" event to server by creating and assigning a timeouthandler which will run out of time and send typing : false. While this is happenning, if change event occurs multiple times, without time runs out, it bypasses and reinitializes the clock. When the server gets the "typing" event, it basically broadcasts that user's name and state, which'll be either true xor false. With that said, Typers component gets updated and... Voila!

Future Improvements:

  • Hiding the author's typing status to itself
  • Instead of user is typing, create an icon only indicator into the roommate panel, by the user.

P.S. There was a messages box layout bug, which was doubling the height of the text-area, overflowing and squeezing the input area. Fixed it via attaching a fixed height.

@vercel
Copy link

vercel bot commented Oct 30, 2021

Someone is attempting to deploy a commit to a Personal Account owned by @manuelalferez on Vercel.

@manuelalferez first needs to authorize it.

@vercel
Copy link

vercel bot commented Oct 30, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

chat – ./client

🔍 Inspect: https://vercel.com/manuelalferez/chat/Bv73u2YZ9qPMkuZgGYcTCNgTLj2M
✅ Preview: https://chat-git-fork-nobalpha-master-manuelalferez.vercel.app

@nobalpha
Copy link
Author

Hey @manuelalferez !

Sorry for taging you, but if you've time, can you review my PR.

Thanks!

"requires": true,
"packages": {
Copy link
Owner

@manuelalferez manuelalferez Nov 1, 2021

Choose a reason for hiding this comment

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

client/package-lock.json and server/package-lock.json are being completely modified, when that shouldn't happen.

You can easily undone the changes of those two files with git-restore.

It happened in #9, if you have problems returning the files to their previous state, check how it was resolved.

Captura de pantalla 2021-11-01 a las 13 14 52

Comment on lines +1 to +17
.alert-enter {
opacity: 0;
transform: scale(0.9);
}
.alert-enter-active {
opacity: 1;
transform: translateX(0);
transition: opacity 300ms, transform 300ms;
}
.alert-exit {
opacity: 1;
}
.alert-exit-active {
opacity: 0;
transform: scale(0.9);
transition: opacity 300ms, transform 300ms;
}
Copy link
Owner

@manuelalferez manuelalferez Nov 1, 2021

Choose a reason for hiding this comment

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

We would like to avoid, as much as possible, having .css files in our project. Could you perform these transactions using tailwindcss only?

});

app.use(router);

server.listen(PORT, () => {
console.log(`Server has started on port ${PORT}`);
console.log(`Server has started on port ${PORT}`);
Copy link
Owner

Choose a reason for hiding this comment

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

It is quite possible that we will have problems with code formatting, just as we have had in the past.
For this we add a configuration with Prettier on the client side. However, I see that we did not include it on the server side.
Would you mind adding such configuration and files on the server side as well?

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.

2 participants