Conversation
…try to use messageData
| <div id="App"> | ||
| <header> | ||
| <h1>Application title</h1> | ||
| <h1>Messenger App</h1> |
There was a problem hiding this comment.
❗️ On line 24, you should have changed the title so that the header rendered "Chat between Vladimir and Estragon"
Can you think of a way to dynamically get the senders' names without hardcoding "Vladimir" and "Estragon" here?
| ); | ||
| }; | ||
|
|
||
| const totalLikes = messages.filter(message => message.liked).length; |
There was a problem hiding this comment.
👍 Nice! You could also use reduce too.
| setMessages(prevMessages => | ||
| prevMessages.map(message => |
There was a problem hiding this comment.
Nice job using the callback-style for updating the state.
| @@ -0,0 +1,22 @@ | |||
| import ChatEntry from "./ChatEntry" | |||
|
|
|||
| const ChatLog = (props) => { | |||
There was a problem hiding this comment.
I prefer props to be destructured, like I mentioned in my comment in ChatEntry.
| {props.entries.map(entry => ( | ||
| <ChatEntry |
There was a problem hiding this comment.
Another common way you'll see components organized using map is to store the result into a variable we use in the JSX result:
const chatEntries = entries.map((entry) => {
return (
<ChatEntry
// your props
/>
);
});
return <section className="chat-log">{chatEntries}</section>;| import TimeStamp from './TimeStamp'; | ||
| import PropTypes from 'prop-types'; | ||
|
|
||
| const ChatEntry = (props) => { |
There was a problem hiding this comment.
Destructuring props can be a matter of preference. I prefer to destructure props so that I don't need to type props. every time I want to get an attribute from the object.
Destructuring props also has some benefits:
- you know every possible prop right away.
- you know when a prop is being unused, in order to remove it.
- knowing which props are being used will also remind you what you need to validate
Ultimately, you'll adapt what you do depending on what your team does.
| <p>Replace with body of ChatEntry</p> | ||
| <p className="entry-time">Replace with TimeStamp component</p> | ||
| <button className="like">🤍</button> | ||
| <article className="chat-entry local "> |
There was a problem hiding this comment.
👀 Nitpick: white spacing is off
| <article className="chat-entry local "> | |
| <article className="chat-entry local"> |
There was a problem hiding this comment.
It was an optional enhancement, but how could you determine if a chat was from a 'local' sender vs a 'remote' sender so that the chat bubbles would have different styles applied to it?
If you have time to re-visit this project for React.js review, it would be good practice to try applying local vs. remote classes to the components so they render differently depending on who sent the message.
| <p className="entry-time">Replace with TimeStamp component</p> | ||
| <button className="like">🤍</button> | ||
| <article className="chat-entry local "> | ||
| <section className="chat-entry local"> |
There was a problem hiding this comment.
I think you could remove this section element since you use the article element directly before this to wrap elements of the ChatEntry component.
The article element also has the exact same classes on it so it feels a little redundant to have article and then section.
| <section className="chat-entry local"> |
| <section className="entry-bubble"> | ||
| <p>{props.body}</p> | ||
| <p className="entry-time"> | ||
| <TimeStamp time={props.timeStamp}></TimeStamp> |
There was a problem hiding this comment.
Nice work using the provided TimeStamp component here 👍
| <p className="entry-time"> | ||
| <TimeStamp time={props.timeStamp}></TimeStamp> | ||
| </p> | ||
| <button className="like" onClick={props.onToggleLike}> |
There was a problem hiding this comment.
Since a ChatEntry component gets liked and it knows its own id then we can pass the function down to ChatComponent and pass the id here, like this:
| <button className="like" onClick={props.onToggleLike}> | |
| <button className="like" onClick={() => props.onToggleLike(id)}> |
2 participants
No description provided.