Conversation
kelsey-steven-ada
left a comment
kelsey-steven-ada
left a comment
There was a problem hiding this comment.
Great work! Let me know if you have any questions on the feedback below =]
| ); | ||
| }; | ||
|
|
||
| const likeCount = messages.filter((e) => e.liked).length; |
There was a problem hiding this comment.
Great work calculating the likes count from the chatsData! Since we don't need the contents of the array we create with filter, another option is to use a higher order function like array.reduce to take our list of messages and reduce it down to a single value.
// This could be returned from a helper function
// totalLikes is a variable that accumulates a value as we loop over each entry in chatEntries
const likesCount = chatEntries.reduce((totalLikes, currentMessage) => {
// If currentMessage.liked is true add 1 to totalLikes, else add 0
return (totalLikes += currentMessage.liked ? 1 : 0);
}, 0); // The 0 here sets the initial value of totalLikes to 0| const onClickLike = (id) => { | ||
| setMessages((prevMessages) => | ||
| prevMessages.map((entry) => | ||
| entry.id === id ? { ...entry, liked: !entry.liked } : entry | ||
| ) | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Nice work ensuring we're creating a new array of messages to trigger the re-render after updating the liked value!
| <h1>Application title</h1> | ||
| <h1>Chat between Vladimir and Estragon</h1> | ||
| <section> | ||
| <span className="widget" id="heartWidget">{`${likeCount} ❤️s`}</span> |
There was a problem hiding this comment.
span is not a semantic element. It's great for when we need to dynamically lookup and affect a section of a larger text, but it should not be used to represent general text on a page.
- How does React handle this text? Do we need to dynamically look up this element and change just a part of it or is it re-rendered with the complete text each time React triggers a re-render?
- Since this is a chunk of text on the page, what HTML element or elements would better represent this subheading with the likes count?
| <p className="entry-time"> | ||
| <TimeStamp time={timeStamp} /> | ||
| </p> | ||
| <button className="like" onClick={() => onToggleLike(id)}> |
There was a problem hiding this comment.
I like this pattern of sending just the id to onToggleLike since it keeps all the state management and message object creation confined to App.
| <button className="like" onClick={() => onToggleLike(id)}> | ||
| {liked ? '❤️' : '🤍'} | ||
| </button> |
There was a problem hiding this comment.
We can increase the accessibility of our page by setting further attributes on elements, especially interactive elements. Below is an example of updates we could make to the button, check out the MDN docs for more info!
const heart = liked ? '❤️' : '🤍';
...
<button
onClick={() => onToggleLike(id)}
className="like"
aria-label={heart}
role="img"
>
{heart}
</button>| const sideClass = sender === 'Vladimir' ? 'local' : 'remote'; | ||
| const colorClass = sender === 'Vladimir' ? 'red' : 'green'; | ||
| return ( | ||
| // Replace the outer tag name with a semantic element that fits our use case | ||
| <replace-with-relevant-semantic-element className="chat-entry local"> | ||
| <h2 className="entry-name">Replace with name of sender</h2> | ||
| <article className={`chat-entry ${sideClass}`}> | ||
| <h2 className="entry-name">{sender}</h2> |
There was a problem hiding this comment.
Great use of ternary operators and interpolated strings to set the relevant CSS!
| <section className="chat-log"> | ||
| {entries.map((message) => ( | ||
| <ChatEntry | ||
| key={message.id} |
There was a problem hiding this comment.
Great use of the unique message ids for the key value!
|
|
||
| ChatLog.propTypes = { | ||
| entries: PropTypes.arrayOf( | ||
| PropTypes.shape({ |
There was a problem hiding this comment.
Great use of Prop-Types and flagging those necessary for render with isRequired, I love the use of PropTypes.shape to ensure the passed array has the necessary contents as well!
2 participants
No description provided.