Conversation
…hatEntry, and ChatLog components
| import TimeStamp from './TimeStamp'; | ||
|
|
||
| const ChatEntry = () => { | ||
| const ChatEntry = ({ id, sender, body, timeStamp, toggleLikeChoice, liked, color }) => { |
There was a problem hiding this comment.
Nice job destructuring the variables instead of just passing props into the parameter list 👍
| <section className={`entry-bubble ${color}`}> | ||
| <p>{body}</p> | ||
| <span className="entry-time"> | ||
| <TimeStamp time={timeStamp} /> |
There was a problem hiding this comment.
Nice use of the TimeStamp component that was supplied.
| <p>Replace with body of ChatEntry</p> | ||
| <p className="entry-time">Replace with TimeStamp component</p> | ||
| <button className="like">🤍</button> | ||
| <article className={`chat-entry ${isLocal ? 'local' : 'remote'}`}> |
There was a problem hiding this comment.
Nice work computing the value of isLocal 👍
| <span className="entry-time"> | ||
| <TimeStamp time={timeStamp} /> | ||
| </span> | ||
| <button className="like" onClick={() => toggleLikeChoice(id)}>{liked ? '❤️' : '🤍'}</button> |
There was a problem hiding this comment.
👍 Passing the id of this message lets the logic defined in the App component find the correct chat entry to update.
| import './ChatLog.css'; | ||
| import PropTypes from 'prop-types'; | ||
|
|
||
| const ChatLog = ({ entries, toggleLikeChoice = () => {}, senderColors = {} }) => { |
There was a problem hiding this comment.
We should pass a function as a prop to a child component just by variable name (like how you did for in the ChatEntry component.
We don't need to provide a default no-op function. We might use no-op functions when we build a library that other developers will use in their programs and that library surfaces an option for developer-supplied functions.
Here, if no function is passed then our component shouldn't work.
The functions passed as props are part of the component’s contract and the parent is responsible for passing it. If it’s missing, that’s a usage error rather than something to silently ignore.
| const ChatLog = ({ entries, toggleLikeChoice = () => {}, senderColors = {} }) => { | |
| const ChatLog = ({ entries, toggleLikeChoice, senderColors}) => { |
| const chatEntryComponents = entries.map(entry => { | ||
| return ( | ||
| <ChatEntry | ||
| key={entry.id} |
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.
| setChatMessages(chatMessages => { | ||
| return chatMessages.map(message => { | ||
| if (message.id === id) { | ||
| return { ...message, liked: !message.liked }; | ||
| } else { | ||
| return message; | ||
| } | ||
| }); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Nice job using the callback-style for updating the state.
| const participants = Array.from(new Set(chatMessages.map(msg => msg.sender))); | ||
| const headerTitle = `Chat between ${participants.join(' and ')}`; |
There was a problem hiding this comment.
I like that you dynamically calculate the participants names for the header. This could accommodate a situation where the senders are not named Vladimir or Estragon. How might you use participants to dynamically generate the keys for the SENDER_COLORS object on lines 6-9?
| <header> | ||
| <h1>Application title</h1> | ||
| <h1 className="header">{headerTitle}</h1> | ||
| <section id="heartWidget" className="widget">{countLikes} ❤️s</section> |
There was a problem hiding this comment.
section tags are recommended to have a heading within them. Use of an h2 around the like count would avoid validation warnings, as well as resemble the sample site more closely.
2 participants
No description provided.