Zoisite - Whitney Shake

[WhitShake]

No description provided.

[yangashley]

Great work job on react-chatlog! All the tests are passing and you did a nice job implementing the remote/local feature.

A couple things to note - you introduce an unnecessary piece of state that can be refactored away. We should prefer fewer state variables when possible to reduce complexity of our project.

Also called out some commonly followed patterns when writing code in React that would be important to implement the next time around.

Let me know if you have questions about my review comments.

[yangashley]

👍 nice job setting up state for chat messages.

Notice that you create a variable called likeCount to keep track of state that represents likes. You pass likeCount to ChatLog as a prop called likeCount on line 40. However, in ChatLog, the prop likeCount is never accessed. This means you create state, pass it as a prop and then do not use it.

We can remove line 10 because we do not need likes to be tracked as state. We can simply write a method that counts all the hearts from each chat in chatData and returns the count. Then you can use this return value to render the heart count.

Something like:

const getHeartCount = () => {
    return chatData.reduce((total, chat) => {
      return chat.liked ? total + 1 : total;
    }, 0)
};

// then on line 33, you can write this:
<h2 className="widget">{getHeartCount()} ❤️s</h2>

[yangashley]

Rather than receiving a new entry with it's like status already changed here, rewrite this to accept only an id. The logic about how to copy the message and change the liked status would be better if it was written in App.js, since the state must have all the information necessary to make a new message.

const updateEntryData = id => {
const entries = chatData.map(entry => {
      if (entry.id === id){
        return {...entry, liked: !entry.liked};
      } 
      else {
        return entry;
      }
    });
    setChatData(entries);
}

Also see my above comment about eliminating likeCount as state and creating a separate method for counting likes. When you get rid of likeCount as state, you reduce the complexity of your application.. We've confirmed that likeCount doesn't actually need to be a state since you pass it as a prop but don't use it in the child component. Therefore, you don't need to update the likeCountChange value here. You can just invoke the getHeartCount() method I detailed above

[yangashley]

👀 Since the next version of the chat data depends on the current version, prefer using the callback style of setChatData:

setChatData(prevEntries => ( 
    // logic to use current chats to make a new copy with the desired liked status flipped (the map logic above) 
    // return the new chats to update the state 
))

[yangashley]

Remember to remove debugging print statements before opening a PR for review. We want to avoid print statements being committed to the code base to keep it readable and concise

[yangashley]

See my comment in app.js about only sending updateChatData an id as a parameter (instead of a constructed updatedEntry object. That would make onHeartClick() look like:

const onHeartClick = () => {
   updateChatData(id);
};

[yangashley]

👌

[yangashley]

Nice job passing the correct prop to the provided TimeStamp component 👍

[yangashley]

Instead of creating a variable chatComponents to reference the return value from calling map() on props.entries, it's more common to directly iterate over the chat entries in your component like this without using a variable:

const ChatLog = ({entries, heartClick}) => {
    return (
     <section>
        {props.entries.map((entry) => (
            <ChatEntry 
                key={entry.id} 
                id={entry.id}
                sender={entry.sender}
                body={entry.body}
                timeStamp={entry.timeStamp}
                liked={entry.liked}
                updateChatData={props.updateChatData}
            />
        ))}
    </section>
    )
};

[yangashley]

Notice that the prop likeCount that was passed to ChatLog on line 40 in App.js isn't part of PropTypes but also isn't referenced in the rest of this component?

[yangashley]

When you write a method that doesn't need likeCount as state, you can just invoke the method getHeartCount() here between the curly braces