C22 - Sphinx -Tatiana Trofimova #40
Conversation
| const App = () => { | ||
| const [messagesData, setMessagesData] = useState(messages); | ||
| const [likesCounter, setLikesCounter] = useState(0); |
There was a problem hiding this comment.
We can calculate the number of likes from messagesData. Therefore we should remove the likesCounter state. When possible, we should prefer to not use state if we can calculate whatever value instead so that we don't introduce additional, unnecessary complexity.
| } | ||
| }); | ||
| }); | ||
| setLikesCounter( (likesCounter) => likesCounter + likeAdjustment); |
There was a problem hiding this comment.
Instead of using state for likesCount, we should remove this state completely.
We can create a helper function that calls reduce that can get the count of all the likes from messages, like:
const getLikedCount = () => {
return messages.reduce((totalLikes, message) => (totalLikes + message.liked), 0);
};| return ( | ||
| <div id="App"> | ||
| <header> | ||
| <h1>Application title</h1> | ||
| <h1>Chat between {participants.join(' and ')}</h1> | ||
| <h2 className='heartWidget'>Likes: {likesCounter}</h2> |
There was a problem hiding this comment.
To have this element look like the provided wireframes, you would write:
<h2 className='heartWidget'>{likesCounter()} ❤️s</h2>| return ( | ||
| <div id="App"> | ||
| <header> | ||
| <h1>Application title</h1> | ||
| <h1>Chat between {participants.join(' and ')}</h1> |
There was a problem hiding this comment.
While this bit of logic isn't complex and is concise, I still think it would be nice to write a helper function and invoke the helper function here. Maybe something like getChatParticipants
| const App = () => { | ||
| const [messagesData, setMessagesData] = useState(messages); | ||
| const [likesCounter, setLikesCounter] = useState(0); | ||
| const [participants, setParticipants] = useState([]); |
There was a problem hiding this comment.
Do we need to keep track of participants by using state? Could you render the header on line 49 without it?
We should prefer to not use state when we don't absolutely need it.
| <p className="entry-time">Replace with TimeStamp component</p> | ||
| <button className="like">🤍</button> | ||
| <p>{body}</p> | ||
| <p className="entry-time">{TimeStamp(timeStamp)}</p> |
There was a problem hiding this comment.
Nice job using the provided TimeStamp component!
| return ( | ||
| <div className='chat-log'> | ||
| {feed} | ||
| </div> |
There was a problem hiding this comment.
Nice job capturing the return value from invoking map in the variable feed and using that variable here. This is typical of what you'd see in industry!
No description provided.