Conversation
| import PropTypes from 'prop-types'; | ||
|
|
||
| const ChatEntry = () => { | ||
| const ChatEntry = ({ id, sender, body, timeStamp, liked, onLike }) => { |
There was a problem hiding this comment.
Nice job destructuring your props.
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
| // Replace the outer tag name with a semantic element that fits our use case | ||
| <replace-with-relevant-semantic-element className="chat-entry local"> | ||
| <h2 className="entry-name">Replace with name of sender</h2> | ||
| <div className="chat-entry"> |
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> | ||
| <p>{body}</p> | ||
| <p className="entry-time"><TimeStamp time={timeStamp}></TimeStamp></p> |
There was a problem hiding this comment.
Nice job using the provided TimeStamp component.
| onLike={onLike} | ||
| /> | ||
| ))} | ||
| </ul> |
There was a problem hiding this comment.
❗️🚨 You use the ul element here. The children of the ul element are li, but your project does not use any li elements.
This isn't a semantic use of the ul element since it doesn't include li.
Consider using a different element like a section element.
| timeStamp={entry.timeStamp} | ||
| liked={entry.liked} | ||
| onLike={onLike} | ||
| /> |
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 <div>{chatEntries}</div>;| setEntryData(preventryData => | ||
| preventryData.map(entry => |
There was a problem hiding this comment.
Nice job using the callback-style for updating the state.
There was a problem hiding this comment.
Your variable name has a small issue with camelCase
| setEntryData(preventryData => | |
| preventryData.map(entry => | |
| setEntryData(prevEntryData => | |
| prevEntryData.map(entry => |
| }; | ||
|
|
||
| const countTotalLikes = (entryData) => { | ||
| return entryData.reduce((total, entry) => total + (entry.liked ? 1 : 0), 0); |
There was a problem hiding this comment.
👀 Notice that you calculate the value of the total likes by checking if the liked attribute for an entry is true or false. Therefore, your message objects do not need a likeCount attribute (like what you added in messages.json).
| <header> | ||
| <h1>Application title</h1> | ||
| <h1>Chat between Vladimir and Estragon</h1> | ||
| <h2>{totalLikes} ❤️s</h2> |
There was a problem hiding this comment.
You could also directly invoke countTotalLikes here, instead of creating the variable totalLikes on line 25.
| <h2>{totalLikes} ❤️s</h2> | |
| <h2>{countTotalLikes(entryData)} ❤️s</h2> |
| <div id="App"> | ||
| <header> | ||
| <h1>Application title</h1> | ||
| <h1>Chat between Vladimir and Estragon</h1> |
There was a problem hiding this comment.
Can you think of a way to dynamically get the senders' names without hardcoding "Vladimir" and "Estragon" here?
| return { ...entry, liked: !entry.liked }; | ||
| }; | ||
|
|
||
| const countTotalLikes = (entryData) => { |
There was a problem hiding this comment.
A more descriptive name for this function might be getTotalLikes which would convey to others that this function returns a total count of the likes.
2 participants
No description provided.