Ruby - Anh Huynh Chat Log #110
Conversation
…one of the tests in Wave 3
kendallatada
left a comment
kendallatada
left a comment
There was a problem hiding this comment.
Hi Anh! Your project has been scored as green. You can find my comments in your PR. Let me know if you have any questions. Nice work! 😊
| // { | ||
| // 'sender':'Vladimir', | ||
| // 'body':'why are you arguing with me', | ||
| // 'timeStamp':'2018-05-29T22:49:06+00:00', | ||
| // } |
There was a problem hiding this comment.
Be sure to remove unused code, print tests, and any comments that shouldn't be published before finalizing a project. It's good practice to keep a clean main branch that's production ready. This helps with the readability and maintainability of a project. 🧹😌
| const likeCounts = () => { | ||
| let likeCount = 0; | ||
| for (let entry of chatData) { | ||
| if (entry.liked === true) { | ||
| likeCount += 1; | ||
| } | ||
| } | ||
| return likeCount; | ||
| }; |
There was a problem hiding this comment.
Nice job calculating the number of likes from state! ✨
| const chatEntryComponents = props.entries.map((entry, index) => { | ||
| return ( | ||
| <li key={index}> | ||
| <ChatEntry |
There was a problem hiding this comment.
Note that it's best practice not to use the index of an item as the list item key in React since the order of items in an array can change. I'd recommend using the entry ID as the key. Also, to cut down on the number of tags, you can put the key attribute directly on the <ChatEntry> component and drop the <li> tag. This means you also won't need the <ul> tag on line 24.
| 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, | ||
| }) | ||
| ), |
There was a problem hiding this comment.
Nice job with your prop types! I'd recommend making liked required and also the entries array required too.
| props.onUpdate(updatedChatEntry); | ||
| }; | ||
|
|
||
| const like = props.liked ? '❤️' : '🤍'; |
There was a problem hiding this comment.
Good usage of the ternary operator ✨
| sender: PropTypes.string.isRequired, | ||
| body: PropTypes.string.isRequired, | ||
| timeStamp: PropTypes.string.isRequired, | ||
| liked: PropTypes.bool, |
There was a problem hiding this comment.
I'd recommend having liked be a required prop here too
2 participants
No description provided.