Heather M C17 Sea Turtles #103
Conversation
| const onLike = (id) => { | ||
| const newMessages = chatMessages.map((message) => { | ||
| if (message.id === id) { | ||
| return { ...message, liked: !message.liked }; |
There was a problem hiding this comment.
Nice managing of data & making sure we return a new object when we need to alter a message in our list. Great use of the spread operator!
| import './App.css'; | ||
| import chatMessages from './data/messages.json'; | ||
| // import ChatEntry from './components/ChatEntry'; |
There was a problem hiding this comment.
We want to remove commented code before opening PRs, especially in long term projects and when working with other folks to help avoid confusion around why the code is there.
| return `${numberOfLikes} ❤️s`; | ||
| }; | ||
|
|
||
| return <div className="hearts-count local">{hearts(props.totalLikes)}</div>; |
There was a problem hiding this comment.
We need to create some sort of wrapper for our JSX content, so we could use a div here, but since div elements don't hold semantic meaning, we want to use them as sparingly as possible. We could refactor our HTML between App and here to remove this div. One option would be to replace this div with an h3, then remove the h3 that surrounds the TotalHearts component in App. The text remains rendered inside an h3 and gets the CSS rules assigned to it, and we've removed a layer of element nesting that isn't required!
| import React from 'react'; | ||
| import PropTypes from 'prop-types'; | ||
|
|
||
| const TotalHearts = (props) => { |
There was a problem hiding this comment.
I like the idea of having this as it's own component! If we wanted to add features (like maybe showing the like count for each user on hover), it's easy to extend without cluttering our App file.
| return chatMessages.reduce((total, message) => { | ||
| return total + message.liked; | ||
| }, 0); |
There was a problem hiding this comment.
Nice use of the existing data & reduce to calculate the total likes!
|
|
||
| const ChatEntry = (props) => { | ||
| const handleLike = () => { | ||
| props.onLike(props.id); |
There was a problem hiding this comment.
I like this pattern of sending just the id to props. onLike since it keeps all the state management and message object creation confined to App.
| id: PropTypes.number.isRequired, | ||
| sender: PropTypes.string.isRequired, | ||
| body: PropTypes.string.isRequired, | ||
| timeStamp: PropTypes.string.isRequired, | ||
| liked: PropTypes.bool, | ||
| onLike: PropTypes.func, |
There was a problem hiding this comment.
Nice use of PropTypes and isRequired!
| }; | ||
|
|
||
| ChatLog.propTypes = { | ||
| entries: PropTypes.arrayOf(PropTypes.object), |
There was a problem hiding this comment.
We can be even more specific with our PropTypes for validation. If we know that we need an array, and that array's objects need to hold certain data, we can check the objects as well! Inside arrayOf we can pass PropTypes.shape() with information about the object's contents. A modified example from stack overflow is below:
list: PropTypes.arrayOf(
PropTypes.shape({
id: PropTypes.number.isRequired,
customTitle: PropTypes.string.isRequired,
btnStyle:PropTypes.object,
})
).isRequired,Source: https://stackoverflow.com/questions/59038307/reactjs-proptypes-validation-for-array-of-objects
|
|
||
| ChatLog.propTypes = { | ||
| entries: PropTypes.arrayOf(PropTypes.object), | ||
| }; |
There was a problem hiding this comment.
I recommend listing all the props we can expect to give to a particular component, including functions like onLike.
| const chatComponents = props.entries.map((chat, index) => { | ||
| return ( | ||
| <ChatEntry | ||
| key={index} |
There was a problem hiding this comment.
If our messages have unique ids, another option would be to use the message id as the key rather than an external index.
No description provided.