MakeSchool-Tutorials / Make-Chat-Tutorial

Build a Slack Clone using Socket.io
1 stars 4 forks source link

5. Start Chatting #7

Closed ryanlsmith4 closed 5 years ago

ryanlsmith4 commented 5 years ago

There appears to be a missing p tag in the usersOnline section... Also when appending usersOnline it should be a p not div`

Online Users

  <p class="userOnline"> </p>
</div> ` .     `  socket.on('new user', (username) => {
console.log(`✋ ${username} has joined the chat! ✋`);
// add the new user to the online users div
$('.usersOnline').append(`<p class='userOnline'>${username}</p>`);

});`

ibirnam commented 5 years ago

@ryanlsmith4 could you clarify where exactly the missing p tag is? I don't see one right now on that page.

Also wanted to know why you think usersOnline should be a p and not a div? I've got a working solution that uses this so curious why you think it should change. Also not clear what your last socket.on code is in your previous comment, if you could explain it more. Thanks!

ryanlsmith4 commented 5 years ago

I noticed that the children of one of the div tags made the online users look wonky when people logged on my solution was to make the things inside the div p's this way they weren't misaligned.

ibirnam commented 5 years ago

@ryanlsmith4 sorry, I need you to be more specific:

There appears to be a missing p tag in the usersOnline section

Where is the missing p tag?

Also when appending usersOnline it should be a p not div

Your reason for this is that it made online users "look wonky", could you provide a screenshot of a before/after of your fix? I changed <div class="usersOnline"> to be a p instead, but didn't see much of a noticeable difference

nsafai commented 5 years ago

I could be wrong, but I think @ryanlsmith4 is pointing out that in the "DISPLAY ONLINE USERS" and "LET'S SEND SOME MESSAGES" sections, we're appending a div:

$('.usersOnline').append(`<div class="userOnline">${username}</div>`);

whereas in later sections, like "BE A HERO AND SAVE THESE USERS!" we're appending a p:

$('.usersOnline').append(`<p class="userOnline">${username}</p>
ibirnam commented 5 years ago

@nsafai - gotcha, thanks for clarifying. I've fixed that issue in 20e48f53427582e727cd572c92a9dd7cdc838aef and it's live on the tutorial now.

@ryanlsmith4 - can you confirm that @nsafai 's comment above is what you were referring to, or was it a separate issue? Once you've confirmed, I can close/keep this issue

ryanlsmith4 commented 5 years ago

That is correct sorry for the delay in responding. Thanks again!

On Tue, Feb 19, 2019, 15:20 Ian Birnam <notifications@github.com wrote:

@nsafai https://github.com/nsafai - gotcha, thanks for clarifying. I've fixed that issue in 20e48f5 https://github.com/MakeSchool-Tutorials/Make-Chat-Tutorial/commit/20e48f53427582e727cd572c92a9dd7cdc838aef and it's live on the tutorial now.

@ryanlsmith4 https://github.com/ryanlsmith4 - can you confirm that @nsafai https://github.com/nsafai 's comment above is what you were referring to, or was it a separate issue? Once you've confirmed, I can close/keep this issue

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/MakeSchool-Tutorials/Make-Chat-Tutorial/issues/7#issuecomment-465352569, or mute the thread https://github.com/notifications/unsubscribe-auth/Ai8uGM3PmUAM1mJiu3h4n8-P-BaTGJlLks5vPIa3gaJpZM4an6fF .