Conversation
|
|
||
| const ChatEntry = (props) => { | ||
|
|
||
| const chatDisplay = props.sender === 'Vladimir' ? 'local' : 'remote'; |
There was a problem hiding this comment.
👍 Nice job writing logic to determine the class to apply to the component so that the components show up with 'local' vs 'remote' styles.
| 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 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.
Good work using the provided TimeStamp component.
| import ChatEntry from './ChatEntry'; | ||
| import PropTypes from 'prop-types'; | ||
|
|
||
| const ChatLog = (props) => { |
There was a problem hiding this comment.
Like my comment above in ChatEntry, I prefer to destructure props.
| const getChatEntries = (entries) => { | ||
| return entries.map((entry) => { | ||
| return (<ChatEntry | ||
| key= {entry.id} |
There was a problem hiding this comment.
Nitpick: incorrect white spacing
| key= {entry.id} | |
| key={entry.id} |
Take care to check your style using a linter or fix up the issues while you're writing the code so that your work conforms to convention and guidelines.
There was a problem hiding this comment.
👍 The key attribute is important for React to be able to detect certain kinds of data changes in an efficient manner. We're also using the id for our own id prop, so it might feel redundant to pass both, but one is for our logic and one is for React internals.
| import entriesData from './data/messages.json'; | ||
| import { useState } from 'react'; | ||
|
|
||
| const countTotalLikes = entriesData => { |
There was a problem hiding this comment.
❗️👀 Functions that are only used within a specific component, like countTotalLikes is only used inside the App component on line 33, should be defined inside the component to make the code more readable because it keeps related logic together.
Therefore, this function should be defined in the App component (like you do with handleLikes on line 18).
| if (!entry.liked) { | ||
| return acc; | ||
| } | ||
| return acc + 1; |
There was a problem hiding this comment.
A more concise way to write this looks like:
return acc + (entry.liked ? 1 : 0);
| setEntriesData(entryData => { | ||
| return entryData.map(entry => { |
There was a problem hiding this comment.
Nice job using the callback-style for updating the state.
| <main> | ||
| {/* Wave 01: Render one ChatEntry component | ||
| Wave 02: Render ChatLog component */} | ||
| <ChatLog entries={entryData} onLikeEntry={handleLikes}></ChatLog> |
There was a problem hiding this comment.
Prefer to have opening tag, closing tag, and props all on separate lines to help with readability. React doesn't care how you write the component so the key is to be consistent in your project.
Also since ChatLog doesn't render any child elements or content within its opening and closing tags, you can use a self closing tag here.
| <ChatLog entries={entryData} onLikeEntry={handleLikes}></ChatLog> | |
| <ChatLog | |
| entries={entryData} | |
| onLikeEntry={handleLikes}> | |
| /> |
| <div id="App"> | ||
| <header> | ||
| <h1>Application title</h1> | ||
| <h2>{countTotalLikes(entryData)} ❤️s</h2> |
There was a problem hiding this comment.
❗️ ^^ above on line 32, 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?
2 participants
No description provided.