Conversation
mikellewade
left a comment
mikellewade
left a comment
There was a problem hiding this comment.
@SophieQA, overall great work on your React Chatlog. I only had a few points of feedback.
|
|
||
|
|
||
| const App = () => { | ||
| const [entries, setEntries] = useState(messages); |
| const toggleLiked = (id) => { | ||
| setEntries((prev) => prev.map((m) => (m.id === id ? { ...m, liked: !m.liked } : m))); | ||
| }; |
There was a problem hiding this comment.
The only piece of feedback I have here is that your naming for the message object, m, is not descriptive of the data that it is holding. This will give room to miscommunication and require more cognitive load on the reader's part to decipher what the shape of the object is. I would suggest for you to choose a name more descriptive like message.
| setEntries((prev) => prev.map((m) => (m.id === id ? { ...m, liked: !m.liked } : m))); | ||
| }; | ||
|
|
||
| const likeCount = entries.filter((m) => m.liked).length; |
There was a problem hiding this comment.
You'll see that there are a number of ways to implement this logic. Though this is functionally sound, I will say that if I see the .filter method I would assume that you would be working an array of data more in-depth. For simply counting the number of liked messages I would opt for something like .reduce instead.
Find more here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reduce
| <div id="App"> | ||
| <header> | ||
| <h1>Application title</h1> | ||
| <div className="likes-count">{likeCount} ❤️s</div> |
| <main> | ||
| {/* Wave 01: Render one ChatEntry component | ||
| Wave 02: Render ChatLog component */} | ||
| <ChatLog entries={entries} onToggleLiked={toggleLiked} /> |
| ChatLog.propTypes = { | ||
| entries: PropTypes.arrayOf( | ||
| PropTypes.shape({ | ||
| id: PropTypes.number.isRequired, | ||
| sender: PropTypes.string.isRequired, | ||
| body: PropTypes.string.isRequired, | ||
| timeStamp: PropTypes.string.isRequired, | ||
| liked: PropTypes.bool.isRequired, | ||
| }) | ||
| ).isRequired, | ||
| onToggleLiked: PropTypes.func.isRequired, | ||
| }; |
|
|
||
| return ( | ||
| <section className="chat-log"> | ||
| {chatEntries} |
| import TimeStamp from './TimeStamp'; | ||
|
|
||
| const ChatEntry = () => { | ||
| const ChatEntry = ({ id, sender, body, timeStamp, liked, onToggleLiked }) => { |
| id: PropTypes.number.isRequired, | ||
| sender: PropTypes.string.isRequired, | ||
| body: PropTypes.string.isRequired, | ||
| timeStamp: PropTypes.string.isRequired, | ||
| liked: PropTypes.bool, | ||
| onToggleLiked: PropTypes.func, |
There was a problem hiding this comment.
Great work on these proptypes, remember that they have been sunsetted in React and that will be using other type checking frameworks like Typescript.
| </p> | ||
| <button | ||
| className="like" | ||
| onClick={() => onToggleLiked && onToggleLiked(id)} |
There was a problem hiding this comment.
@SophieQA, not sure why you have onToggleLiked && onToggleLiked(id) as the return value here. You are essentially writing a function that returns true. Simply need to have a function to receive the automatically passed in event and then make a call to onToggleLiked with the appropriate id. You can remove the onToggleLiked function object.
| onClick={() => onToggleLiked && onToggleLiked(id)} | |
| onClick={() => onToggleLiked(id)} |
2 participants
No description provided.