Conversation
mikellewade
left a comment
mikellewade
left a comment
There was a problem hiding this comment.
@FluenceMind, nice work on the project! I had a couple of questions for you that you will find in the PR review.
| import entriesData from "./data/messages.json"; | ||
|
|
||
| const App = () => { | ||
| const [entries, setEntries] = useState(entriesData); |
| const App = () => { | ||
| const [entries, setEntries] = useState(entriesData); | ||
|
|
||
| const likedCount = entries.reduce((sum, e) => sum + (e.liked ? 1 : 0), 0); |
There was a problem hiding this comment.
Great work, @FluenceMind. Though you will find a number of implementations for this, the .reduce method is the one I would prefer for the given case.
There was a problem hiding this comment.
Given that this function is not dependent on any piece of data that is locally scoped to our component you could opt to move its definition outside of the component and into, say, its own file.
| function handleToggleLike(entryId) { | ||
| setEntries((prev) => | ||
| prev.map((e) => | ||
| e.id === entryId ? { ...e, liked: !e.liked } : e |
There was a problem hiding this comment.
Very succinct, however I would encourage you to use a more descriptive variable name than just e. For someone looking to understand the function e doesn't readily explain what its structure and content may be—resulting in more cognitive load from the reader.
| <div id="App"> | ||
| <header> | ||
| <h1>Application title</h1> | ||
| <p className="liked-count">{likedCount} ❤️s</p> |
There was a problem hiding this comment.
✅
An alternative could be to call the function here instead of assigning the value to a variable earlier in the code, especially since the variable is not being leveraged in any other way.
| <main> | ||
| {/* Wave 01: Render one ChatEntry component | ||
| Wave 02: Render ChatLog component */} | ||
| <ChatLog entries={entries} onToggleLike={handleToggleLike} /> |
| const showLikeButton = | ||
| typeof id === "number" && typeof onToggleLike === "function"; |
There was a problem hiding this comment.
@FluenceMind, also wondering the reason for having this logic for deciding if the like button will be shown based off the typing for the mentioned props?
| <p className="entry-time">Replace with TimeStamp component</p> | ||
| <button className="like">🤍</button> | ||
| <p>{body}</p> | ||
|
|
| <p>{body}</p> | ||
|
|
||
| <p className="entry-time"> | ||
| <TimeStamp time={timeStamp} /> |
| {showLikeButton && ( | ||
| <button className="like" onClick={() => onToggleLike(id)}> | ||
| {liked ? "❤️" : "🤍"} |
There was a problem hiding this comment.
Agnostic of the reason, love see you implementing this! This is essentially how you have functionality like having a submit button appear only when the form is filled out correctly.
| sender: PropTypes.string.isRequired, | ||
| body: PropTypes.string.isRequired, | ||
| timeStamp: PropTypes.string.isRequired, | ||
| id: PropTypes.number, | ||
| liked: PropTypes.bool, | ||
| onToggleLike: 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.
2 participants
No description provided.