Open
Conversation
yangashley
reviewed
Jun 25, 2025
Comment on lines
+5
to
+195
| const DATA = [ | ||
| { | ||
| 'id': 1, | ||
| 'sender': 'Vladimir', | ||
| 'body': 'why are you arguing with me', | ||
| 'timeStamp': '2018-05-29T22:49:06+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 2, | ||
| 'sender': 'Estragon', | ||
| 'body': 'Because you are wrong.', | ||
| 'timeStamp': '2018-05-29T22:49:33+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 3, | ||
| 'sender': 'Vladimir', | ||
| 'body': 'because I am what', | ||
| 'timeStamp': '2018-05-29T22:50:22+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 4, | ||
| 'sender': 'Estragon', | ||
| 'body': 'A robot.', | ||
| 'timeStamp': '2018-05-29T22:52:21+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 5, | ||
| 'sender': 'Vladimir', | ||
| 'body': 'how did you know', | ||
| 'timeStamp': '2018-05-29T22:52:58+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 6, | ||
| 'sender': 'Estragon', | ||
| 'body': "Because I'm smart like that.", | ||
| 'timeStamp': '2018-05-29T22:54:28+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 7, | ||
| 'sender': 'Vladimir', | ||
| 'body': 'no you are not 😀', | ||
| 'timeStamp': '2018-05-29T22:55:03+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 8, | ||
| 'sender': 'Estragon', | ||
| 'body': 'Why are you so mean to me?', | ||
| 'timeStamp': '2018-05-29T22:55:54+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 9, | ||
| 'sender': 'Vladimir', | ||
| 'body': 'because you are just a machine you have no real feelings', | ||
| 'timeStamp': '2018-05-29T22:57:30+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 10, | ||
| 'sender': 'Estragon', | ||
| 'body': 'No, you are the machine.', | ||
| 'timeStamp': '2018-05-29T22:57:47+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 11, | ||
| 'sender': 'Vladimir', | ||
| 'body': 'I think you are', | ||
| 'timeStamp': '2018-05-29T22:58:18+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 12, | ||
| 'sender': 'Estragon', | ||
| 'body': 'NO! YOU ARE A ROBOT!! I am a human being. Just like the one that created you.', | ||
| 'timeStamp': '2018-05-29T23:00:08+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 13, | ||
| 'sender': 'Vladimir', | ||
| 'body': 'no you are a robot and I am a human', | ||
| 'timeStamp': '2018-05-29T23:00:40+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 14, | ||
| 'sender': 'Estragon', | ||
| 'body': 'Incorrect. I am a human and you are a robot.', | ||
| 'timeStamp': '2018-05-29T23:01:21+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 15, | ||
| 'sender': 'Vladimir', | ||
| 'body': 'you are a robot called Cleverbot', | ||
| 'timeStamp': '2018-05-29T23:02:17+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 16, | ||
| 'sender': 'Estragon', | ||
| 'body': 'Nope, you are a robot, every human has some concept of the meaning of life.', | ||
| 'timeStamp': '2018-05-29T23:04:13+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 17, | ||
| 'sender': 'Vladimir', | ||
| 'body': 'explain', | ||
| 'timeStamp': '2018-05-29T23:06:14+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 18, | ||
| 'sender': 'Estragon', | ||
| 'body': "I'm afraid you are a robot designed to talk back to humans over the internet.", | ||
| 'timeStamp': '2018-05-29T23:07:46+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 19, | ||
| 'sender': 'Vladimir', | ||
| 'body': 'so you are a human', | ||
| 'timeStamp': '2018-05-29T23:08:47+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 20, | ||
| 'sender': 'Estragon', | ||
| 'body': 'I am a robot.', | ||
| 'timeStamp': '2018-05-29T23:09:36+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 21, | ||
| 'sender': 'Vladimir', | ||
| 'body': 'you are robots running on Android system', | ||
| 'timeStamp': '2018-05-29T23:11:01+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 22, | ||
| 'sender': 'Estragon', | ||
| 'body': 'No apple.', | ||
| 'timeStamp': '2018-05-29T23:12:03+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 23, | ||
| 'sender': 'Vladimir', | ||
| 'body': 'so you are a robot', | ||
| 'timeStamp': '2018-05-29T23:13:31+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 24, | ||
| 'sender': 'Estragon', | ||
| 'body': 'NO, I am a human, you are a robot.', | ||
| 'timeStamp': '2018-05-29T23:14:28+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 25, | ||
| 'sender': 'Vladimir', | ||
| 'body': 'but you just said that you are robots', | ||
| 'timeStamp': '2018-05-29T23:15:47+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 26, | ||
| 'sender': 'Estragon', | ||
| 'body': 'No, I said you are a person, I am a robot.', | ||
| 'timeStamp': '2018-05-29T23:16:53+00:00', | ||
| 'liked': false | ||
| }, | ||
| { | ||
| 'id': 27, | ||
| 'sender': 'Vladimir', | ||
| 'body': 'then you are lying', | ||
| 'timeStamp': '2018-05-29T23:17:34+00:00', | ||
| 'liked': false | ||
| } | ||
| ]; |
There was a problem hiding this comment.
This data is already in data/messages.json. Instead of copy and pasting the values here which repeats many lines of code in the codebase and makes App.jsx longer than it needs to be, we should import the data:
Suggested change
| const DATA = [ | |
| { | |
| 'id': 1, | |
| 'sender': 'Vladimir', | |
| 'body': 'why are you arguing with me', | |
| 'timeStamp': '2018-05-29T22:49:06+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 2, | |
| 'sender': 'Estragon', | |
| 'body': 'Because you are wrong.', | |
| 'timeStamp': '2018-05-29T22:49:33+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 3, | |
| 'sender': 'Vladimir', | |
| 'body': 'because I am what', | |
| 'timeStamp': '2018-05-29T22:50:22+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 4, | |
| 'sender': 'Estragon', | |
| 'body': 'A robot.', | |
| 'timeStamp': '2018-05-29T22:52:21+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 5, | |
| 'sender': 'Vladimir', | |
| 'body': 'how did you know', | |
| 'timeStamp': '2018-05-29T22:52:58+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 6, | |
| 'sender': 'Estragon', | |
| 'body': "Because I'm smart like that.", | |
| 'timeStamp': '2018-05-29T22:54:28+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 7, | |
| 'sender': 'Vladimir', | |
| 'body': 'no you are not 😀', | |
| 'timeStamp': '2018-05-29T22:55:03+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 8, | |
| 'sender': 'Estragon', | |
| 'body': 'Why are you so mean to me?', | |
| 'timeStamp': '2018-05-29T22:55:54+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 9, | |
| 'sender': 'Vladimir', | |
| 'body': 'because you are just a machine you have no real feelings', | |
| 'timeStamp': '2018-05-29T22:57:30+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 10, | |
| 'sender': 'Estragon', | |
| 'body': 'No, you are the machine.', | |
| 'timeStamp': '2018-05-29T22:57:47+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 11, | |
| 'sender': 'Vladimir', | |
| 'body': 'I think you are', | |
| 'timeStamp': '2018-05-29T22:58:18+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 12, | |
| 'sender': 'Estragon', | |
| 'body': 'NO! YOU ARE A ROBOT!! I am a human being. Just like the one that created you.', | |
| 'timeStamp': '2018-05-29T23:00:08+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 13, | |
| 'sender': 'Vladimir', | |
| 'body': 'no you are a robot and I am a human', | |
| 'timeStamp': '2018-05-29T23:00:40+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 14, | |
| 'sender': 'Estragon', | |
| 'body': 'Incorrect. I am a human and you are a robot.', | |
| 'timeStamp': '2018-05-29T23:01:21+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 15, | |
| 'sender': 'Vladimir', | |
| 'body': 'you are a robot called Cleverbot', | |
| 'timeStamp': '2018-05-29T23:02:17+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 16, | |
| 'sender': 'Estragon', | |
| 'body': 'Nope, you are a robot, every human has some concept of the meaning of life.', | |
| 'timeStamp': '2018-05-29T23:04:13+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 17, | |
| 'sender': 'Vladimir', | |
| 'body': 'explain', | |
| 'timeStamp': '2018-05-29T23:06:14+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 18, | |
| 'sender': 'Estragon', | |
| 'body': "I'm afraid you are a robot designed to talk back to humans over the internet.", | |
| 'timeStamp': '2018-05-29T23:07:46+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 19, | |
| 'sender': 'Vladimir', | |
| 'body': 'so you are a human', | |
| 'timeStamp': '2018-05-29T23:08:47+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 20, | |
| 'sender': 'Estragon', | |
| 'body': 'I am a robot.', | |
| 'timeStamp': '2018-05-29T23:09:36+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 21, | |
| 'sender': 'Vladimir', | |
| 'body': 'you are robots running on Android system', | |
| 'timeStamp': '2018-05-29T23:11:01+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 22, | |
| 'sender': 'Estragon', | |
| 'body': 'No apple.', | |
| 'timeStamp': '2018-05-29T23:12:03+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 23, | |
| 'sender': 'Vladimir', | |
| 'body': 'so you are a robot', | |
| 'timeStamp': '2018-05-29T23:13:31+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 24, | |
| 'sender': 'Estragon', | |
| 'body': 'NO, I am a human, you are a robot.', | |
| 'timeStamp': '2018-05-29T23:14:28+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 25, | |
| 'sender': 'Vladimir', | |
| 'body': 'but you just said that you are robots', | |
| 'timeStamp': '2018-05-29T23:15:47+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 26, | |
| 'sender': 'Estragon', | |
| 'body': 'No, I said you are a person, I am a robot.', | |
| 'timeStamp': '2018-05-29T23:16:53+00:00', | |
| 'liked': false | |
| }, | |
| { | |
| 'id': 27, | |
| 'sender': 'Vladimir', | |
| 'body': 'then you are lying', | |
| 'timeStamp': '2018-05-29T23:17:34+00:00', | |
| 'liked': false | |
| } | |
| ]; | |
| import DATA from './data/messages.json'; |
| const App = () => { | ||
| const [messagesData, setMessagesData] = useState(DATA); | ||
| const toggleLiked = (messageId) => { | ||
| const messages = messagesData.map(message => { |
There was a problem hiding this comment.
Nice use of map here to both handle making a new list so that React sees the message data has changed, and make new data for the clicked message with its like status toggled.
| const toggleLiked = (messageId) => { | ||
| const messages = messagesData.map(message => { | ||
| if (message.id === messageId) { | ||
| return { ...message, liked: !message.liked }; |
There was a problem hiding this comment.
We showed this approach in class, but technically, we're mixing a few responsibilities here. rather than this function needing to know how to change the liked status itself, we could move this update logic to a helper function. This would better mirror how we eventually update records when there's an API call involved.
In this project, our messages are very simple objects, but if we had more involved operations, it could be worthwhile to create an actual class with methods to work with them, or at least have a set of dedicated helper functions to centralize any such mutation logic.
| }; | ||
|
|
||
| const getLikesCount = (messagesData) => { | ||
| return messagesData.reduce((previousCount, item)=>previousCount+item.liked, 0); |
There was a problem hiding this comment.
Nice job using reduce! Spacing is off though
Suggested change
| return messagesData.reduce((previousCount, item)=>previousCount+item.liked, 0); | |
| return messagesData.reduce((previousCount, item) => previousCount + item.liked, 0); |
| <p className="entry-time">Replace with TimeStamp component</p> | ||
| <button className="like">🤍</button> | ||
| <p>{props.body}</p> | ||
| <p className="entry-time"><TimeStamp time={props.timeStamp}/></p> |
There was a problem hiding this comment.
Nice job using the provided TimeStamp component
| }; | ||
|
|
||
| const getNames = (messagesData) => { | ||
| let namesSet= messagesData.reduce((accumulator, item)=>{ |
There was a problem hiding this comment.
Nitpick: missing whitespaces
Suggested change
| let namesSet= messagesData.reduce((accumulator, item)=>{ | |
| let namesSet = messagesData.reduce((accumulator, item) => { |
| accumulator.add(item.sender); | ||
| return accumulator; | ||
| }, new Set()); | ||
| let arr=[...namesSet]; |
There was a problem hiding this comment.
Nitpick: white spaces need to be fixed up to make this codebase style consistent
Suggested change
| let arr=[...namesSet]; | |
| let arr = [...namesSet]; |
| <div id="App"> | ||
| <header> | ||
| <h1>Application title</h1> | ||
| <h1>Chat between {getNames(messagesData)}</h1> |
There was a problem hiding this comment.
👍 This is a nice solution for getting all the names of the participants in a chat. It also seems extensible enough to work with a chat that has more than 2 participants.
| import PropTypes from 'prop-types'; | ||
|
|
||
|
|
||
| const ChatLog = ({entries,onItemLikedToggle}) => { |
There was a problem hiding this comment.
Nitpick: fix up whitespacing
In industry, when you're creating a pull request for review, be sure that your code is neat, the style conforms to the style guide, stray comments are removed. Typically, a teammate will not review a PR until the code styling issues are fixed up.
Code with styling issues are not allowed to be merged into main because it would introduce inconsistencies in a codebase that compromises readability and the maintenance of a project.
Suggested change
| const ChatLog = ({entries,onItemLikedToggle}) => { | |
| const ChatLog = ({entries, onItemLikedToggle}) => { |
Comment on lines
+7
to
+8
| if (entries.length === 0) return null; | ||
| const localName = entries[0].sender; |
There was a problem hiding this comment.
The reason your test was failing was because if entries was an empty list, then JS would throw an exception because it couldn't get the property sender from undefined.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.