Use a single function to process and render messages #596
Conversation
|
lgtm 👍 |
|
|
||
| for (var i = 0; i < channels.length; i++) { | ||
| renderChannelMessages(channels[i]); | ||
| } |
There was a problem hiding this comment.
Could also
channels.forEach(renderChannelMessages);
There was a problem hiding this comment.
Agreed, this is much nicer and functional style, good catch @williamboman!
It actually could, if later we can display a scrollback from a previous join, so good! :-) |
|
Hey @xPaw, would you mind adding some inline comments please? To be honest, it would help me with the review, and also for future ourselves :-) |
| @@ -182,7 +188,7 @@ $(function() { | |||
| chan.click(); | |||
| }); | |||
|
|
|||
| socket.on("msg", function(data) { | |||
| function renderChatMessage(data) { | |||
There was a problem hiding this comment.
renderChatMessage builds a message and returns it, while renderChannelMessage actually renders something, i.e. builds it and shove it into the DOM.
I would call this buildChatMessage or equivalent buildMessage is probably enough, but it's up to you) to avoid the confusion.
|
@xPaw, looks great! |
|
I didn't include compiled templates because including them in a pull request becomes very messy quickly. |
|
So what's your solution for this PR? If we merge this as is, Shout doesn't run, and master should be deployable as is at all times. Can you just include the changes in the compiled assets that correspond to your changes in the templates please? |
|
Well the thing is, if I compile it myself and commit it, every |
|
I don't believe git will let you do that: https://help.github.com/articles/dealing-with-line-endings/ |
|
👍 Awesome PR! Thanks! |
Use a single function to process and render messages
Fixes #594 and will also help #588.
This pull request explicitly renders all received messages (on page load, channel join (unlikely to have any messages?), and loading more history) through a single function which correctly processes it and applies all the magic (e.g. makes channel names clickable).